[PATCH] Initial commit for the llvm-dsymutil tool.

Alexey Samsonov vonosmas at gmail.com
Tue Dec 9 15:39:49 PST 2014


>>! In D6242#17, @friss wrote:
>>>! In D6242#15, @samsonov wrote:
>>>>! In D6242#14, @friss wrote:
>>> ping!
>> 
>> Sorry about that, will take a look at the revised patch shortly.
> 
> No worries, I wasn't really expecting anyone to do further review :-)
> 
>>> 
>>> I committed this briefly this morning considering that I could count your silence as approval, but Chandler told me not to do so. Sorry about that, I really didn't mean to force anything through.
>>> 
>>> My patch's short but intense in-tree live has produced one bot failure: http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux/builds/19112
>>> It seems the llvm-dsymutil tool hasn't been built at all there, thus I must be missing something in the build system hooks. I tried cmake and configure builds in Debug/Release before committing,  so I'm not sure what I screwed up.
>> 
>> You should probably add "llvm-dsymutil" to test/lit.cfg, next to all the other llvm-XXX tools listed there.
> 
> I don't think this will solve it. If I'm looking at the right place, then this seems to just ensure that we call the tool with its build directory added. Looking at the build log I have the feeling that llvm-dsymutil wasn't built at all. I spotted that I used the 'dsymutil' name in LLVMBuild.txt, maybe that's it. I'm just surprised that it isn't break in any of my builds.

Ah, you should also add llvm-dsymutil to LLVM_TEST_DEPENDS in test/CMakeLists.txt.

> 
> Thanks a bunch!

================
Comment at: tools/dsymutil/DebugMap.cpp:37
@@ +36,3 @@
+  typedef StringMapEntry<SymbolMapping> MapEntryTy;
+  std::vector<const MapEntryTy *> Entries;
+  Entries.reserve(Symbols.getNumItems());
----------------
friss wrote:
> samsonov wrote:
> > Looks like SymbolMapping is lightweight enough. This code can be faster if you just use
> >   std::vector<std::pair<StringRef, SymbolMapping>> Entries;
> > fill it with single scan over `Symbols`, and then `std::sort` it and print it.
> Why do you think it would be faster? locality?
Yes. But as you're comparing StringRef's, there are still indirection, so maybe it's not a big deal.

================
Comment at: tools/dsymutil/dsymutil.cpp:65
@@ +64,3 @@
+
+  llvm::DwarfLinker Linker(InputFile + ".dwarf");
+  return !Linker.link(*DebugMap.get());
----------------
friss wrote:
> samsonov wrote:
> > If InputFile is stdin ("-"), this will be "-.dwarf"
> hehe, nice catch. I'm on the edge as to what to do with this: hardcoding the output file (a.out.dwarf?) or simply refusing to do that (and maybe allowing it later by adding a -o option). 
Pipe the output to stdout? :) a.out.dwarf looks reasonable as well.

http://reviews.llvm.org/D6242






More information about the llvm-commits mailing list