[PATCH] D131589: [llvm-objdump] Handle multiple syms at same addr in disassembly.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Note to self: I still need to review the tests, but will do that once the approach re. multiple symbols has been settled on and the tests updated accordingly.



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1505-1508
+        for (const SymbolInfoTy &Symbol : SymbolsHere)
+          DemangledSymNamesHere.push_back(demangle(Symbol.Name.str()));
+        for (const std::string &DemangledName : DemangledSymNamesHere)
+          SymNamesHere.push_back(DemangledName);
----------------
simon_tatham wrote:
> jhenderson wrote:
> > You can do this, right?
> I //nearly// did, but I wasn't confident that a `StringRef` to a `std::string` stored in a `std::vector` is guaranteed to stay valid if the `std::vector` has to resize itself. What if the `std::string` implementation stores short enough strings without a separate allocation, and the vector resize involves a realloc?
> 
> So instead I did something I'm //sure// is safe, which is to set up the entire vector of strings, commit to never modifying it again, and //then// start making `StringRef`s pointing into it.
> 
> (Another option I'd have been completely confident of would be to make a vector of `unique_ptr<string>`, so that even if the vector resizes, each pointed-to string stays put. That forces even more allocations, though.)
Good point - keep the loop as-is. It turns out that the "Small String Optimization" that std::string can use under-the-hood, means that perhaps unintuitively, the string's data can be moved when the string itself is moved, rather than just the pointer to the data (see https://stackoverflow.com/questions/57723963/is-it-safe-to-store-the-pointer-to-the-data-of-a-stdstring).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131589