-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[llvm-objcopy][COFF] Add aliases for some --subsystem options #98036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
GNU objcopy has some --subsystem options that aren't present in LLVM's. They are in fact just aliases to some of the existing options. For the sake of compatibility with the GNU toolchain, this patch adds these aliases to LLVM objcopy. The alias list is not exhaustive as this is an incremental change.
@llvm/pr-subscribers-llvm-binary-utilities Author: Victor Campos (vhscampos) ChangesGNU objcopy has some --subsystem options that aren't present in LLVM's. They are in fact just aliases to some of the existing options. For the sake of compatibility with the GNU toolchain, this patch adds these aliases to LLVM objcopy. The alias list is not exhaustive as this is an incremental change. Full diff: https://github.com/llvm/llvm-project/pull/98036.diff 2 Files Affected:
diff --git a/llvm/test/tools/llvm-objcopy/COFF/subsystem.test b/llvm/test/tools/llvm-objcopy/COFF/subsystem.test
index 4d73ed83941c7..92d49e80a8d00 100644
--- a/llvm/test/tools/llvm-objcopy/COFF/subsystem.test
+++ b/llvm/test/tools/llvm-objcopy/COFF/subsystem.test
@@ -30,6 +30,15 @@
# INVALID-MAJOR-NUMBER: 'foo' is not a valid subsystem major version
# INVALID-MINOR-NUMBER: 'bar' is not a valid subsystem minor version
+# RUN: llvm-objcopy --subsystem=efi_application %t.in.exe
+# RUN: llvm-objcopy --subsystem=efi-app %t.in.exe
+
+# RUN: llvm-objcopy --subsystem=efi_boot_service_driver %t.in.exe
+# RUN: llvm-objcopy --subsystem=efi-bsd %t.in.exe
+
+# RUN: llvm-objcopy --subsystem=efi_runtime_driver %t.in.exe
+# RUN: llvm-objcopy --subsystem=efi-rtd %t.in.exe
+
--- !COFF
OptionalHeader:
AddressOfEntryPoint: 4096
diff --git a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
index 4ab3b7265f2f6..7e7ed2a23889c 100644
--- a/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
+++ b/llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
@@ -678,12 +678,13 @@ objcopy::parseObjcopyOptions(ArrayRef<const char *> RawArgsArr,
.Case("boot_application",
COFF::IMAGE_SUBSYSTEM_WINDOWS_BOOT_APPLICATION)
.Case("console", COFF::IMAGE_SUBSYSTEM_WINDOWS_CUI)
- .Case("efi_application", COFF::IMAGE_SUBSYSTEM_EFI_APPLICATION)
- .Case("efi_boot_service_driver",
- COFF::IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER)
+ .Cases("efi_application", "efi-app",
+ COFF::IMAGE_SUBSYSTEM_EFI_APPLICATION)
+ .Cases("efi_boot_service_driver", "efi-bsd",
+ COFF::IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER)
.Case("efi_rom", COFF::IMAGE_SUBSYSTEM_EFI_ROM)
- .Case("efi_runtime_driver",
- COFF::IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER)
+ .Cases("efi_runtime_driver", "efi-rtd",
+ COFF::IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER)
.Case("native", COFF::IMAGE_SUBSYSTEM_NATIVE)
.Case("posix", COFF::IMAGE_SUBSYSTEM_POSIX_CUI)
.Case("windows", COFF::IMAGE_SUBSYSTEM_WINDOWS_GUI)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test case isn't really doing anything useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM, with some suggestions for the commit title.
I suggest adding the [COFF]
tag to the PR title before landing. Also, the tag should be [llvm-objcopy]
, since simply objcopy
tends to refer to the GNU tool. Finally, to shorten the title, you can drop the "llvm-objcopy" bit after "some", since it is made redundant by the tags:
[llvm-objcopy][COFF] Add aliases for some --subsystem options
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/1224 Here is the relevant piece of the build log for the reference:
|
…8036) GNU objcopy has some --subsystem options that aren't present in LLVM's. They are in fact just aliases to some of the existing options. For the sake of compatibility with the GNU toolchain, this patch adds these aliases to LLVM objcopy. The alias list is not exhaustive as this is an incremental change.
GNU objcopy has some --subsystem options that aren't present in LLVM's. They are in fact just aliases to some of the existing options.
For the sake of compatibility with the GNU toolchain, this patch adds these aliases to LLVM objcopy.
The alias list is not exhaustive as this is an incremental change.