[LLD][COFF] Add support for the -defArm64Native argument #123850
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
|
Depends on #123849. |
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-coff Author: Jacek Caban (cjacek) ChangesMSVC ignores the Full diff: https://github.com/llvm/llvm-project/pull/123850.diff 6 Files Affected:
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 4e0678282eed01..6cffc530bc4702 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -30,7 +30,6 @@
#include "llvm/LTO/LTO.h"
#include "llvm/Object/ArchiveWriter.h"
#include "llvm/Object/COFFImportFile.h"
-#include "llvm/Object/COFFModuleDefinition.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Option/Option.h"
@@ -1012,67 +1011,6 @@ void LinkerDriver::createImportLibrary(bool asLib) {
}
}
-void LinkerDriver::parseModuleDefs(StringRef path) {
- llvm::TimeTraceScope timeScope("Parse def file");
- std::unique_ptr<MemoryBuffer> mb =
- CHECK(MemoryBuffer::getFile(path, /*IsText=*/false,
- /*RequiresNullTerminator=*/false,
- /*IsVolatile=*/true),
- "could not open " + path);
- COFFModuleDefinition m = check(parseCOFFModuleDefinition(
- mb->getMemBufferRef(), ctx.config.machine, ctx.config.mingw));
-
- // Include in /reproduce: output if applicable.
- ctx.driver.takeBuffer(std::move(mb));
-
- if (ctx.config.outputFile.empty())
- ctx.config.outputFile = std::string(saver().save(m.OutputFile));
- ctx.config.importName = std::string(saver().save(m.ImportName));
- if (m.ImageBase)
- ctx.config.imageBase = m.ImageBase;
- if (m.StackReserve)
- ctx.config.stackReserve = m.StackReserve;
- if (m.StackCommit)
- ctx.config.stackCommit = m.StackCommit;
- if (m.HeapReserve)
- ctx.config.heapReserve = m.HeapReserve;
- if (m.HeapCommit)
- ctx.config.heapCommit = m.HeapCommit;
- if (m.MajorImageVersion)
- ctx.config.majorImageVersion = m.MajorImageVersion;
- if (m.MinorImageVersion)
- ctx.config.minorImageVersion = m.MinorImageVersion;
- if (m.MajorOSVersion)
- ctx.config.majorOSVersion = m.MajorOSVersion;
- if (m.MinorOSVersion)
- ctx.config.minorOSVersion = m.MinorOSVersion;
-
- for (COFFShortExport e1 : m.Exports) {
- Export e2;
- // Renamed exports are parsed and set as "ExtName = Name". If Name has
- // the form "OtherDll.Func", it shouldn't be a normal exported
- // function but a forward to another DLL instead. This is supported
- // by both MS and GNU linkers.
- if (!e1.ExtName.empty() && e1.ExtName != e1.Name &&
- StringRef(e1.Name).contains('.')) {
- e2.name = saver().save(e1.ExtName);
- e2.forwardTo = saver().save(e1.Name);
- } else {
- e2.name = saver().save(e1.Name);
- e2.extName = saver().save(e1.ExtName);
- }
- e2.exportAs = saver().save(e1.ExportAs);
- e2.importName = saver().save(e1.ImportName);
- e2.ordinal = e1.Ordinal;
- e2.noname = e1.Noname;
- e2.data = e1.Data;
- e2.isPrivate = e1.Private;
- e2.constant = e1.Constant;
- e2.source = ExportSource::ModuleDefinition;
- ctx.symtab.exports.push_back(e2);
- }
-}
-
void LinkerDriver::enqueueTask(std::function<void()> task) {
taskQueue.push_back(std::move(task));
}
@@ -2352,7 +2290,13 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
// Handle /def
if (auto *arg = args.getLastArg(OPT_deffile)) {
// parseModuleDefs mutates Config object.
- parseModuleDefs(arg->getValue());
+ mainSymtab.parseModuleDefs(arg->getValue());
+ if (ctx.hybridSymtab) {
+ // MSVC ignores the /defArm64Native argument on non-ARM64X targets.
+ // It is also ignored if the /def option is not specified.
+ if (auto *arg = args.getLastArg(OPT_defarm64native))
+ ctx.symtab.parseModuleDefs(arg->getValue());
+ }
}
// Handle generation of import library from a def file.
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 12724cbd1eef49..58dc5458e9a544 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -143,8 +143,6 @@ class LinkerDriver {
// Used by the resolver to parse .drectve section contents.
void parseDirectives(InputFile *file);
- void parseModuleDefs(StringRef path);
-
// Parse an /order file. If an option is given, the linker places COMDAT
// sections int he same order as their names appear in the given file.
void parseOrderFile(StringRef arg);
diff --git a/lld/COFF/Options.td b/lld/COFF/Options.td
index c7ceb51f70b70a..b6fd3d0daaef99 100644
--- a/lld/COFF/Options.td
+++ b/lld/COFF/Options.td
@@ -140,6 +140,9 @@ def incl : Joined<["/", "-", "/?", "-?"], "include:">,
def deffile : Joined<["/", "-", "/?", "-?"], "def:">,
HelpText<"Use module-definition file">;
+def defarm64native
+ : P<"defarm64native",
+ "Use a module-definition file for the native view in a hybrid image.">;
def debug : F<"debug">, HelpText<"Embed a symbol table in the image">;
def debug_opt : P<"debug", "Embed a symbol table in the image with option">;
def debugtype : P<"debugtype", "Debug Info Options">;
diff --git a/lld/COFF/SymbolTable.cpp b/lld/COFF/SymbolTable.cpp
index ecccc7d6ed70c7..32ea4a5b2e1fc3 100644
--- a/lld/COFF/SymbolTable.cpp
+++ b/lld/COFF/SymbolTable.cpp
@@ -20,6 +20,7 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/IR/Mangler.h"
#include "llvm/LTO/LTO.h"
+#include "llvm/Object/COFFModuleDefinition.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/GlobPattern.h"
#include "llvm/Support/Parallel.h"
@@ -29,6 +30,7 @@
using namespace llvm;
using namespace llvm::COFF;
+using namespace llvm::object;
using namespace llvm::support;
namespace lld::coff {
@@ -1253,6 +1255,67 @@ void SymbolTable::assignExportOrdinals() {
<< Twine(std::numeric_limits<uint16_t>::max()) << ")";
}
+void SymbolTable::parseModuleDefs(StringRef path) {
+ llvm::TimeTraceScope timeScope("Parse def file");
+ std::unique_ptr<MemoryBuffer> mb =
+ CHECK(MemoryBuffer::getFile(path, /*IsText=*/false,
+ /*RequiresNullTerminator=*/false,
+ /*IsVolatile=*/true),
+ "could not open " + path);
+ COFFModuleDefinition m = check(parseCOFFModuleDefinition(
+ mb->getMemBufferRef(), machine, ctx.config.mingw));
+
+ // Include in /reproduce: output if applicable.
+ ctx.driver.takeBuffer(std::move(mb));
+
+ if (ctx.config.outputFile.empty())
+ ctx.config.outputFile = std::string(saver().save(m.OutputFile));
+ ctx.config.importName = std::string(saver().save(m.ImportName));
+ if (m.ImageBase)
+ ctx.config.imageBase = m.ImageBase;
+ if (m.StackReserve)
+ ctx.config.stackReserve = m.StackReserve;
+ if (m.StackCommit)
+ ctx.config.stackCommit = m.StackCommit;
+ if (m.HeapReserve)
+ ctx.config.heapReserve = m.HeapReserve;
+ if (m.HeapCommit)
+ ctx.config.heapCommit = m.HeapCommit;
+ if (m.MajorImageVersion)
+ ctx.config.majorImageVersion = m.MajorImageVersion;
+ if (m.MinorImageVersion)
+ ctx.config.minorImageVersion = m.MinorImageVersion;
+ if (m.MajorOSVersion)
+ ctx.config.majorOSVersion = m.MajorOSVersion;
+ if (m.MinorOSVersion)
+ ctx.config.minorOSVersion = m.MinorOSVersion;
+
+ for (COFFShortExport e1 : m.Exports) {
+ Export e2;
+ // Renamed exports are parsed and set as "ExtName = Name". If Name has
+ // the form "OtherDll.Func", it shouldn't be a normal exported
+ // function but a forward to another DLL instead. This is supported
+ // by both MS and GNU linkers.
+ if (!e1.ExtName.empty() && e1.ExtName != e1.Name &&
+ StringRef(e1.Name).contains('.')) {
+ e2.name = saver().save(e1.ExtName);
+ e2.forwardTo = saver().save(e1.Name);
+ } else {
+ e2.name = saver().save(e1.Name);
+ e2.extName = saver().save(e1.ExtName);
+ }
+ e2.exportAs = saver().save(e1.ExportAs);
+ e2.importName = saver().save(e1.ImportName);
+ e2.ordinal = e1.Ordinal;
+ e2.noname = e1.Noname;
+ e2.data = e1.Data;
+ e2.isPrivate = e1.Private;
+ e2.constant = e1.Constant;
+ e2.source = ExportSource::ModuleDefinition;
+ exports.push_back(e2);
+ }
+}
+
Symbol *SymbolTable::addUndefined(StringRef name) {
return addUndefined(name, nullptr, false);
}
diff --git a/lld/COFF/SymbolTable.h b/lld/COFF/SymbolTable.h
index e5b02ce5904c49..c8d7251838842f 100644
--- a/lld/COFF/SymbolTable.h
+++ b/lld/COFF/SymbolTable.h
@@ -160,6 +160,7 @@ class SymbolTable {
void fixupExports();
void assignExportOrdinals();
+ void parseModuleDefs(StringRef path);
// Iterates symbols in non-determinstic hash table order.
template <typename T> void forEachSymbol(T callback) {
diff --git a/lld/test/COFF/arm64x-export.test b/lld/test/COFF/arm64x-export.test
index 526be633973581..ea7a5f411707e0 100644
--- a/lld/test/COFF/arm64x-export.test
+++ b/lld/test/COFF/arm64x-export.test
@@ -55,6 +55,13 @@ RUN: loadconfig-arm64.obj loadconfig-arm64ec.obj arm64ec-drectve.obj -n
RUN: llvm-objdump -d out-drectve-ec.dll | FileCheck --check-prefix=DISASM-EC %s
RUN: llvm-readobj --headers --coff-exports out-drectve-ec.dll | FileCheck --check-prefix=EXPORTS-EC %s
+# A command-line def file applies only to EC exports.
+
+RUN: lld-link -machine:arm64x -dll -out:out-def-ec.dll arm64ec-func.obj arm64-func.obj \
+RUN: loadconfig-arm64.obj loadconfig-arm64ec.obj -def:func.def -noentry
+RUN: llvm-objdump -d out-def-ec.dll | FileCheck --check-prefix=DISASM-EC %s
+RUN: llvm-readobj --headers --coff-exports out-def-ec.dll | FileCheck --check-prefix=EXPORTS-EC %s
+
# Export using the EC .edata section.
RUN: lld-link -machine:arm64x -dll -out:out-edata-ec.dll arm64ec-func.obj arm64-func.obj \
@@ -155,6 +162,29 @@ EXPORTS-BOTH-NEXT: RVA: 0x3000
EXPORTS-BOTH-NEXT: }
EXPORTS-BOTH-NEXT: }
+# Export using both the -def and -defarm64native arguments.
+
+RUN: lld-link -machine:arm64x -dll -out:out-def-both.dll arm64ec-func.obj arm64-func.obj \
+RUN: loadconfig-arm64.obj loadconfig-arm64ec.obj -def:func.def -defarm64native:func.def -noentry
+RUN: llvm-objdump -d out-def-both.dll | FileCheck --check-prefix=DISASM-BOTH %s
+RUN: llvm-readobj --headers --coff-exports out-def-both.dll | FileCheck --check-prefix=EXPORTS-BOTH %s
+
+# -defarm64native is ignored if -def is not specified.
+
+RUN: lld-link -machine:arm64x -dll -out:out-def-native.dll arm64ec-func.obj arm64-func.obj \
+RUN: loadconfig-arm64.obj loadconfig-arm64ec.obj -defarm64native:func.def -noentry
+RUN: llvm-readobj --headers --coff-exports out-def-native.dll | FileCheck --check-prefix=NO-EXPORT %s
+NO-EXPORT: ExportTableRVA: 0x0
+NO-EXPORT: ExportTableSize: 0x0
+NO-EXPORT: HybridObject {
+NO-EXPORT: ExportTableRVA: 0x0
+NO-EXPORT: ExportTableSize: 0x0
+NO-EXPORT: }
+
+# -defarm64native is ignored on ARM64 target.
+
+RUN: lld-link -machine:arm64 -dll -out:out-arm64-def.dll arm64-func.obj -defarm64native:undef.def -def:func.def -noentry
+
# Export using both the native and EC .edata sections.
RUN: lld-link -machine:arm64x -dll -out:out-edata-both.dll arm64ec-func.obj arm64-func.obj \
@@ -227,3 +257,8 @@ funcname_func:
name:
.asciz "out-edata.dll"
+
+#--- func.def
+LIBRARY out.dll
+EXPORTS
+ func
|
mstorsjo
approved these changes
Jan 22, 2025
| // MSVC ignores the /defArm64Native argument on non-ARM64X targets. | ||
| // It is also ignored if the /def option is not specified. | ||
| if (auto *arg = args.getLastArg(OPT_defarm64native)) | ||
| ctx.symtab.parseModuleDefs(arg->getValue()); |
There was a problem hiding this comment.
Side note: The naming of the variables constantly feels unintuitive to me here, where mainSymtab is the EC symbol table, while ctx.symtab (which similarly feels like the "main" one) is the native one.
We can't probably do much about it, because this is how the linker is supposed to be have, if mimicing link.exe, but we perhaps make it clearer. I think it would be good to have a comment on the definition of mainSymtab to really spell out what it is, and why.
There was a problem hiding this comment.
Yes, which symbol table is considered 'main' or 'primary' depends on the context. In the writer, the main one is ctx.symtab, as it’s directly referenced by the PE headers. Here, for lack of a better term, I meant the symbol table to which the main arguments should apply (though not all do yet). I'll add a comment for clarification.
lld/test/COFF/arm64x-export.test
Outdated
|
|
||
| # -defarm64native is ignored on ARM64 target. | ||
|
|
||
| RUN: lld-link -machine:arm64 -dll -out:out-arm64-def.dll arm64-func.obj -defarm64native:undef.def -def:func.def -noentry |
There was a problem hiding this comment.
This test doesn't really verify that it is ignored; it only says that linking does succeed - but it doesn't verify e.g. that we don't print any warning. We can test that, without needing to specify any explicit -NOT tests, by doing 2>&1 | count 0 on the run line.
There was a problem hiding this comment.
I pass an invalid file there, so if it were interpreted, it would error out due to the missing file. I'll add an additional check for warnings, thanks.
MSVC ignores the /defArm64Native argument on non-ARM64X targets. It is also ignored if the /def option is not specified.
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.
MSVC ignores the
/defArm64Nativeargument on non-ARM64X targets.It is also ignored if the
/defoption is not specified.