[LLD][COFF] Introduce hybrid symbol table for EC input files on ARM64X #119294
Conversation
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld Author: Jacek Caban (cjacek) ChangesOn hybrid ARM64X targets, ARM64 and ARM64EC input files operate in separate namespaces and cannot reference each other. This change introduces separate Patch is 58.27 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119294.diff 16 Files Affected:
diff --git a/lld/COFF/COFFLinkerContext.h b/lld/COFF/COFFLinkerContext.h
index 5d89e97a7f7761..bdd625b8c3916b 100644
--- a/lld/COFF/COFFLinkerContext.h
+++ b/lld/COFF/COFFLinkerContext.h
@@ -32,6 +32,27 @@ class COFFLinkerContext : public CommonLinkerContext {
SymbolTable symtab;
COFFOptTable optTable;
+ // A hybrid ARM64EC symbol table on ARM64X target.
+ std::optional<SymbolTable> hybridSymtab;
+
+ // Pointer to the ARM64EC symbol table: either symtab for an ARM64EC target or
+ // hybridSymtab for an ARM64X target.
+ SymbolTable *symtabEC = nullptr;
+
+ // Returns the appropriate symbol table for the specified machine type.
+ SymbolTable &getSymtab(llvm::COFF::MachineTypes machine) {
+ if (hybridSymtab && (machine == ARM64EC || machine == AMD64))
+ return *hybridSymtab;
+ return symtab;
+ }
+
+ // Invoke the specified callback for each symbol table.
+ void forEachSymtab(std::function<void(SymbolTable &symtab)> f) {
+ f(symtab);
+ if (hybridSymtab)
+ f(*hybridSymtab);
+ }
+
std::vector<ObjFile *> objFileInstances;
std::map<std::string, PDBInputFile *> pdbInputFileInstances;
std::vector<ImportFile *> importFileInstances;
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 23fab0e66bb67f..6490adcc81d66e 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -56,7 +56,7 @@ SectionChunk::SectionChunk(ObjFile *f, const coff_section *h, Kind k)
// files will be built with -ffunction-sections or /Gy, so most things worth
// stripping will be in a comdat.
if (file)
- live = !file->ctx.config.doGC || !isCOMDAT();
+ live = !file->symtab.ctx.config.doGC || !isCOMDAT();
else
live = true;
}
@@ -129,7 +129,7 @@ void SectionChunk::applyRelX64(uint8_t *off, uint16_t type, OutputSection *os,
case IMAGE_REL_AMD64_REL32_4: add32(off, s - p - 8); break;
case IMAGE_REL_AMD64_REL32_5: add32(off, s - p - 9); break;
case IMAGE_REL_AMD64_SECTION:
- applySecIdx(off, os, file->ctx.outputSections.size());
+ applySecIdx(off, os, file->symtab.ctx.outputSections.size());
break;
case IMAGE_REL_AMD64_SECREL: applySecRel(this, off, os, s); break;
default:
@@ -149,7 +149,7 @@ void SectionChunk::applyRelX86(uint8_t *off, uint16_t type, OutputSection *os,
case IMAGE_REL_I386_DIR32NB: add32(off, s); break;
case IMAGE_REL_I386_REL32: add32(off, s - p - 4); break;
case IMAGE_REL_I386_SECTION:
- applySecIdx(off, os, file->ctx.outputSections.size());
+ applySecIdx(off, os, file->symtab.ctx.outputSections.size());
break;
case IMAGE_REL_I386_SECREL: applySecRel(this, off, os, s); break;
default:
@@ -225,7 +225,7 @@ void SectionChunk::applyRelARM(uint8_t *off, uint16_t type, OutputSection *os,
case IMAGE_REL_ARM_BRANCH24T: applyBranch24T(off, sx - p - 4); break;
case IMAGE_REL_ARM_BLX23T: applyBranch24T(off, sx - p - 4); break;
case IMAGE_REL_ARM_SECTION:
- applySecIdx(off, os, file->ctx.outputSections.size());
+ applySecIdx(off, os, file->symtab.ctx.outputSections.size());
break;
case IMAGE_REL_ARM_SECREL: applySecRel(this, off, os, s); break;
case IMAGE_REL_ARM_REL32: add32(off, sx - p - 4); break;
@@ -346,7 +346,7 @@ void SectionChunk::applyRelARM64(uint8_t *off, uint16_t type, OutputSection *os,
case IMAGE_REL_ARM64_SECREL_HIGH12A: applySecRelHigh12A(this, off, os, s); break;
case IMAGE_REL_ARM64_SECREL_LOW12L: applySecRelLdr(this, off, os, s); break;
case IMAGE_REL_ARM64_SECTION:
- applySecIdx(off, os, file->ctx.outputSections.size());
+ applySecIdx(off, os, file->symtab.ctx.outputSections.size());
break;
case IMAGE_REL_ARM64_REL32: add32(off, s - p - 4); break;
default:
@@ -427,7 +427,8 @@ void SectionChunk::applyRelocation(uint8_t *off,
// section is needed to compute SECREL and SECTION relocations used in debug
// info.
Chunk *c = sym ? sym->getChunk() : nullptr;
- OutputSection *os = c ? file->ctx.getOutputSection(c) : nullptr;
+ COFFLinkerContext &ctx = file->symtab.ctx;
+ OutputSection *os = c ? ctx.getOutputSection(c) : nullptr;
// Skip the relocation if it refers to a discarded section, and diagnose it
// as an error if appropriate. If a symbol was discarded early, it may be
@@ -435,7 +436,7 @@ void SectionChunk::applyRelocation(uint8_t *off,
// it was an absolute or synthetic symbol.
if (!sym ||
(!os && !isa<DefinedAbsolute>(sym) && !isa<DefinedSynthetic>(sym))) {
- maybeReportRelocationToDiscarded(this, sym, rel, file->ctx.config.mingw);
+ maybeReportRelocationToDiscarded(this, sym, rel, ctx.config.mingw);
return;
}
@@ -443,7 +444,7 @@ void SectionChunk::applyRelocation(uint8_t *off,
// Compute the RVA of the relocation for relative relocations.
uint64_t p = rva + rel.VirtualAddress;
- uint64_t imageBase = file->ctx.config.imageBase;
+ uint64_t imageBase = ctx.config.imageBase;
switch (getArch()) {
case Triple::x86_64:
applyRelX64(off, rel.Type, os, s, p, imageBase);
@@ -669,7 +670,7 @@ void SectionChunk::getRuntimePseudoRelocs(
toString(file));
continue;
}
- int addressSizeInBits = file->ctx.config.is64() ? 64 : 32;
+ int addressSizeInBits = file->symtab.ctx.config.is64() ? 64 : 32;
if (sizeInBits < addressSizeInBits) {
warn("runtime pseudo relocation in " + toString(file) + " against " +
"symbol " + target->getName() + " is too narrow (only " +
@@ -1098,7 +1099,7 @@ void CHPERedirectionChunk::writeTo(uint8_t *buf) const {
}
ImportThunkChunkARM64EC::ImportThunkChunkARM64EC(ImportFile *file)
- : ImportThunkChunk(file->ctx, file->impSym), file(file) {}
+ : ImportThunkChunk(file->symtab.ctx, file->impSym), file(file) {}
size_t ImportThunkChunkARM64EC::getSize() const {
if (!extended)
@@ -1122,7 +1123,7 @@ void ImportThunkChunkARM64EC::writeTo(uint8_t *buf) const {
applyArm64Addr(buf + 8, exitThunkRVA, rva + 8, 12);
applyArm64Imm(buf + 12, exitThunkRVA & 0xfff, 0);
- Defined *helper = cast<Defined>(file->ctx.config.arm64ECIcallHelper);
+ Defined *helper = cast<Defined>(file->symtab.ctx.config.arm64ECIcallHelper);
if (extended) {
// Replace last instruction with an inline range extension thunk.
memcpy(buf + 16, arm64Thunk, sizeof(arm64Thunk));
@@ -1136,7 +1137,7 @@ void ImportThunkChunkARM64EC::writeTo(uint8_t *buf) const {
bool ImportThunkChunkARM64EC::verifyRanges() {
if (extended)
return true;
- auto helper = cast<Defined>(file->ctx.config.arm64ECIcallHelper);
+ auto helper = cast<Defined>(file->symtab.ctx.config.arm64ECIcallHelper);
return isInt<28>(helper->getRVA() - rva - 16);
}
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 0f6a40a41ca00f..875ada9d605394 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -160,13 +160,14 @@ class AuxImportChunk : public NonSectionChunk {
void writeTo(uint8_t *buf) const override {
uint64_t impchkVA = 0;
if (file->impchkThunk)
- impchkVA = file->impchkThunk->getRVA() + file->ctx.config.imageBase;
+ impchkVA =
+ file->impchkThunk->getRVA() + file->symtab.ctx.config.imageBase;
write64le(buf, impchkVA);
}
void getBaserels(std::vector<Baserel> *res) override {
if (file->impchkThunk)
- res->emplace_back(rva, file->ctx.config.machine);
+ res->emplace_back(rva, file->symtab.machine);
}
private:
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 714de67e88b065..64be0413f86ea1 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -241,7 +241,7 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
break;
case file_magic::pecoff_executable:
if (ctx.config.mingw) {
- ctx.symtab.addFile(make<DLLFile>(ctx, mbref));
+ ctx.symtab.addFile(make<DLLFile>(ctx.symtab, mbref));
break;
}
if (filename.ends_with_insensitive(".dll")) {
@@ -591,6 +591,25 @@ std::optional<StringRef> LinkerDriver::findLibIfNew(StringRef filename) {
return path;
}
+void LinkerDriver::setMachine(MachineTypes machine) {
+ assert(ctx.config.machine == IMAGE_FILE_MACHINE_UNKNOWN);
+ assert(machine != IMAGE_FILE_MACHINE_UNKNOWN);
+
+ ctx.config.machine = machine;
+
+ if (machine != ARM64X) {
+ ctx.symtab.machine = machine;
+ if (machine == ARM64EC)
+ ctx.symtabEC = &ctx.symtab;
+ } else {
+ ctx.symtab.machine = ARM64;
+ ctx.hybridSymtab.emplace(ctx, ARM64EC);
+ ctx.symtabEC = &*ctx.hybridSymtab;
+ }
+
+ addWinSysRootLibSearchPaths();
+}
+
void LinkerDriver::detectWinSysRoot(const opt::InputArgList &Args) {
IntrusiveRefCntPtr<vfs::FileSystem> VFS = vfs::getRealFileSystem();
@@ -1887,10 +1906,10 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
{
llvm::TimeTraceScope timeScope2("Machine arg");
if (auto *arg = args.getLastArg(OPT_machine)) {
- config->machine = getMachineType(arg->getValue());
- if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN)
+ MachineTypes machine = getMachineType(arg->getValue());
+ if (machine == IMAGE_FILE_MACHINE_UNKNOWN)
Fatal(ctx) << "unknown /machine argument: " << arg->getValue();
- addWinSysRootLibSearchPaths();
+ setMachine(machine);
}
}
@@ -2298,8 +2317,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// not we assume x64.
if (config->machine == IMAGE_FILE_MACHINE_UNKNOWN) {
Warn(ctx) << "/machine is not specified. x64 is assumed";
- config->machine = AMD64;
- addWinSysRootLibSearchPaths();
+ setMachine(AMD64);
}
config->wordsize = config->is64() ? 8 : 4;
@@ -2511,54 +2529,56 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (config->imageBase == uint64_t(-1))
config->imageBase = getDefaultImageBase();
- ctx.symtab.addSynthetic(mangle("__ImageBase"), nullptr);
- if (config->machine == I386) {
- ctx.symtab.addAbsolute("___safe_se_handler_table", 0);
- ctx.symtab.addAbsolute("___safe_se_handler_count", 0);
- }
-
- ctx.symtab.addAbsolute(mangle("__guard_fids_count"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_fids_table"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_flags"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_iat_count"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_iat_table"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_longjmp_count"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_longjmp_table"), 0);
- // Needed for MSVC 2017 15.5 CRT.
- ctx.symtab.addAbsolute(mangle("__enclave_config"), 0);
- // Needed for MSVC 2019 16.8 CRT.
- ctx.symtab.addAbsolute(mangle("__guard_eh_cont_count"), 0);
- ctx.symtab.addAbsolute(mangle("__guard_eh_cont_table"), 0);
-
- if (isArm64EC(config->machine)) {
- ctx.symtab.addAbsolute("__arm64x_extra_rfe_table", 0);
- ctx.symtab.addAbsolute("__arm64x_extra_rfe_table_size", 0);
- ctx.symtab.addAbsolute("__arm64x_redirection_metadata", 0);
- ctx.symtab.addAbsolute("__arm64x_redirection_metadata_count", 0);
- ctx.symtab.addAbsolute("__hybrid_auxiliary_delayload_iat_copy", 0);
- ctx.symtab.addAbsolute("__hybrid_auxiliary_delayload_iat", 0);
- ctx.symtab.addAbsolute("__hybrid_auxiliary_iat", 0);
- ctx.symtab.addAbsolute("__hybrid_auxiliary_iat_copy", 0);
- ctx.symtab.addAbsolute("__hybrid_code_map", 0);
- ctx.symtab.addAbsolute("__hybrid_code_map_count", 0);
- ctx.symtab.addAbsolute("__hybrid_image_info_bitfield", 0);
- ctx.symtab.addAbsolute("__x64_code_ranges_to_entry_points", 0);
- ctx.symtab.addAbsolute("__x64_code_ranges_to_entry_points_count", 0);
- ctx.symtab.addSynthetic("__guard_check_icall_a64n_fptr", nullptr);
- ctx.symtab.addSynthetic("__arm64x_native_entrypoint", nullptr);
- }
-
- if (config->pseudoRelocs) {
- ctx.symtab.addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST__"), 0);
- ctx.symtab.addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST_END__"), 0);
- }
- if (config->mingw) {
- ctx.symtab.addAbsolute(mangle("__CTOR_LIST__"), 0);
- ctx.symtab.addAbsolute(mangle("__DTOR_LIST__"), 0);
- }
- if (config->debug || config->buildIDHash != BuildIDHash::None)
- if (ctx.symtab.findUnderscore("__buildid"))
- ctx.symtab.addUndefined(mangle("__buildid"));
+ ctx.forEachSymtab([&](SymbolTable &symtab) {
+ symtab.addSynthetic(mangle("__ImageBase"), nullptr);
+ if (symtab.machine == I386) {
+ symtab.addAbsolute("___safe_se_handler_table", 0);
+ symtab.addAbsolute("___safe_se_handler_count", 0);
+ }
+
+ symtab.addAbsolute(mangle("__guard_fids_count"), 0);
+ symtab.addAbsolute(mangle("__guard_fids_table"), 0);
+ symtab.addAbsolute(mangle("__guard_flags"), 0);
+ symtab.addAbsolute(mangle("__guard_iat_count"), 0);
+ symtab.addAbsolute(mangle("__guard_iat_table"), 0);
+ symtab.addAbsolute(mangle("__guard_longjmp_count"), 0);
+ symtab.addAbsolute(mangle("__guard_longjmp_table"), 0);
+ // Needed for MSVC 2017 15.5 CRT.
+ symtab.addAbsolute(mangle("__enclave_config"), 0);
+ // Needed for MSVC 2019 16.8 CRT.
+ symtab.addAbsolute(mangle("__guard_eh_cont_count"), 0);
+ symtab.addAbsolute(mangle("__guard_eh_cont_table"), 0);
+
+ if (isArm64EC(ctx.config.machine)) {
+ symtab.addAbsolute("__arm64x_extra_rfe_table", 0);
+ symtab.addAbsolute("__arm64x_extra_rfe_table_size", 0);
+ symtab.addAbsolute("__arm64x_redirection_metadata", 0);
+ symtab.addAbsolute("__arm64x_redirection_metadata_count", 0);
+ symtab.addAbsolute("__hybrid_auxiliary_delayload_iat_copy", 0);
+ symtab.addAbsolute("__hybrid_auxiliary_delayload_iat", 0);
+ symtab.addAbsolute("__hybrid_auxiliary_iat", 0);
+ symtab.addAbsolute("__hybrid_auxiliary_iat_copy", 0);
+ symtab.addAbsolute("__hybrid_code_map", 0);
+ symtab.addAbsolute("__hybrid_code_map_count", 0);
+ symtab.addAbsolute("__hybrid_image_info_bitfield", 0);
+ symtab.addAbsolute("__x64_code_ranges_to_entry_points", 0);
+ symtab.addAbsolute("__x64_code_ranges_to_entry_points_count", 0);
+ symtab.addSynthetic("__guard_check_icall_a64n_fptr", nullptr);
+ symtab.addSynthetic("__arm64x_native_entrypoint", nullptr);
+ }
+
+ if (config->pseudoRelocs) {
+ symtab.addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST__"), 0);
+ symtab.addAbsolute(mangle("__RUNTIME_PSEUDO_RELOC_LIST_END__"), 0);
+ }
+ if (config->mingw) {
+ symtab.addAbsolute(mangle("__CTOR_LIST__"), 0);
+ symtab.addAbsolute(mangle("__DTOR_LIST__"), 0);
+ }
+ if (config->debug || config->buildIDHash != BuildIDHash::None)
+ if (symtab.findUnderscore("__buildid"))
+ symtab.addUndefined(mangle("__buildid"));
+ });
// This code may add new undefined symbols to the link, which may enqueue more
// symbol resolution tasks, so we need to continue executing tasks until we
@@ -2801,7 +2821,8 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
if (auto *arg = args.getLastArg(OPT_print_symbol_order))
config->printSymbolOrder = arg->getValue();
- ctx.symtab.initializeECThunks();
+ if (ctx.symtabEC)
+ ctx.symtabEC->initializeECThunks();
// Identify unreferenced COMDAT sections.
if (config->doGC) {
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 3889feb7511c0a..e94a961953581f 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -80,9 +80,7 @@ class LinkerDriver {
void linkerMain(llvm::ArrayRef<const char *> args);
- // Adds various search paths based on the sysroot. Must only be called once
- // config->machine has been set.
- void addWinSysRootLibSearchPaths();
+ void setMachine(llvm::COFF::MachineTypes machine);
void addClangLibSearchPaths(const std::string &argv0);
@@ -116,6 +114,10 @@ class LinkerDriver {
// Determines the location of the sysroot based on `args`, environment, etc.
void detectWinSysRoot(const llvm::opt::InputArgList &args);
+ // Adds various search paths based on the sysroot. Must only be called once
+ // config->machine has been set.
+ void addWinSysRootLibSearchPaths();
+
// Symbol names are mangled by prepending "_" on x86.
StringRef mangle(StringRef sym);
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index f32bc5bbbc35f1..30aef8d13fa363 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -78,7 +78,7 @@ const COFFSyncStream &coff::operator<<(const COFFSyncStream &s,
/// Checks that Source is compatible with being a weak alias to Target.
/// If Source is Undefined and has no weak alias set, makes it a weak
/// alias to Target.
-static void checkAndSetWeakAlias(COFFLinkerContext &ctx, InputFile *f,
+static void checkAndSetWeakAlias(SymbolTable &symtab, InputFile *f,
Symbol *source, Symbol *target,
bool isAntiDep) {
if (auto *u = dyn_cast<Undefined>(source)) {
@@ -92,9 +92,9 @@ static void checkAndSetWeakAlias(COFFLinkerContext &ctx, InputFile *f,
// of another symbol emitted near the weak symbol.
// Just use the definition from the first object file that defined
// this weak symbol.
- if (ctx.config.allowDuplicateWeak)
+ if (symtab.ctx.config.allowDuplicateWeak)
return;
- ctx.symtab.reportDuplicate(source, f);
+ symtab.reportDuplicate(source, f);
}
}
u->setWeakAlias(target, isAntiDep);
@@ -106,22 +106,23 @@ static bool ignoredSymbolName(StringRef name) {
}
ArchiveFile::ArchiveFile(COFFLinkerContext &ctx, MemoryBufferRef m)
- : InputFile(ctx, ArchiveKind, m) {}
+ : InputFile(ctx.symtab, ArchiveKind, m) {}
void ArchiveFile::parse() {
+ COFFLinkerContext &ctx = symtab.ctx;
// Parse a MemoryBufferRef as an archive file.
file = CHECK(Archive::create(mb), this);
// Try to read symbols from ECSYMBOLS section on ARM64EC.
- if (isArm64EC(ctx.config.machine)) {
+ if (ctx.symtabEC) {
iterator_range<Archive::symbol_iterator> symbols =
CHECK(file->ec_symbols(), this);
if (!symbols.empty()) {
for (const Archive::Symbol &sym : symbols)
- ctx.symtab.addLazyArchive(this, sym);
+ ctx.symtabEC->addLazyArchive(this, sym);
// Read both EC and native symbols on ARM64X.
- if (ctx.config.machine != ARM64X)
+ if (!ctx.hybridSymtab)
return;
}
}
@@ -134,14 +135,14 @@ void ArchiveFile::parse() {
// Returns a buffer pointing to a member file containing a given symbol.
void ArchiveFile::addMember(const Archive::Symbol &sym) {
const Archive::Child &c =
- CHECK(sym.getMember(),
- "could not get the member for symbol " + toCOFFString(ctx, sym));
+ CHECK(sym.getMember(), "could not get the member for symbol " +
+ toCOFFString(symtab.ctx, sym));
// Return an empty buffer if we have already returned the same buffer.
if (!seen.insert(c.getChildOffset()).second)
return;
- ctx.driver.enqueueArchiveMember(c, sym, getName());
+ symtab.ctx.driver.enqueueArchiveMember(c, sym, getName());
}
std::vector<MemoryBufferRef>
@@ -161,6 +162,9 @@ lld::coff::getArchiveMembers(COFFLinkerContext &ctx, Archive *file) {
return v;
}
+ObjFile::ObjFile(COFFLinkerContext &ctx, MemoryBufferRef m, bool lazy)
+ : InputFile(ctx.getSymtab(getMachineType(m)), ObjectKind, m, lazy) {}
+
void ObjFile::parseLazy() {
// Native object file.
std::unique_ptr<Binary> coffObjPtr = CHECK(createBinary(mb), this);
@@ -174,7 +178,7 @@ void ObjFile::parseLazy() {
StringRef name = check(coffObj->getSymbolName(coffSym));
if (coffSym.isAbsolute() && ignoredSymbolName(name))
continue;
- ctx.symtab.addLazyObject(this, name);
+ symtab.addLazyObject(this, name);
i += coffSym.getNumberOfAuxSymbols();
}
}
@@ -188,7 +192,8 @@ struct ECMapEntry {
void ObjFile::initializeECThunks() {
for (SectionChunk *chunk : hybmpChunks) {
if (chunk->getContents().size() % sizeof(ECMapEntry)) {
- Err(ctx) << "Invalid .hybmp chunk size " << chunk->getContents().size();
+ Err(symtab.ctx) << "Invalid .hybmp chunk size "
+ << chunk->getContents().size();
continue;
}
@@ -199,15 +204,15 @@ void ObjFile::initializeECThunks() {
auto entry = reinterpret_cast<const ECMapEntry *>(iter);
switch (entry->type) {
case Arm64ECThunkType::Entry:
- ctx.symtab.addEntryThunk(getSymbol(entry->src), getSymbol(entry->dst));
+ symtab.a...
[truncated]
|
|
This is the beginning of a separate namespace implementation required for hybrid ARM64X image support. This change introduces just enough functionality to handle both native and EC load configs passed to the linker. This is how it should work, but there are still many missing pieces. In particular, CHPE metadata should only be present in the EC version and copied over to the native load config. I’ve created an arm64x-loadcfg branch with additional ARM64X load config fixes to provide a clearer context. For now, I build the native load config from A lot of existing code assumes a single symbol table. Most of the remaining ARM64X changes will involve:
I will create separated PRs for changes preparing for the final commit. |
lld/COFF/Chunks.cpp
Outdated
| @@ -129,7 +129,7 @@ void SectionChunk::applyRelX64(uint8_t *off, uint16_t type, OutputSection *os, | |||
| case IMAGE_REL_AMD64_REL32_4: add32(off, s - p - 8); break; | |||
| case IMAGE_REL_AMD64_REL32_5: add32(off, s - p - 9); break; | |||
| case IMAGE_REL_AMD64_SECTION: | |||
| applySecIdx(off, os, file->ctx.outputSections.size()); | |||
| applySecIdx(off, os, file->symtab.ctx.outputSections.size()); | |||
There was a problem hiding this comment.
drive-by: why file->symtab.ctx instead of file->ctx?
The number of InputFile isn't large, and we can afford having both ctx and symtab in InputFile, if symtab is necessary.
There was a problem hiding this comment.
This change stems from addressing the opposite scenario: determining the correct symtab from the input file. With hybrid images and two namespaces, file->ctx.symtab isn’t always accurate. EC files should use the hybrid symbol table instead, which would look something like file->ctx.getSymtab(file->getMachineType()) in this PR.
My initial draft followed this approach (encapsulated in an InputFile helper) and updated all relevant code to utilize it. However, this approach felt suboptimal - it distributed complexity across the codebase and increased the risk of mistakes or omissions. Instead, storing symtab directly in InputFile and determining the correct symbol table during object construction made it easier to access and encapsulated the complexity.
I considered storing both COFFLinkerContext and SymbolTable references in the InputFile class. This would allow both file->ctx and file->symtab to work. However, since the context is easily accessible from the symbol table, this approach felt redundant. To save memory, I opted to store only symtab, replacing file->ctx with file->symtab.ctx as seen in #119296.
The required changes to use file->symtab.ctx seemed manageable to me. Additionally, with further refactoring, the number of such instances will likely decrease. For instance, commit 48b055d in #119298 removes many such cases. Another example would be updating ImportThunkChunk to store symtab instead of ctx, further reducing these instances.
|
NFC refactoring is done is separate PRs, this depends on #119298. |
There was a problem hiding this comment.
Overall this looks very straightforward, almost suspiciously straightforward :-)
Can you verbally explain what the state is after this commit? We essentially do symbol resolution separately within the both namespaces - and when writing the final image, we just serialize all included Chunks into the output image?
What's the reason why we now link two separate copies of the load config in the testcases - what difference does that make? (If we wouldn't be linking two load configs but run the old testcases as they were, what would the results be?)
| symtab.addAbsolute(mangle("__guard_eh_cont_count"), 0); | ||
| symtab.addAbsolute(mangle("__guard_eh_cont_table"), 0); | ||
|
|
||
| if (isArm64EC(ctx.config.machine)) { |
There was a problem hiding this comment.
I guess this bit, about what symbols we set in the load config, is going to change further in some subsequent change?
There was a problem hiding this comment.
Yes, this should ultimately use symtab.isEC() instead. Changing it now would break CHPE data in the native load config. CHPE metadata should only exist in the EC namespace, but maintaining compatibility with existing tests requires linker support to copy it from the EC load config to the native one, as well as updates to the writer to correctly set those symbols in the appropriate namespace. This is implemented in this commit.
lld/test/COFF/arm64x-undef.s
Outdated
|
|
||
| // RUN: not lld-link -machine:arm64x -dll -noentry -out:out.dll symref-aarch64.obj sym-arm64ec.obj \ | ||
| // RUN: 2>&1 | FileCheck --check-prefix=UNDEF %s | ||
| // UNDEF: lld-link: error: undefined symbol: sym |
There was a problem hiding this comment.
I think this test would be clearer if we'd also have a positive test, to see that symbols within each namespace can get resolved correctly. I guess that's kinda implicitly tested in all the other existing tests, but it would be nice for clarity here as well.
There was a problem hiding this comment.
Good idea, I'll add such a test.
There was a problem hiding this comment.
Done. I also added a test reading both ECSYMBOLS and native symbol indexes from an archive.
|
Thanks for reviews!
Yes, with this commit, we now have two distinct namespaces, each with independent name resolution. The writer still operates on file and/or chunk collections globally rather than being tied to a specific namespace. Its existing functionality remains unchanged, as it already handles different chunk machine types, including sorting, splitting, and aligning code chunks. Those parts are largely complete. Most of the writer and driver code still uses
PE header writer uses the native namespace to look up the load config referenced by the PE data directory. Without adding the second load config, executables created by the tests would lack a load config entirely. The hybrid load config is only accessible after applying ARM64X relocations, but those relocations are exposed to the loader via the native load config itself. Without a native load config, they wouldn't be visible (load config data directory ARM64X relocations aren't part of this patch; see this commit for how I plan to implement that part). For this commit, replacing the EC load config with the native one might work but wouldn't represent real progress. Ultimately, we should pass both load configs. The current hack, which allowed for splitting commits, involves providing CHPE metadata in the native load config. This metadata should instead be present only in the EC version, which we then copy over by the linker to the native version. See this commit for an implementation that aligns tests with the intended use of load configs on ARM64X. |
On hybrid ARM64X targets, ARM64 and ARM64EC input files operate in separate namespaces and cannot reference each other. This change introduces separate SymbolTable instances and associates each InputFile with the appropriate table to reflect this behavior.
cjacek
force-pushed
the
hybrid-symtab
branch
from
302cd6f to
1121037
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/10350 Here is the relevant piece of the build log for the reference |
|
I reverted the commit due to sanitizer test failures. |
The addFile implementation does not rely on the SymbolTable object. With llvm#119294, the symbol table for input files is determined during the construction of the objects representing them. To clarify that relationship, this change moves the implementation from the SymbolTable class to the LinkerDriver class.
The addFile implementation does not rely on the SymbolTable object. With #119294, the symbol table for input files is determined during the construction of the objects representing them. To clarify that relationship, this change moves the implementation from the SymbolTable class to the LinkerDriver class.
On hybrid ARM64X targets, ARM64 and ARM64EC input files operate in separate namespaces and cannot reference each other. This change introduces separate
SymbolTableinstances and associates eachInputFilewith the appropriate table to reflect this behavior.