[PATCH] D131290: [llvm-objdump] Create name for fake sections

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/include/llvm/Object/ELF.h:795
+    // Create a section name based on the p_type and index.
+    SecNames.push_back(StringRef("PT_LOAD#" + std::to_string(Idx)));
+    FakeShdr.sh_name = StrTab.add(SecNames.back());
----------------
I'm pretty sure this isn't going to work as intended in many cases: you create a StringRef to a std::string temporary, and stick that in SecNames. It's likely that the StringRef will be pointing at invalid memory immediately after its construction (it's likely just working because nothing has scribbled over the data in memory yet). Could you not just populate `SecNames` with the result of the `"PT_LOAD#" + std::to_string(Idx)` expression directly?

If the StringTableBuilder contains StringRefs to the strings, using a std::vector will potentially invalidate them, if it needs to increase its capacity, right? We probably want a different strategy here. Probably simplest is simply to give the vector an initial capacity equal to the total number of program headers. Given that there are rarely that many, this shouldn't have any significant impact on performance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131290