[PATCH] D58194: [DebugInfo] add SectionedAddress to DebugInfo interfaces.

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp:204
+
+  Expected<OwningBinary<Binary>> BinaryOrErr = createBinary(ModuleName);
+
----------------
avl wrote:
> avl wrote:
> > delcypher wrote:
> > > @avl This patch has broken sanitizer tests on Darwin (rdar://problem/48474543). In particular this now fails.
> > > 
> > > ```
> > > FILECHECK_DUMP_INPUT_ON_FAILURE=1 bin/llvm-lit -v projects/compiler-rt/test/asan/X86_64DarwinConfig/TestCases/Darwin/haswell-symbolication.cc
> > > ```
> > > 
> > > I've tracked down the issue to this commit and this function (`getModuleSectionIndexForAddress()`). The problem is that this code assumes the module name is the same as the path to the binary. This doesn't seem to be true on Darwin because while running the sanitizer test `llvm-symbolizer` tries to open...
> > > 
> > > ```
> > > /path/to/llvm_build/projects/compiler-rt/test/asan/X86_64DarwinConfig/TestCases/Darwin/Output/haswell-symbolication.cc.tmp.thin.x86_64:x86_64
> > > ```
> > > 
> > > Note the `:x86_64` at the end of the module name. That's the slice name inside the file and is not part of the file name itself. We need to land a fix for this or we'll need to revert this patch.
> > > 
> > > I suspect this issue must be handled elsewhere in the codebase because Darwin specific logic probably doesn't belong here.
> > > 
> > > @JDevlieghere Any ideas if there's an API that can be used here to get the binary file path from the module name?
> > > 
> > > 
> > I will research how to fix it. Make me known if there is proper solution for this, please.
> I see that situation already handled in  LLVMSymbolizer::getOrCreateModuleInfo(). I am going to handle this in similar manner :
> 
> size_t ColonPos = ModuleName.find_last_of(':');
>   // Verify that substring after colon form a valid arch name.
@avl Thanks for looking into this. That sounds sensible. It might nice to factor out that little bit of code too to avoid duplication.


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

https://reviews.llvm.org/D58194