[PATCH] D65367: [Object] Create MutableELFObject Class for Doing Mutations on ELFObjectFiles [Part 2]
Jordan Rupprecht via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 15 11:48:21 PDT 2019
rupprecht accepted this revision.
rupprecht added a comment.
This revision is now accepted and ready to land.
I don't see anything major left, but please wait for James to take another look
================
Comment at: llvm/include/llvm/Object/MutableELFObject.h:337-350
+template <typename ELFT>
+DataRefImpl MutableELFObject<ELFT>::toDataRef(uintptr_t Ptr) {
+ DataRefImpl Ref;
+ Ref.p = Ptr;
+ return Ref;
+}
+
----------------
I don't think these methods have anything to do with MutableELFObject, so they should moved `Object/SymbolicFile.h` so others can use it. (A FIXME comment is fine for now).
================
Comment at: llvm/unittests/Object/MutableELFObjectTest.cpp:364-368
+ EXPECT_EQ(MutSymOrErr->Name, "new_name");
+ TestSym = symbol_iterator(++MutableObject.symbol_begin());
+ auto NameOrErr = TestSym->getName();
+ ASSERT_THAT_EXPECTED(NameOrErr, Succeeded());
+ EXPECT_EQ(*NameOrErr, "new_name");
----------------
abrachet wrote:
> rupprecht wrote:
> > What happens to the string table in this case?
> >
> > The names for symbols in the symbol table is backed by the string table, so if you change the name from "test" to "new_name", the string table needs to be rewritten for that, but it doesn't looks like there's a corresponding `MutableTable StringTable` to back that change.
> Nothing happens for now. The idea, and this is the [[ https://github.com/llvm/llvm-project/blob/master/llvm/tools/llvm-objcopy/ELF/Object.cpp#L751 | same ]] as `objcopy::elf::Object`, is that it gets updated only when doing final layout.
I suppose it depends on how the writer/finalizer for `MutableElfObject` looks like -- a naive implementation that just iterates over sections and writes out the contents will lose the renamed values, because those will be privately owned by `MutableELFSymbol`, and not reflected in the (non-mutated) string table.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65367/new/
https://reviews.llvm.org/D65367
More information about the llvm-commits
mailing list