-
Notifications
You must be signed in to change notification settings - Fork 14.5k
Add necessary linker flags when -static-pie is enabled in BareMetal Toolchain #147589
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang Author: Garvit Gupta (quic-garvgupt) ChangesFull diff: https://github.com/llvm/llvm-project/pull/147589.diff 9 Files Affected:
diff --git a/clang/include/clang/Driver/CommonArgs.h b/clang/include/clang/Driver/CommonArgs.h
index 26aa3ccf84786..d8877903c892f 100644
--- a/clang/include/clang/Driver/CommonArgs.h
+++ b/clang/include/clang/Driver/CommonArgs.h
@@ -85,6 +85,8 @@ const char *RelocationModelName(llvm::Reloc::Model Model);
std::tuple<llvm::Reloc::Model, unsigned, bool>
ParsePICArgs(const ToolChain &ToolChain, const llvm::opt::ArgList &Args);
+bool getStaticPIE(const llvm::opt::ArgList &Args, const ToolChain &TC);
+
unsigned ParseFunctionAlignment(const ToolChain &TC,
const llvm::opt::ArgList &Args);
diff --git a/clang/lib/Driver/ToolChains/BareMetal.cpp b/clang/lib/Driver/ToolChains/BareMetal.cpp
index e670696cd59ae..6ee3fbe400566 100644
--- a/clang/lib/Driver/ToolChains/BareMetal.cpp
+++ b/clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -599,11 +599,18 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const Driver &D = getToolChain().getDriver();
const llvm::Triple::ArchType Arch = TC.getArch();
const llvm::Triple &Triple = getToolChain().getEffectiveTriple();
+ const bool IsStaticPIE = getStaticPIE(Args, TC);
if (!D.SysRoot.empty())
CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot));
CmdArgs.push_back("-Bstatic");
+ if(IsStaticPIE) {
+ CmdArgs.push_back("-pie");
+ CmdArgs.push_back("--no-dynamic-linker");
+ CmdArgs.push_back("-z");
+ CmdArgs.push_back("text");
+ }
if (const char *LDMOption = getLDMOption(TC.getTriple(), Args)) {
CmdArgs.push_back("-m");
@@ -633,14 +640,18 @@ void baremetal::Linker::ConstructJob(Compilation &C, const JobAction &JA,
const char *CRTBegin, *CRTEnd;
if (NeedCRTs) {
- if (!Args.hasArg(options::OPT_r))
- CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath("crt0.o")));
+ if (!Args.hasArg(options::OPT_r)) {
+ const char *crt = "crt0.o";
+ if (IsStaticPIE)
+ crt = "rcrt1.o";
+ CmdArgs.push_back(Args.MakeArgString(TC.GetFilePath(crt)));
+ }
if (TC.hasValidGCCInstallation() || detectGCCToolchainAdjacent(D)) {
auto RuntimeLib = TC.GetRuntimeLibType(Args);
switch (RuntimeLib) {
case (ToolChain::RLT_Libgcc): {
- CRTBegin = "crtbegin.o";
- CRTEnd = "crtend.o";
+ CRTBegin = IsStaticPIE ? "crtbeginS.o" : "crtbegin.o";
+ CRTEnd = IsStaticPIE ? "crtendS.o" : "crtend.o";
break;
}
case (ToolChain::RLT_CompilerRT): {
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index bdd77ac84913c..f8f97b02a5f95 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -2089,6 +2089,18 @@ tools::ParsePICArgs(const ToolChain &ToolChain, const ArgList &Args) {
return std::make_tuple(RelocM, 0U, false);
}
+bool tools::getStaticPIE(const ArgList &Args, const ToolChain &TC) {
+ bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
+ if (HasStaticPIE && Args.hasArg(options::OPT_no_pie)) {
+ const Driver &D = TC.getDriver();
+ const llvm::opt::OptTable &Opts = D.getOpts();
+ StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+ StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
+ D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
+ }
+ return HasStaticPIE;
+}
+
// `-falign-functions` indicates that the functions should be aligned to the
// backend's preferred alignment.
//
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f5e2655857432..01b146db24f3e 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -219,18 +219,6 @@ void tools::gcc::Linker::RenderExtraToolArgs(const JobAction &JA,
// The types are (hopefully) good enough.
}
-static bool getStaticPIE(const ArgList &Args, const ToolChain &TC) {
- bool HasStaticPIE = Args.hasArg(options::OPT_static_pie);
- if (HasStaticPIE && Args.hasArg(options::OPT_no_pie)) {
- const Driver &D = TC.getDriver();
- const llvm::opt::OptTable &Opts = D.getOpts();
- StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
- StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
- D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
- }
- return HasStaticPIE;
-}
-
static bool getStatic(const ArgList &Args) {
return Args.hasArg(options::OPT_static) &&
!Args.hasArg(options::OPT_static_pie);
diff --git a/clang/test/Driver/aarch64-toolchain.c b/clang/test/Driver/aarch64-toolchain.c
index cfad4b8eb6829..4dc970f0e8685 100644
--- a/clang/test/Driver/aarch64-toolchain.c
+++ b/clang/test/Driver/aarch64-toolchain.c
@@ -157,3 +157,15 @@
// AARCH64-BAREMETAL-UNWINDLIB: "{{.*}}clang_rt.crtbegin.o"
// AARCH64-BAREMETAL-UNWINDLIB: "--start-group" "{{.*}}libclang_rt.builtins{{.*}}.a" "--as-needed" "-lunwind" "--no-as-needed" "-lc" "-lgloss" "--end-group"
// AARCH64-BAREMETAL-UNWINDLIB: "{{.*}}clang_rt.crtend.o"
+
+// RUN: %clang -static-pie -### %s -fuse-ld= \
+// RUN: --target=aarch64-none-elf --rtlib=libgcc --unwindlib=platform \
+// RUN: --gcc-toolchain=%S/Inputs/basic_aarch64_gcc_tree \
+// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/aarch64-none-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=C-ARM-STATIC-PIE %s
+
+// C-ARM-STATIC-PIE: "-Bstatic" "-pie" "--no-dynamic-linker" "-z" "text" "-m" "aarch64linux" "-EL"
+// C-ARM-STATIC-PIE: "{{.*}}rcrt1.o"
+// C-ARM-STATIC-PIE: "{{.*}}crtbeginS.o"
+// C-ARM-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "-lgloss" "--end-group"
+// C-ARM-STATIC-PIE: "{{.*}}crtendS.o"
diff --git a/clang/test/Driver/arm-toolchain.c b/clang/test/Driver/arm-toolchain.c
index c367594b0a758..f164fee723e25 100644
--- a/clang/test/Driver/arm-toolchain.c
+++ b/clang/test/Driver/arm-toolchain.c
@@ -158,3 +158,15 @@
// ARM-BAREMETAL-UNWINDLIB: "{{.*}}clang_rt.crtbegin.o"
// ARM-BAREMETAL-UNWINDLIB: "--start-group" "{{.*}}libclang_rt.builtins.a" "--as-needed" "-lunwind" "--no-as-needed" "-lc" "-lgloss" "--end-group"
// ARM-BAREMETAL-UNWINDLIB: "{{.*}}clang_rt.crtend.o"
+
+// RUN: %clang -static-pie -### %s -fuse-ld= \
+// RUN: --target=armv6m-none-eabi --rtlib=libgcc --unwindlib=platform \
+// RUN: --gcc-toolchain=%S/Inputs/basic_arm_gcc_tree \
+// RUN: --sysroot=%S/Inputs/basic_arm_gcc_tree/armv6m-none-eabi 2>&1 \
+// RUN: | FileCheck -check-prefix=C-ARM-STATIC-PIE %s
+
+// C-ARM-STATIC-PIE: "-Bstatic" "-pie" "--no-dynamic-linker" "-z" "text" "-m" "armelf_linux_eabi" "-EL"
+// C-ARM-STATIC-PIE: "{{.*}}rcrt1.o"
+// C-ARM-STATIC-PIE: "{{.*}}crtbeginS.o"
+// C-ARM-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "-lgloss" "--end-group"
+// C-ARM-STATIC-PIE: "{{.*}}crtendS.o"
diff --git a/clang/test/Driver/baremetal.cpp b/clang/test/Driver/baremetal.cpp
index 4dc320191317e..1e86432bb0d43 100644
--- a/clang/test/Driver/baremetal.cpp
+++ b/clang/test/Driver/baremetal.cpp
@@ -591,3 +591,29 @@
// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf \
// RUN: | FileCheck --check-prefix=CHECK-RV64-RELAX %s
// CHECK-RV64-RELAX-NOT: "--no-relax"
+
+// Check that "-static -pie" is forwarded to linker when "-static-pie" is used
+
+// RUN: %clang -static-pie -### %s 2>&1 \
+// RUN: --target=armv6m-none-eabi -rtlib=platform --unwindlib=platform \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+
+// RUN: %clang -static-pie -### %s 2>&1 \
+// RUN: --target=aarch64-none-elf -rtlib=platform --unwindlib=platform \
+// RUN: --sysroot=%S/Inputs/baremetal_arm \
+// RUN: | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+
+// RUN: %clang -static-pie -### %s 2>&1 \
+// RUN: --target=riscv32-unknown-elf-rtlib=platform --unwindlib=platform \
+// RUN: --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf \
+// RUN: | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+
+// RUN: %clang -static-pie -### %s 2>&1 \
+// RUN: --target=riscv64-unknown-elf-rtlib=platform --unwindlib=platform \
+// RUN: --sysroot=%S/Inputs/basic_riscv32_tree/riscv64-unknown-elf \
+// RUN: | FileCheck --check-prefix=CHECK-CLANG-LD-STATIC-PIE %s
+
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-STATIC-PIE-SAME: "-static" "-pie" "--no-dynamic-linker" "-z" "text"
+// CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
\ No newline at end of file
diff --git a/clang/test/Driver/riscv32-toolchain.c b/clang/test/Driver/riscv32-toolchain.c
index 8cf20aa592a3a..f54e6cf8a6c03 100644
--- a/clang/test/Driver/riscv32-toolchain.c
+++ b/clang/test/Driver/riscv32-toolchain.c
@@ -247,6 +247,19 @@
// RUN: | FileCheck -check-prefix=CHECK-RV32-GNU-RELAX %s
// CHECK-RV32-GNU-RELAX-NOT: "--no-relax"
+// Check that "-static -pie" is forwarded to linker when "-static-pie" is used
+// RUN: %clang -static-pie -### %s -fuse-ld= \
+// RUN: --target=riscv32-unknown-elf -rtlib=platform --unwindlib=platform \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV32-STATIC-PIE %s
+
+// C-RV32-STATIC-PIE: "-Bstatic" "-pie" "--no-dynamic-linker" "-z" "text" "-m" "elf32lriscv" "-X"
+// C-RV32-STATIC-PIE: "{{.*}}rcrt1.o"
+// C-RV32-STATIC-PIE: "{{.*}}crtbeginS.o"
+// C-RV32-STATIC-PIE: "--start-group" "-lgcc" "-lc" "-lgloss" "--end-group"
+// C-RV32-STATIC-PIE: "{{.*}}crtendS.o"
+
typedef __builtin_va_list va_list;
typedef __SIZE_TYPE__ size_t;
typedef __PTRDIFF_TYPE__ ptrdiff_t;
diff --git a/clang/test/Driver/riscv64-toolchain.c b/clang/test/Driver/riscv64-toolchain.c
index 1550f46af8c9c..c3f54bf444ee2 100644
--- a/clang/test/Driver/riscv64-toolchain.c
+++ b/clang/test/Driver/riscv64-toolchain.c
@@ -203,6 +203,19 @@
// RUN: | FileCheck -check-prefix=CHECK-RV64-GNU-RELAX %s
// CHECK-RV64-GNU-RELAX-NOT: "--no-relax"
+// Check that "-static -pie" is forwarded to linker when "-static-pie" is used
+// RUN: %clang -static-pie -### %s -fuse-ld= \
+// RUN: --target=riscv64-unknown-elf -rtlib=platform --unwindlib=platform \
+// RUN: --gcc-toolchain=%S/Inputs/basic_riscv64_tree \
+// RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
+// RUN: | FileCheck -check-prefix=C-RV64-STATIC-PIE %s
+
+// C-RV64-STATIC-PIE: "-Bstatic" "-pie" "--no-dynamic-linker" "-z" "text" "-m" "elf64lriscv" "-X"
+// C-RV64-STATIC-PIE: "{{.*}}rcrt1.o"
+// C-RV64-STATIC-PIE: "{{.*}}crtbeginS.o"
+// C-RV64-STATIC-PIE: "--start-group" "-lgcc" "-lc" "-lgloss" "--end-group"
+// C-RV64-STATIC-PIE: "{{.*}}crtendS.o"
+
typedef __builtin_va_list va_list;
typedef __SIZE_TYPE__ size_t;
typedef __PTRDIFF_TYPE__ ptrdiff_t;
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
7f9e12f
to
a1f3797
Compare
toolchain Change-Id: I580875585e9eac2e9568e84650265f71d028f3ff
|
||
if (!D.SysRoot.empty()) | ||
CmdArgs.push_back(Args.MakeArgString("--sysroot=" + D.SysRoot)); | ||
|
||
CmdArgs.push_back("-Bstatic"); | ||
if (IsStaticPIE) { | ||
CmdArgs.push_back("-pie"); | ||
CmdArgs.push_back("--no-dynamic-linker"); |
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.
This is already the default for LLD, is this needed for other linkers like GNU ld?
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.
This change is needed for both LLD and GNU LD. I dont see static -pie
flags being passed by default for LLD if -static-pie
is passed to clang driver. Moreover, without this change we get a unused argument warning as well.
Pls see below for verbose logs.
clang --target=aarch64-none-elf -fuse-ld=lld -static-pie empty.c -###
clang: warning: argument unused during compilation: '-static-pie' [-Wunused-command-line-argument]
...............................
ld.lld" "-Bstatic" "-m" "aarch64linux" "-EL" "crt0.o" "-L<path-to-bin>/../lib/clang-runtimes/aarch64-none-elf/lib" "-L<path>lib/clang/21/lib/aarch64-unknown-none-elf" "-L<path-to-bin>/../lib/clang-runtimes/aarch64-none-elf/lib" "/tmp/empty-09660d.o" "--start-group" "<path>/lib/clang/21/lib/aarch64-unknown-none-elf/libclang_rt.builtins.a" "-lc" "--end-group" "-o" "a.out"
Pls let me know if I missed something in your comment or interpreted it differently.
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.
I think @petrhosek was asking specifically about the "--no-dynamic-linker" flag.
I think for gnu ld it is necessary as it's possible to mix -static and -shared according to the manpage. I presume this ensures that you get an error in that case rather than a statically linked shared library.
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.
think @petrhosek was asking specifically about the "--no-dynamic-linker" flag.
Thanks for clarifying this.
I think for gnu ld it is necessary as it's possible to mix -static and -shared according to the manpage. I presume this ensures that you get an error in that case rather than a statically linked shared library.
Yes, if it’s the default behavior for LLD, then it would still be necessary for GNU LD. When both -static
and -shared
are used i.e., when creating a statically linked shared library, all definitions within the library are fully resolved at link time and pulled from static libraries. In such cases, there is no need for a dynamic linker.
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.
Do we know what -static-pie means in the context of the bare-metal driver? It maybe that the RISCV GNU toolchains do, but this does not look like it is universal [1].
I don't think that this should necessarily prevent this patch from landing. To some degree users can just not use the option if the toolchain doesn't support it. In an ideal world we'd have a warning that it wasn't supported, but I don't think that it is going to be easy to work that out in the compiler driver alone.
As an aside on -static-pie
As I understand it, -static-pie at least for Linux like platforms creates an ELF file with a dynamic section and .rela.dyn and the glibc startup code includes _dl_relocate_static_pie
that reads the dynamic section to find the dynamic relocations: https://codebrowser.dev/glibc/glibc/elf/dl-reloc-static-pie.c.html
This isn't going to work well with most bare-metal platforms as they may not have an ELF file and the dynamic section would likely not be needed. We'd probably want the .rela.dyn to be delimited by linker defined symbols so that a relocation resolver could find and resolve them. Arm's proprietary toolchain did that with its --baremetal-pie option [2].
I'm interested in what the RISCV bare-metal toolchains do in this case?
[1] Some of the bits in this patch definitely won't work for the arm-none-eabi and aarch64-none-elf GNU toolchains as there is no rcvrt1.o, crtbeginS.o and crtendS.o provided.
[2] https://developer.arm.com/documentation/100748/0624/Mapping-Code-and-Data-to-the-Target/Bare-metal-Position-Independent-Executables
The file names for crt files are borrowed from their linux equivalent. However as already pointed out, if a user wants to link against different set of
In such scenarios, the responsibility for generating startup code with an appropriate relocation resolver is delegated to customer image vendors. They are expected to use options like -nostartfiles or -nostdlib, and provide their own startup routines that include an implementation of a custom relocation resolver. Additionally, they must define linker script symbols to delimit the dynamic relocations. The custom resolver then parses each .rela.dyn relocation entry within the boundaries defined by these symbols. |
Thanks for the information. That sounds like how I'd expect it to work in bare-metal platform. Although out of scope of this patch, I've often thought if there were enough common ground to be able to standardise on the symbols and perhaps have the linker define them, with the runtime able to provide a generic relocation resolver based on them. One land-mine I've seen some people run into with LLD, is that it is --no-apply-dynamic-relocs by default. This means that if --pie is used then there must be a loader. However with --apply-dynamic-relocs no loader is needed if the program is run at its static link address. |
No description provided.