[PATCH] D48502: Fix direct calls to __wrap_sym when it is relocated

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 26 00:13:40 PDT 2018


grimar added inline comments.


================
Comment at: ELF/SymbolTable.cpp:213-214
+  for (WrappedSymbol &W : WrappedSymbols) {
+    memcpy(W.Wrap, W.Sym, sizeof(SymbolUnion));
+
+    // Keep this so that this copy of the symbol remains dropped
----------------
matthew.koontz wrote:
> ruiu wrote:
> > This seems a bit hacky to me, as it always overwrites W.Wrap even though we wrote some data to W.Wrap before in another function. Can you remove that code to initialize W.Wrap?
> I'm confused as to what you mean. Are you talking about line 202? That is still needed to ensure the change I made to Relocations.cpp works properly.
> 
> I agree this is kinda hacky. The problem is the relocation code affects these symbols in a way that needs to be shared between them, but the relocation code also needs to know about the wrapping. We could avoid the memcpy here and just copy the fields that have been changed, which would be more clear but prone to breaking if fields are added. I'm open to other suggestions as well.
How many fields need to be copied now if you avoid doing memcpy? I guess it might be a bit cleaner way.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D48502





More information about the llvm-commits mailing list