-
Notifications
You must be signed in to change notification settings - Fork 331
Redacted registry memory mapping in crash tracking #11025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
8f17333
b8ae4df
ab51afc
c591230
f3b0447
e3ba878
61998ec
3ddb175
c71deb4
e6f6472
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -62,6 +62,7 @@ enum State { | |||||||||||||
| SUMMARY, | ||||||||||||||
| THREAD, | ||||||||||||||
| STACKTRACE, | ||||||||||||||
| REGISTER_TO_MEMORY_MAPPING, | ||||||||||||||
| REGISTERS, | ||||||||||||||
| PROCESS, | ||||||||||||||
| VM_ARGUMENTS, | ||||||||||||||
|
|
@@ -95,12 +96,15 @@ public HotspotCrashLogParser() { | |||||||||||||
| // per line. | ||||||||||||||
| private static final Pattern REGISTER_ENTRY_PARSER = | ||||||||||||||
| Pattern.compile("([A-Za-z][A-Za-z0-9]*)\\s*=\\s*(0x[0-9a-fA-F]+)"); | ||||||||||||||
| private static final Pattern REGISTER_TO_MEMORY_MAPPING_PARSER = | ||||||||||||||
| Pattern.compile("^\\s*([A-Za-z][A-Za-z0-9]*)\\s*="); | ||||||||||||||
| // Used for the REGISTERS-state exit condition only: the register name must start the line | ||||||||||||||
| // (after optional whitespace). This prevents lines like "Top of Stack: (sp=0x...)" and | ||||||||||||||
| // "Instructions: (pc=0x...)" from being mistaken for register entries by REGISTER_ENTRY_PARSER's | ||||||||||||||
| // find(), which would otherwise match the lowercase "sp"/"pc" tokens embedded in those lines. | ||||||||||||||
| private static final Pattern REGISTER_LINE_START = | ||||||||||||||
| Pattern.compile("^\\s*[A-Za-z][A-Za-z0-9]*\\s*=\\s*0x"); | ||||||||||||||
| private static final Pattern SUBSECTION_TITLE = Pattern.compile("^\\s*[A-Za-z][\\w ]*:.+$"); | ||||||||||||||
| private static final Pattern COMPILED_JAVA_ADDRESS_PARSER = | ||||||||||||||
| Pattern.compile("@\\s+(0x[0-9a-fA-F]+)\\s+\\[(0x[0-9a-fA-F]+)\\+(0x[0-9a-fA-F]+)\\]"); | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -350,10 +354,14 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| String datetimeRaw = null; | ||||||||||||||
| boolean incomplete = false; | ||||||||||||||
| String oomMessage = null; | ||||||||||||||
| Map<String, String> registers = null; | ||||||||||||||
| Map<String, String> registers = new LinkedHashMap<>(); | ||||||||||||||
| Map<String, String> registerToMemoryMapping = new LinkedHashMap<>(); | ||||||||||||||
| String currentRegisterToMemoryMapping = ""; | ||||||||||||||
| List<String> runtimeArgs = null; | ||||||||||||||
| List<String> dynamicLibraryLines = null; | ||||||||||||||
| String dynamicLibraryKey = null; | ||||||||||||||
| boolean previousLineBlank = false; | ||||||||||||||
| State nextThreadSectionState = null; | ||||||||||||||
|
|
||||||||||||||
| String[] lines = NEWLINE_SPLITTER.split(crashLog); | ||||||||||||||
| outer: | ||||||||||||||
|
|
@@ -410,7 +418,10 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| } | ||||||||||||||
| break; | ||||||||||||||
| case STACKTRACE: | ||||||||||||||
| if (line.startsWith("siginfo:")) { | ||||||||||||||
| nextThreadSectionState = nextThreadSectionState(line, previousLineBlank); | ||||||||||||||
| if (nextThreadSectionState != null) { | ||||||||||||||
| state = nextThreadSectionState; | ||||||||||||||
| } else if (line.startsWith("siginfo:")) { | ||||||||||||||
| // spotless:off | ||||||||||||||
| // siginfo: si_signo: 11 (SIGSEGV), si_code: 1 (SEGV_MAPERR), si_addr: 0x70 | ||||||||||||||
| // siginfo: si_signo: 11 (SIGSEGV), si_code: 0 (SI_USER), si_pid: 554848, si_uid: 1000 | ||||||||||||||
|
|
@@ -426,11 +437,6 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| Integer siUid = safelyParseInt(siginfoMatcher.group(7)); | ||||||||||||||
| sigInfo = new SigInfo(number, name, siCode, sigAction, address, siPid, siUid); | ||||||||||||||
| } | ||||||||||||||
| } else if (line.startsWith("Registers:")) { | ||||||||||||||
| registers = new LinkedHashMap<>(); | ||||||||||||||
| state = State.REGISTERS; | ||||||||||||||
| } else if (line.contains("P R O C E S S")) { | ||||||||||||||
| state = State.PROCESS; | ||||||||||||||
| } else { | ||||||||||||||
| // Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code) | ||||||||||||||
| final StackFrame frame = parseLine(line); | ||||||||||||||
|
|
@@ -439,8 +445,28 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| break; | ||||||||||||||
| case REGISTER_TO_MEMORY_MAPPING: | ||||||||||||||
| nextThreadSectionState = nextThreadSectionState(line, previousLineBlank); | ||||||||||||||
| if (nextThreadSectionState != null) { | ||||||||||||||
| currentRegisterToMemoryMapping = ""; | ||||||||||||||
| state = nextThreadSectionState; | ||||||||||||||
| } else if (!line.isEmpty()) { | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Ghost: argus] [Confidence: 95%] [Severity: HIGH] In the In practice
Suggested change
|
||||||||||||||
| final Matcher m = REGISTER_TO_MEMORY_MAPPING_PARSER.matcher(line); | ||||||||||||||
| if (m.lookingAt()) { | ||||||||||||||
| currentRegisterToMemoryMapping = m.group(1); | ||||||||||||||
| registerToMemoryMapping.put( | ||||||||||||||
| currentRegisterToMemoryMapping, line.substring(line.indexOf('=') + 1)); | ||||||||||||||
|
Comment on lines
+457
to
+458
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Ghost: heimdall]
Suggested change
|
||||||||||||||
| } else if (!currentRegisterToMemoryMapping.isEmpty()) { | ||||||||||||||
| registerToMemoryMapping.computeIfPresent( | ||||||||||||||
| currentRegisterToMemoryMapping, (key, value) -> value + "\n" + line); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| break; | ||||||||||||||
| case REGISTERS: | ||||||||||||||
| if (!line.isEmpty() && !REGISTER_LINE_START.matcher(line).find()) { | ||||||||||||||
| nextThreadSectionState = nextThreadSectionState(line, previousLineBlank); | ||||||||||||||
| if (nextThreadSectionState != null) { | ||||||||||||||
| state = nextThreadSectionState; | ||||||||||||||
| } else if (!line.isEmpty() && !REGISTER_LINE_START.matcher(line).find()) { | ||||||||||||||
| // non-empty line that does not start with a register entry signals end of section | ||||||||||||||
| state = State.STACKTRACE; | ||||||||||||||
| } else { | ||||||||||||||
|
|
@@ -508,6 +534,7 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| // unexpected parser state; bail out | ||||||||||||||
| break outer; | ||||||||||||||
| } | ||||||||||||||
| previousLineBlank = line.isEmpty(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // PROCESS and SYSTEM sections are late enough that all critical data is captured | ||||||||||||||
|
|
@@ -571,10 +598,16 @@ public CrashLog parse(String uuid, String crashLog) { | |||||||||||||
| Metadata metadata = new Metadata("dd-trace-java", VersionInfo.VERSION, "java", null); | ||||||||||||||
| Integer parsedPid = safelyParseInt(pid); | ||||||||||||||
| ProcInfo procInfo = parsedPid != null ? new ProcInfo(parsedPid) : null; | ||||||||||||||
| Map<String, String> resolvedMapping = null; | ||||||||||||||
| if (!registerToMemoryMapping.isEmpty()) { | ||||||||||||||
| registerToMemoryMapping.replaceAll((k, v) -> RedactUtils.redactRegisterToMemoryMapping(v)); | ||||||||||||||
| resolvedMapping = registerToMemoryMapping; | ||||||||||||||
| } | ||||||||||||||
| Experimental experimental = | ||||||||||||||
| (registers != null && !registers.isEmpty()) | ||||||||||||||
| !registers.isEmpty() | ||||||||||||||
| || resolvedMapping != null | ||||||||||||||
| || (runtimeArgs != null && !runtimeArgs.isEmpty()) | ||||||||||||||
| ? new Experimental(registers, runtimeArgs) | ||||||||||||||
| ? new Experimental(registers, resolvedMapping, runtimeArgs) | ||||||||||||||
| : null; | ||||||||||||||
| DynamicLibs files = | ||||||||||||||
| (dynamicLibraryLines != null && !dynamicLibraryLines.isEmpty()) | ||||||||||||||
|
|
@@ -608,6 +641,25 @@ static String dateTimeToISO(String datetime) { | |||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private static State nextThreadSectionState(String line, boolean previousLineBlank) { | ||||||||||||||
| if (line.startsWith("Register to memory mapping:")) { | ||||||||||||||
| return State.REGISTER_TO_MEMORY_MAPPING; | ||||||||||||||
| } | ||||||||||||||
| if (line.startsWith("Registers:")) { | ||||||||||||||
| return State.REGISTERS; | ||||||||||||||
| } | ||||||||||||||
| if (line.startsWith("siginfo:")) { | ||||||||||||||
| return null; | ||||||||||||||
| } | ||||||||||||||
| if (line.contains("P R O C E S S")) { | ||||||||||||||
| return State.PROCESS; | ||||||||||||||
| } | ||||||||||||||
| if (previousLineBlank && SUBSECTION_TITLE.matcher(line).matches()) { | ||||||||||||||
| return State.STACKTRACE; | ||||||||||||||
| } | ||||||||||||||
| return null; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** | ||||||||||||||
| * Detects whether the Dynamic libraries section comes from Linux {@code /proc/self/maps} (address | ||||||||||||||
| * range format {@code addr-addr perms ...}) or from the BSD/macOS dyld callback (format {@code | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since
registersis now always initialized tonew LinkedHashMap<>()here,ucontextin theExperimentalobject will never benull(it may be empty, but not null). Does that mean thepayload.experimental.ucontext != nullguard inCrashUploaderis now superfluous? And should an emptyregistersmap still produce a"ucontext": {}entry in the payload, or should the serializer skip it when the map is empty?