[LLD][COFF][NFC] Always align null chunks #116677
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is similar to #80014 and #84697. Currently, null chunks always follow other aligned chunks, so this patch is NFC. However, it will become observable once support for ARM64X imports is added. The import tables are shared between the native and EC views. They are usually very similar, but in cases where they differ, ARM64X relocations handle the discrepancies. If a DLL is only imported by EC code, the native view will see it as importing zero functions from this DLL (with ARM64X relocations replacing those null chunks with actual imports). In this scenario, the null chunks may appear as the very first chunks, meaning there is nothing else forcing their alignment. |
|
@llvm/pr-subscribers-platform-windows @llvm/pr-subscribers-lld-coff Author: Jacek Caban (cjacek) ChangesFull diff: https://github.com/llvm/llvm-project/pull/116677.diff 1 Files Affected:
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 2d20b094888c7a..797d9f1490253a 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -131,7 +131,14 @@ class ImportDirectoryChunk : public NonSectionChunk {
// Contents of this chunk is always null bytes.
class NullChunk : public NonSectionChunk {
public:
- explicit NullChunk(size_t n) : size(n) { hasData = false; }
+ explicit NullChunk(size_t n, uint32_t align) : size(n) {
+ hasData = false;
+ setAlignment(align);
+ }
+ explicit NullChunk(COFFLinkerContext &ctx)
+ : NullChunk(ctx.config.wordsize, ctx.config.wordsize) {}
+ explicit NullChunk(COFFLinkerContext &ctx, size_t n)
+ : NullChunk(n, ctx.config.wordsize) {}
size_t getSize() const override { return size; }
void writeTo(uint8_t *buf) const override {
@@ -737,11 +744,11 @@ void IdataContents::create(COFFLinkerContext &ctx) {
}
}
// Terminate with null values.
- lookups.push_back(make<NullChunk>(ctx.config.wordsize));
- addresses.push_back(make<NullChunk>(ctx.config.wordsize));
+ lookups.push_back(make<NullChunk>(ctx));
+ addresses.push_back(make<NullChunk>(ctx));
if (ctx.config.machine == ARM64EC) {
- auxIat.push_back(make<NullChunk>(ctx.config.wordsize));
- auxIatCopy.push_back(make<NullChunk>(ctx.config.wordsize));
+ auxIat.push_back(make<NullChunk>(ctx));
+ auxIatCopy.push_back(make<NullChunk>(ctx));
}
for (int i = 0, e = syms.size(); i < e; ++i)
@@ -755,7 +762,7 @@ void IdataContents::create(COFFLinkerContext &ctx) {
dirs.push_back(dir);
}
// Add null terminator.
- dirs.push_back(make<NullChunk>(sizeof(ImportDirectoryTableEntry)));
+ dirs.push_back(make<NullChunk>(sizeof(ImportDirectoryTableEntry), 4));
}
std::vector<Chunk *> DelayLoadContents::getChunks() {
@@ -830,17 +837,16 @@ void DelayLoadContents::create(Defined *h) {
saver().save("__tailMerge_" + syms[0]->getDLLName().lower());
ctx.symtab.addSynthetic(tmName, tm);
// Terminate with null values.
- addresses.push_back(make<NullChunk>(8));
- names.push_back(make<NullChunk>(8));
+ addresses.push_back(make<NullChunk>(ctx, 8));
+ names.push_back(make<NullChunk>(ctx, 8));
if (ctx.config.machine == ARM64EC) {
- auxIat.push_back(make<NullChunk>(8));
- auxIatCopy.push_back(make<NullChunk>(8));
+ auxIat.push_back(make<NullChunk>(ctx, 8));
+ auxIatCopy.push_back(make<NullChunk>(ctx, 8));
}
for (int i = 0, e = syms.size(); i < e; ++i)
syms[i]->setLocation(addresses[base + i]);
- auto *mh = make<NullChunk>(8);
- mh->setAlignment(8);
+ auto *mh = make<NullChunk>(8, 8);
moduleHandles.push_back(mh);
// Fill the delay import table header fields.
@@ -853,7 +859,8 @@ void DelayLoadContents::create(Defined *h) {
if (unwind)
unwindinfo.push_back(unwind);
// Add null terminator.
- dirs.push_back(make<NullChunk>(sizeof(delay_import_directory_table_entry)));
+ dirs.push_back(
+ make<NullChunk>(sizeof(delay_import_directory_table_entry), 4));
}
Chunk *DelayLoadContents::newTailMergeChunk(Chunk *dir) {
|
|
@llvm/pr-subscribers-lld Author: Jacek Caban (cjacek) ChangesFull diff: https://github.com/llvm/llvm-project/pull/116677.diff 1 Files Affected:
diff --git a/lld/COFF/DLL.cpp b/lld/COFF/DLL.cpp
index 2d20b094888c7a..797d9f1490253a 100644
--- a/lld/COFF/DLL.cpp
+++ b/lld/COFF/DLL.cpp
@@ -131,7 +131,14 @@ class ImportDirectoryChunk : public NonSectionChunk {
// Contents of this chunk is always null bytes.
class NullChunk : public NonSectionChunk {
public:
- explicit NullChunk(size_t n) : size(n) { hasData = false; }
+ explicit NullChunk(size_t n, uint32_t align) : size(n) {
+ hasData = false;
+ setAlignment(align);
+ }
+ explicit NullChunk(COFFLinkerContext &ctx)
+ : NullChunk(ctx.config.wordsize, ctx.config.wordsize) {}
+ explicit NullChunk(COFFLinkerContext &ctx, size_t n)
+ : NullChunk(n, ctx.config.wordsize) {}
size_t getSize() const override { return size; }
void writeTo(uint8_t *buf) const override {
@@ -737,11 +744,11 @@ void IdataContents::create(COFFLinkerContext &ctx) {
}
}
// Terminate with null values.
- lookups.push_back(make<NullChunk>(ctx.config.wordsize));
- addresses.push_back(make<NullChunk>(ctx.config.wordsize));
+ lookups.push_back(make<NullChunk>(ctx));
+ addresses.push_back(make<NullChunk>(ctx));
if (ctx.config.machine == ARM64EC) {
- auxIat.push_back(make<NullChunk>(ctx.config.wordsize));
- auxIatCopy.push_back(make<NullChunk>(ctx.config.wordsize));
+ auxIat.push_back(make<NullChunk>(ctx));
+ auxIatCopy.push_back(make<NullChunk>(ctx));
}
for (int i = 0, e = syms.size(); i < e; ++i)
@@ -755,7 +762,7 @@ void IdataContents::create(COFFLinkerContext &ctx) {
dirs.push_back(dir);
}
// Add null terminator.
- dirs.push_back(make<NullChunk>(sizeof(ImportDirectoryTableEntry)));
+ dirs.push_back(make<NullChunk>(sizeof(ImportDirectoryTableEntry), 4));
}
std::vector<Chunk *> DelayLoadContents::getChunks() {
@@ -830,17 +837,16 @@ void DelayLoadContents::create(Defined *h) {
saver().save("__tailMerge_" + syms[0]->getDLLName().lower());
ctx.symtab.addSynthetic(tmName, tm);
// Terminate with null values.
- addresses.push_back(make<NullChunk>(8));
- names.push_back(make<NullChunk>(8));
+ addresses.push_back(make<NullChunk>(ctx, 8));
+ names.push_back(make<NullChunk>(ctx, 8));
if (ctx.config.machine == ARM64EC) {
- auxIat.push_back(make<NullChunk>(8));
- auxIatCopy.push_back(make<NullChunk>(8));
+ auxIat.push_back(make<NullChunk>(ctx, 8));
+ auxIatCopy.push_back(make<NullChunk>(ctx, 8));
}
for (int i = 0, e = syms.size(); i < e; ++i)
syms[i]->setLocation(addresses[base + i]);
- auto *mh = make<NullChunk>(8);
- mh->setAlignment(8);
+ auto *mh = make<NullChunk>(8, 8);
moduleHandles.push_back(mh);
// Fill the delay import table header fields.
@@ -853,7 +859,8 @@ void DelayLoadContents::create(Defined *h) {
if (unwind)
unwindinfo.push_back(unwind);
// Add null terminator.
- dirs.push_back(make<NullChunk>(sizeof(delay_import_directory_table_entry)));
+ dirs.push_back(
+ make<NullChunk>(sizeof(delay_import_directory_table_entry), 4));
}
Chunk *DelayLoadContents::newTailMergeChunk(Chunk *dir) {
|
mstorsjo
approved these changes
Nov 18, 2024
|
I updated the commit message and merged. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, null chunks always follow other aligned chunks, so this patch is NFC. However, it will become observable once support for ARM64X imports is added. The import tables are shared between the native and EC views. They are usually very similar, but in cases where they differ, ARM64X relocations handle the discrepancies. If a DLL is only imported by EC code, the native view will see it as importing zero functions from this DLL (with ARM64X relocations replacing those null chunks with actual imports). In this scenario, the null chunks may appear as the very first chunks, meaning there is nothing else forcing their alignment.