[PATCH] D89241: [RTDYLD] be more defensive about relocations with empty symbol names

[Jameson Nash via Phabricator via llvm-commits]

vtjnash added a comment.

I think these need to be handled in these places, given how clang emits relocations in `.dwarf` sections. For some contextual history, this was added as a sentinel for absolute symbols in https://github.com/llvm/llvm-project/commit/ad6d349fbcb2f9ced3c57b5b61231315175bcb47 (and later adjusted in https://github.com/llvm/llvm-project/commit/8f1f87c73487dcb83cf5d6d44b062f1e9796fae6). If I compare `readelf --relocations` of clang vs. gcc, I see GCC emit a name for these symbols for x86 inside the `.dwarf` sections, but that clang uses the name NULL, which gets translated there into the empty string.



================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp:311-312
                         << " flags: " << *FlagsOrErr << "\n");
+      assert(!Name.empty()); // Should not enter an empty symbol
       GlobalSymbolTable[Name] = SymbolTableEntry(SectionID, Addr, *JITSymFlags);
     } else if (SymType == object::SymbolRef::ST_Function ||
----------------



================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp:344-346
+      assert(!Name.empty()); // Should not enter an empty symbol
       GlobalSymbolTable[Name] =
           SymbolTableEntry(SectionID, SectOffset, *JITSymFlags);
----------------



================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp:1116
+        assert(!Name.empty() &&
+               "Empty symbol should not be in GlobalSymbolTable");
         // We found the symbol in our global table.  It was probably in a
----------------
This is unreachable, since line 1092 already handled it (which I think is also the reason for needing the rest of this patch)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89241