[PATCH] D89432: [llvm-elfabi] Emit ELF .dynsym and .dynamic sections

[Haowei Wu via Phabricator via llvm-commits]

haowei added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:150
+    Entries[Index] = Entry;
+  }
+
----------------
grimar wrote:
> Can't this be the following?
> 
> ```
>   void modifyAddr(size_t Index, uint64_t Addr) {
>     Entries[Index].d_un.d_ptr = Addr;
>   }
> ```
> 
> (the same below)
Fixed in latest diff.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:233
+      uint16_t Shndx = Sym.Undefined ? SHN_UNDEF : 1;
+      DynSym.Content.add(StrTab.Content.getOffset(Sym.Name), Sym.Size, Bind,
+                         (uint8_t)Sym.Type, 0, Shndx);
----------------
grimar wrote:
> I am not sure what is happening here. Could you explain?
> Why a symbol is attached to `.text` and why it is assumed that `.text` has index 1?
> Also, where it is tested?
This snippet adds symbols from the ELFStub structure into the .dynsym section builder. The "1" here is a placeholder. It does not mean ".text" has to be index 1. The issue here is for non-Undefined symbols, the shndx needs to be a value other than 0. Please correct me if I am wrong, at link time, for dynamic linking, the shndx value does not matter as long as it is not SHN_UNDEF for non-Undefined symbols, that why I put 1 here, which in current state, it points to ".dynsym" itself.

The TODO message means I would like to add a dummy place holder ".text" section and point symbols to it in the stub SO in case any tool need a valid shndx value. For linking with ld/lld, I don't think that is necessary.

I added section name in the binary-write-sheaders.test to verify shndx is filled with expected value. I do want to add some integration test to verify this elfabi tool will work with clang and lld, but it looks like I don't have access to the lld in the llvm test. If you have any good suggestion for integration testing, please let me know.

We did performed end to end testing with elfabi tool in Fuchsia build and the linked binary from stub SO were equivalent to the ones linked from original SO files. If we chose to preserve the order of symbol table, the build results were the same bit by bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89432