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

[vchuravy]

Summary

I am unsure if we should add these asserts, so I am putting this up for discussions.

The impetus for this was https://github.com/JuliaLang/julia/pull/37842#issuecomment-706651005
IIUC we map many relocations to the symbol name "". (See https://github.com/llvm/llvm-project/blame/176249bd6732a8044d457092ed932768724a6f06/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp#L1199)
and https://github.com/llvm/llvm-project/blob/176249bd6732a8044d457092ed932768724a6f06/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp#L929 looks at GlobalSymbolTable to determine whether or not
this is an absolute relocation. If someone happens to insert a symbol that has a "" as the name, all the absolute relocations will now be treated wrong.

So my question is: We are using "" as a sentinel value for absolute relocations. Should we ensure that actually is a sentinel value, or is there a platform were "" is a valid symbol name,
and we should use a different sentinel, like the original NULL pointer?

For JuliaLang I am currently filterting out the wrong relocation, after I was unable to find the producer of it.

[vchuravy]

clang-format

[vtjnash]

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.

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

311–312

344–346

1116

This is unreachable, since line 1092 already handled it (which I think is also the reason for needing the rest of this patch)

llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

if (*FlagsOrErr & SymbolRef::SF_Absolute &&
         return AddrOrErr.takeError();
 
       unsigned SectionID = AbsoluteSymbolSection;
 
       LLVM_DEBUG(dbgs() << "\tType: " << SymType << " (absolute) Name: " << Name
                         << " SID: " << SectionID
                         << " Offset: " << format("%p", (uintptr_t)Addr)
                         << " 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 ||
                SymType == object::SymbolRef::ST_Data ||
                SymType == object::SymbolRef::ST_Unknown ||
                SymType == object::SymbolRef::ST_Other) {
 
       section_iterator SI = Obj.section_end();
       if (auto SIOrErr = I->getSection())
         SI = *SIOrErr;
if (*FlagsOrErr & SymbolRef::SF_Absolute &&
         SectionID = *SectionIDOrErr;
       else
         return SectionIDOrErr.takeError();
 
       LLVM_DEBUG(dbgs() << "\tType: " << SymType << " Name: " << Name
                         << " SID: " << SectionID
                         << " Offset: " << format("%p", (uintptr_t)SectOffset)
                         << " flags: " << *FlagsOrErr << "\n");
+      assert(!Name.empty()); // Should not enter an empty symbol
       GlobalSymbolTable[Name] =
           SymbolTableEntry(SectionID, SectOffset, *JITSymFlags);
     }
   }
 
   // Allocate common symbols
   if (auto Err = emitCommonSymbols(Obj, CommonSymbolsToAllocate, CommonSize,
                                    CommonAlign))
     return std::move(Err);
 
for (auto &Sym : SymbolsToAllocate) {
     }
     auto JITSymFlags = getJITSymbolFlags(Sym);
 
     if (!JITSymFlags)
       return JITSymFlags.takeError();
 
     LLVM_DEBUG(dbgs() << "Allocating common symbol " << Name << " address "
                       << format("%p", Addr) << "\n");
+    assert(!Name.empty()); // Should not enter an empty symbol
     GlobalSymbolTable[Name] =
         SymbolTableEntry(SectionID, Offset, std::move(*JITSymFlags));
     Offset += Size;
     Addr += Size;
   }
 
   return Error::success();
 }
void RuntimeDyldImpl::addRelocationForSymbol(const RelocationEntry &RE,
                                              StringRef SymbolName) {
   // Relocation by symbol.  If the symbol is found in the global symbol table,
   // create an appropriate section relocation.  Otherwise, add it to
   // ExternalSymbolRelocations.
   RTDyldSymbolTable::const_iterator Loc = GlobalSymbolTable.find(SymbolName);
   if (Loc == GlobalSymbolTable.end()) {
     ExternalSymbolRelocations[SymbolName].push_back(RE);
   } else {
+    assert(!SymbolName.empty() &&
+           "Empty symbol should not be in GlobalSymbolTable");
     // Copy the RE since we want to modify its addend.
     RelocationEntry RECopy = RE;
     const auto &SymInfo = Loc->second;
     RECopy.Addend += SymInfo.getOffset();
     Relocations[SymInfo.getSectionID()].push_back(RECopy);
   }
 }
 
if (Name.size() == 0) {
         // The call to getSymbolAddress may have caused additional modules to
         // be loaded, which may have added new entries to the
         // ExternalSymbolRelocations map.  Consquently, we need to update our
         // iterator.  This is also why retrieval of the relocation list
         // associated with this symbol is deferred until below this point.
         // New entries may have been added to the relocation list.
         i = ExternalSymbolRelocations.find(Name);
       } else {
+        assert(!Name.empty() &&
+               "Empty symbol should not be in GlobalSymbolTable");
         // We found the symbol in our global table.  It was probably in a
         // Module that we loaded previously.
         const auto &SymInfo = Loc->second;
         Addr = getSectionLoadAddress(SymInfo.getSectionID()) +
                SymInfo.getOffset();
         Flags = SymInfo.getFlags();
       }