[PATCH] D148045: [NFC][Symbolizer] Refactor out MarkupContext

Fri Apr 21 12:23:29 PDT 2023

phosek added inline comments.

================
Comment at: llvm/include/llvm/DebugInfo/Symbolize/MarkupContext.h:37
+/// module-relative addresses.
+class MarkupContext {
+public:
----------------
mysterymath wrote:
> phosek wrote:
> > Do you think it'd be more descriptive to name this class `ProcessContext`?
> I like this a lot. It was a bit strange that llvm-objump referred to markup in the patch at the end of the stack; just calling these process contexts makes markup one way to produce them, but frees up the possibiliy for others.
That was exactly my thinking, I could imagine producing the context from other sources such as `/proc/<pid>/maps` which is frequently done even in LLVM, for example https://github.com/llvm/llvm-project/blob/fa2a8c2e1cdf188ce985d69fca6f78866390b715/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_linux.cpp or https://github.com/llvm/llvm-project/blob/fa2a8c2e1cdf188ce985d69fca6f78866390b715/lldb/source/Plugins/Process/Utility/LinuxProcMaps.cpp#L27.

Relatedly, in LLVM, we already have https://github.com/llvm/llvm-project/blob/fa2a8c2e1cdf188ce985d69fca6f78866390b715/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h#L39, https://github.com/llvm/llvm-project/blob/fa2a8c2e1cdf188ce985d69fca6f78866390b715/lldb/include/lldb/Target/MemoryRegionInfo.h#L21 and https://github.com/llvm/llvm-project/blob/fa2a8c2e1cdf188ce985d69fca6f78866390b715/llvm/include/llvm/ProfileData/MemProfData.inc#L54 which are used for similar purpose. I don't think it's worth trying to unify these at this point, but something to keep in mind for the future.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148045/new/

https://reviews.llvm.org/D148045