[RTDYLD] support absolute relocations where needed

[vtjnash]

Summary

These appear in some sections, such as DWARF tables, since
RuntimeDyldELF explicitly maps to this as a sentinel value:
https://github.com/llvm/llvm-project/blob/29d1fba7b5335d969e3e5daa84b7a25cd1fa75ef/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp#L1199

That could then be a source of problems if it tried to examine these
sections (for example, with either setProcessAllSections(true) or ORCv2 on i686).

Replaces https://reviews.llvm.org/D89241

[vchuravy]

Looks good from my side. @lhames should sign off though.

Is there a good standalone test that we could add?

[lhames]

Hi Jameson, Valentin,

Valentin -- Sorry I didn't get to your initial review earlier!

Jameson -- Thanks for catching the DWARF / sentinel value issue. I would have missed that.

This looks great to me. I think the original problem was misapplied relocations in the presence of empty symbols, right? In that case the correct way to test would be to write an assembly file that triggers generation of one empty symbol, plus a relocation that's expected to fail in the presence of that empty symbol. Then you can use llvm-rtdyld in -verify mode to verify that the relocation was handled correctly. See https://github.com/llvm/llvm-project/blob/master/llvm/test/ExecutionEngine/RuntimeDyld/X86/ELF-large-pic-relocations.s for an example of this style of test.

I don't know exactly which relocations were failing for you in the presence of empty symbols, but say it was plain R_X86_64_64s. You could write a program like this to generate one:

void bad_target(void) {}

void good_target(void) {}

void test_relocation(void) {
  good_target();
}

Convert it to assembly with:

% clang -mcmodel=large --target=x86_64-unknown-linux -fno-asynchronous-unwind-tables -S -o ELF_empty_symbol_name.s input.c

Then add an # rtdyld-check line to verify that the relocation applies correctly.

# RUN: rm -rf %t && mkdir -p %t
# RUN: llvm-mc -triple=x86_64-unknown-freebsd -filetype=obj -o %t/ELF_empty_symbol_name.o %s
# RUN: llvm-rtdyld -triple=x86_64-unknown-linux -verify -check=%s %t/ELF_empty_symbol_name.o

	.text
	.globl	bad_target
	.p2align	4, 0x90
	.type	bad_target,@function
bad_target:
	retq
.Lfunc_end0:
	.size	bad_target, .Lfunc_end0-bad_target

	.globl	good_target
	.p2align	4, 0x90
	.type	good_target,@function
good_target:
	retq
.Lfunc_end1:
	.size	good_target, .Lfunc_end1-good_target

	.globl	test_relocation
	.p2align	4, 0x90
	.type	test_relocation,@function

# rtdyld-check: decode-operand(test_relocation) = good_target
test_relocation:
	movabsq	$good_target, %rax
	callq	*%rax
	retq
.Lfunc_end2:
	.size	test_relocation, .Lfunc_end2-test_relocation

	.ident	"Apple clang version 12.0.0 (clang-1200.0.32.27)"
	.section	".note.GNU-stack","",@progbits
	.addrsig
	.addrsig_sym good_target

Then add whatever assembly / DWARF you need to trigger the bug and verify that the test fails. Then apply your patch and verify that it passes again.

If it's too fiddly I don't mind this going in without a test case right now -- I think extra cycles would be just as well spent bringing up / testing JITLink for ELF. We should make sure we don't replicate this issue by using empty strings as sentinels there.

I'll be on the road a lot the next few days: If you have any follow-up questions in the short term discord will be the best way to get hold of me. Otherwise I'll check back in here in a week or so.

[vtjnash]

If it's too fiddly I don't mind this going in without a test case right now -- I think extra cycles would be just as well spent bringing up / testing JITLink for ELF. We should make sure we don't replicate this issue by using empty strings as sentinels there.

Yeah, I'm thinking this is not worth chasing too much with tests, and focusing instead on JITLink. I was mostly interested in getting this working so I can start using ORCv2 and then incrementally testing the frontiers (ie JITLink). In this case, RTDyld seemed to be thinking it was a R_X86_64_32 relocation, so the main goal is to just avoid loading that section (which will abort) or doing anything too regrettable (like trying to record the address of it) as we examine the relocations list.

(Aside on this detail: I'm not sure precisely how this DWARF relocation is supposed to be represented in the struct here, but I vaguely know it should be a 32-bit section-relative symbol that's computed statically, so that no relocation needs to actually happen dynamically, regardless of how the sections eventually get laid out in memory. The debugger or unwinder is responsible for computing the real address on the fly.)

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");
-      GlobalSymbolTable[Name] = SymbolTableEntry(SectionID, Addr, *JITSymFlags);
+      if (!Name.empty()) // Skip absolute symbol relocations.
+        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");
+      if (!Name.empty()) // Skip absolute symbol relocations
         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");
+    if (!Name.empty()) // Skip absolute symbol relocations.
       GlobalSymbolTable[Name] =
           SymbolTableEntry(SectionID, Offset, std::move(*JITSymFlags));
     Offset += Size;
     Addr += Size;
   }
 
   return Error::success();
 }
 
 Expected<unsigned>
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);
   }
 }
 
auto PostResolveContinuation =
         else
           OnEmitted(std::move(O), std::move(Info), Error::success());
       };
 
   JITSymbolResolver::LookupSet Symbols;
 
   for (auto &RelocKV : SharedThis->ExternalSymbolRelocations) {
     StringRef Name = RelocKV.first();
-    assert(!Name.empty() && "Symbol has no name?");
+    if (Name.empty()) // Skip absolute symbol relocations.
+      continue;
     assert(!SharedThis->GlobalSymbolTable.count(Name) &&
            "Name already processed. RuntimeDyld instances can not be re-used "
            "when finalizing with finalizeAsync.");
     Symbols.insert(Name);
   }
 
   if (!Symbols.empty()) {
     SharedThis->Resolver.lookup(Symbols, std::move(PostResolveContinuation));