Merge branch 'seanmcm/localizationJune29_2' of https://github.com/microsoft/vscode-cpptools into seanmcm/localizationJune29_2

Author: sean-mcmanus

Extension/src/LanguageServer/extension.ts

@@ -1165,13 +1165,10 @@ export function watchForCrashes(crashDirectory: string): void {
 let previousCrashData: string;
 let previousCrashCount: number = 0;
 
-function logCrashTelemetry(data: string, type: string, offsetData?: string, crashLog?: string): void {
+function logCrashTelemetry(data: string, type: string, crashLog?: string): void {
     const crashObject: Record<string, string> = {};
     const crashCountObject: Record<string, number> = {};
     crashObject.CrashingThreadCallStack = data;
-    if (offsetData !== undefined) {
-        crashObject.CrashingThreadCallStackOffsets = offsetData;
-    }
     if (crashLog !== undefined) {
         crashObject.CrashLog = crashLog;
     }
@@ -1185,8 +1182,8 @@ function logMacCrashTelemetry(data: string): void {
     logCrashTelemetry(data, "MacCrash");
 }
 
-function logCppCrashTelemetry(data: string, offsetData?: string, crashLog?: string): void {
-    logCrashTelemetry(data, "CppCrash", offsetData, crashLog);
+function logCppCrashTelemetry(data: string, crashLog?: string): void {
+    logCrashTelemetry(data, "CppCrash", crashLog);
 }
 
 function handleMacCrashFileRead(err: NodeJS.ErrnoException | undefined | null, data: string): void {
@@ -1292,6 +1289,39 @@ function containsFilteredTelemetryData(str: string): boolean {
     return regex.test(str);
 }
 
+// Non-null fault addresses are randomized by ASLR (and use-after-free/wild pointers vary run to
+// run), so embedding the raw value in CrashingThreadCallStack would fragment crash buckets and
+// make CrashCount meaningless. Preserve near-null addresses (typical null-pointer dereferences,
+// which are stable and useful for bucketing), but replace arbitrary addresses with a stable
+// placeholder so identical crashes still de-duplicate.
+function bucketSignalAddress(address: string): string {
+    let value: bigint;
+    try {
+        value = BigInt(address.trim());
+    } catch {
+        return address; // Not a parseable address; leave it untouched.
+    }
+    // 0x10000 (64 KB) covers null plus small member/array offsets off a null pointer.
+    return value < 0x10000n ? address : "<non-null>";
+}
+
+// An unsymbolized frame is reported as a raw runtime address. Addresses in the fixed-base main
+// executable (non-PIE on Linux) stay constant across runs and are useful for bucketing, but
+// addresses in the ASLR-randomized shared-library/mmap region (Linux 0x7f..., and on macOS the
+// PIE main image and dyld shared cache) shift every launch and would fragment crash buckets. Keep
+// the low, fixed addresses but replace high (relocated) ones with a stable placeholder. 4 GB is a
+// safe cut: a non-PIE executable's own code loads well below it, while the relocated region is far
+// above it.
+function bucketFrameAddress(address: string): string {
+    let value: bigint;
+    try {
+        value = BigInt(address.trim());
+    } catch {
+        return address; // Not a parseable address; leave it untouched.
+    }
+    return value < 0x100000000n ? address : "<relocated>";
+}
+
 async function handleCrashFileRead(crashDirectory: string, crashFile: string, crashDate: Date, err: NodeJS.ErrnoException | undefined | null, data: string): Promise<void> {
     if (err) {
         if (err.code === "ENOENT") {
@@ -1301,16 +1331,15 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, cr
     }
 
     const lines: string[] = data.split("\n");
-    let addressData: string;
-    const isCppToolsSrv2: boolean = crashFile.startsWith("cpptools-srv2");
-    const isCppToolsSrv: boolean = crashFile.startsWith("cpptools-srv");
-    const telemetryHeader: string = (isCppToolsSrv2 ? "cpptools-srv2.txt" : isCppToolsSrv ? "cpptools-srv.txt" : crashFile) + "\n";
+    let signalInfo: string;
+    const processName: string = (crashFile.startsWith("cpptools-srv2") ? "cpptools-srv2 process" :
+        crashFile.startsWith("cpptools-srv") ? "cpptools-srv process" :
+            crashFile.startsWith("cpptools-wordexp") ? "cpptools-wordexp process" : "cpptools process") + "\n";
     const filtPath: string | null = which.sync("c++filt", { nothrow: true });
     const isMac: boolean = process.platform === "darwin";
     const startStr: string = isMac ? " _" : "<";
     const offsetStr: string = isMac ? " + " : "+";
     const endOffsetStr: string = isMac ? " " : " <";
-    const dotStr: string = "…\n";
     let signalType: string;
     let crashLog: string = "";
     let crashStackStartLine: number = 0;
@@ -1333,40 +1362,38 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, cr
     }
     if (lines[crashStackStartLine].startsWith("SIG")) {
         signalType = `${lines[crashStackStartLine]}\n`;
-        addressData = `${lines[crashStackStartLine + 1]}:${lines[crashStackStartLine + 2]}\n`; // signalCode:signalAddr
+        signalInfo = `si_code=${lines[crashStackStartLine + 1]}, si_addr=${bucketSignalAddress(lines[crashStackStartLine + 2])}\n`;
         crashStackStartLine += 3;
     } else {
         // The signal type may fail to be written.
         // Intentionally different from SIGUNKNOWN from cpptools,
         // and not SIG-? to avoid matching the regex in containsFilteredTelemetryData.
         signalType = "SIGMISSING\n";
-        addressData = ".\n";
+        signalInfo = "";
     }
-    data = telemetryHeader + signalType;
+    data = processName + signalType + signalInfo;
     let crashCallStack: string = "";
     let validFrameFound: boolean = false;
     for (let lineNum: number = crashStackStartLine; lineNum < lines.length - 3; ++lineNum) { // skip last lines
         const line: string = lines[lineNum];
         const startPos: number = line.indexOf(startStr);
         let pendingCallStack: string = "";
         if (startPos === -1 || line[startPos + (isMac ? 1 : 4)] === "+") {
-            pendingCallStack = dotStr;
             const startAddressPos: number = line.indexOf("0x");
             const endAddressPos: number = line.indexOf(endOffsetStr, startAddressPos + 2);
             if (startAddressPos === -1 || endAddressPos === -1 || startAddressPos >= endAddressPos) {
-                addressData += "Unexpected offset\n";
+                pendingCallStack = "Unexpected offset\n";
             } else {
-                let pendingAddressData: string = line.substring(startAddressPos, endAddressPos) + "\n";
+                let pendingAddressData: string = bucketFrameAddress(line.substring(startAddressPos, endAddressPos)) + "\n";
                 if (containsFilteredTelemetryData(pendingAddressData)) {
                     pendingAddressData = "?\n";
                 }
-                addressData += pendingAddressData;
+                pendingCallStack = pendingAddressData;
             }
         } else {
             const offsetPos: number = line.indexOf(offsetStr, startPos + startStr.length);
             if (offsetPos === -1) {
                 pendingCallStack = "Missing offsetStr\n";
-                addressData += "\n";
             } else {
                 const startPos2: number = startPos + 1;
                 let funcStr: string = line.substring(startPos2, offsetPos);
@@ -1397,27 +1424,13 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, cr
                 // Compute pendingOffset.
                 if (isMac) {
                     pendingOffset += line.substring(offsetPos2);
-                    const startAddressPos: number = line.indexOf("0x");
-                    if (startAddressPos === -1 || startAddressPos >= startPos) {
-                        // unexpected
-                        pendingOffset += "<Missing 0x>";
-                        addressData += "\n";
-                    } else {
-                        let pendingAddressData: string = line.substring(startAddressPos, startPos) + "\n";
-                        if (containsFilteredTelemetryData(pendingAddressData)) {
-                            pendingAddressData = "?\n";
-                        }
-                        addressData += pendingAddressData;
-                    }
                 } else {
                     const endPos: number = line.indexOf(">", offsetPos2);
                     if (endPos === -1) {
                         pendingOffset += "<Missing > >"; // unexpected
                     } else {
                         pendingOffset += line.substring(offsetPos2, endPos);
                     }
-                    addressData += "\n";
-                    // TODO: It seems like addressData should be obtained on Linux in case the function is filtered.
                 }
                 pendingOffset += "\n";
                 pendingCallStack = funcStr + pendingOffset;
@@ -1447,19 +1460,18 @@ async function handleCrashFileRead(crashDirectory: string, crashFile: string, cr
     }
 
     crashCallStack = crashCallStack.trimEnd();
-    addressData = addressData.trimEnd();
 
     if (crashCallStack !== prevCppCrashCallStackData) {
         prevCppCrashCallStackData = crashCallStack;
 
         if (lines.length >= 6 && util.getLoggingLevel() >= 1) {
-            getCrashCallStacksChannel().appendLine(`\n${isCppToolsSrv2 ? "cpptools-srv2" : isCppToolsSrv ? "cpptools-srv" : "cpptools"}\n${crashDate.toLocaleString()}\n${signalType}${crashCallStack}${crashLog.length > 0 ? "\n\n" + crashLog : ""}`);
+            getCrashCallStacksChannel().appendLine(`\n${processName}${crashDate.toLocaleString()}\n${signalType}${signalInfo}${crashCallStack}${crashLog.length > 0 ? "\n\n" + crashLog : ""}`);
         }
     }
 
     data += crashCallStack;
 
-    logCppCrashTelemetry(data, addressData, crashLog);
+    logCppCrashTelemetry(data, crashLog);
 
     await util.deleteFile(path.resolve(crashDirectory, crashFile)).catch(logAndReturn.undefined);
     if (crashFile === "cpptools.txt") {

Extension/src/LanguageServer/settings.ts

@@ -4,11 +4,10 @@
  * ------------------------------------------------------------------------------------------ */
 'use strict';
 
-import { execSync } from 'child_process';
+import { execFileSync } from 'child_process';
 import * as os from 'os';
 import * as path from 'path';
 import * as semver from 'semver';
-import { quote } from 'shell-quote';
 import * as vscode from 'vscode';
 import * as nls from 'vscode-nls';
 import * as which from 'which';
@@ -297,7 +296,7 @@ export class CppSettings extends Settings {
                 let bundledVersion: string;
                 try {
                     const bundledPath: string = getExtensionFilePath(`./LLVM/bin/${clangName}`);
-                    const output: string = execSync(quote([bundledPath, '--version'])).toString();
+                    const output: string = execFileSync(bundledPath, ['--version']).toString();
                     bundledVersion = output.match(/(\d+\.\d+\.\d+)/)?.[1] ?? "";
                     if (!semver.valid(bundledVersion)) {
                         return path;
@@ -309,7 +308,7 @@ export class CppSettings extends Settings {
 
                 // Invoke the version on the system to compare versions.  Use ours if it's more recent.
                 try {
-                    const output: string = execSync(`"${path}" --version`).toString();
+                    const output: string = execFileSync(path, ['--version']).toString();
                     const userVersion = output.match(/(\d+\.\d+\.\d+)/)?.[1] ?? "";
                     if (semver.ltr(userVersion, bundledVersion)) {
                         path = "";