[Clang][Driver] Expose relocation model as multilib flags #149132
Conversation
If a multilib collection contains libraries built for different methods of accessing global data (via absolute address, or via a GOT in -fPIC style, or as an offset from a fixed register in Arm -frwpi style), then `multilib.yaml` will need to know which relocation model an application is using in order to select the right library. Even if a multilib collection only supports one relocation model, it's still useful for `multilib.yaml` to be able to tell if the user has selected the right one, so as to give a useful error message if they haven't, instead of silently selecting a library that won't work. In this commit we determine the PIC / ROPI / RWPI status using the existing logic in `ParsePICArgs`, and translate it back into a canonical set of multilib selection flags.
statham-arm
requested review from
compnerd,
zygoloid,
vhscampos,
jroelofs and
smithp35
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Simon Tatham (statham-arm) ChangesIf a multilib collection contains libraries built for different methods of accessing global data (via absolute address, or via a GOT in -fPIC style, or as an offset from a fixed register in Arm -frwpi style), then Even if a multilib collection only supports one relocation model, it's still useful for In this commit we determine the PIC / ROPI / RWPI status using the existing logic in Full diff: https://github.com/llvm/llvm-project/pull/149132.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..5738c0aa9b1d2 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -343,6 +343,7 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
std::vector<std::string> Result;
const llvm::Triple Triple(ComputeEffectiveClangTriple(Args));
Result.push_back("--target=" + Triple.str());
+ bool IsARM = false;
switch (Triple.getArch()) {
case llvm::Triple::aarch64:
@@ -355,6 +356,7 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
getARMMultilibFlags(D, Triple, Args, Result);
+ IsARM = true; // for ROPI/RWPI below
break;
case llvm::Triple::riscv32:
case llvm::Triple::riscv64:
@@ -376,6 +378,41 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
else
Result.push_back("-fexceptions");
+ // A difference of relocation model (absolutely addressed data, PIC, Arm
+ // ROPI/RWPI) is likely to change whether a particular multilib variant is
+ // compatible with a given link. Determine the relocation model of the
+ // current link, and add appropriate
+ {
+ RegisterEffectiveTriple TripleRAII(
+ *this, llvm::Triple(ComputeEffectiveClangTriple(Args)));
+
+ auto [RelocationModel, PICLevel, IsPIE] = tools::ParsePICArgs(*this, Args);
+
+ // ROPI and RWPI are only meaningful on Arm, so for other architectures, we
+ // expect never to find out they're enabled. But it seems confusing to add
+ // -fno-ropi and -fno-rwpi unconditionally to every other architecture's
+ // multilib flags, so instead we leave them out completely.
+ if (IsARM) {
+ if (RelocationModel == llvm::Reloc::ROPI ||
+ RelocationModel == llvm::Reloc::ROPI_RWPI)
+ Result.push_back("-fropi");
+ else
+ Result.push_back("-fno-ropi");
+
+ if (RelocationModel == llvm::Reloc::RWPI ||
+ RelocationModel == llvm::Reloc::ROPI_RWPI)
+ Result.push_back("-frwpi");
+ else
+ Result.push_back("-fno-rwpi");
+ }
+
+ if (RelocationModel == llvm::Reloc::PIC_)
+ Result.push_back(IsPIE ? (PICLevel > 1 ? "-fPIE" : "-fpie")
+ : (PICLevel > 1 ? "-fPIC" : "-fpic"));
+ else
+ Result.push_back("-fno-pic");
+ }
+
// Sort and remove duplicates.
std::sort(Result.begin(), Result.end());
Result.erase(llvm::unique(Result), Result.end());
diff --git a/clang/test/Driver/print-multi-selection-flags.c b/clang/test/Driver/print-multi-selection-flags.c
index 5f9383fbed8f4..1d794091970d3 100644
--- a/clang/test/Driver/print-multi-selection-flags.c
+++ b/clang/test/Driver/print-multi-selection-flags.c
@@ -107,3 +107,22 @@
// CHECK-AARCH64-MULTILIB-CUSTOM-FLAG: --target=aarch64-unknown-none-eabi
// CHECK-MULTILIB-CUSTOM-FLAG-DAG: -fmultilib-flag=foo
// CHECK-MULTILIB-CUSTOM-FLAG-DAG: -fmultilib-flag=bar
+
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fropi | FileCheck --check-prefixes=CHECK-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -frwpi | FileCheck --check-prefixes=CHECK-RWPI,CHECK-NO-ROPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fropi -frwpi | FileCheck --check-prefixes=CHECK-ROPI,CHECK-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fno-ropi -fno-rwpi | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fpic | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIC1 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fPIC | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIC2 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fpie | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIE1 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fPIE | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIE2 %s
+// CHECK-ROPI: -fropi
+// CHECK-NO-ROPI: -fno-ropi
+// CHECK-RWPI: -frwpi
+// CHECK-NO-RWPI: -fno-rwpi
+// CHECK-PIC1: -fpic
+// CHECK-PIC2: -fPIC
+// CHECK-PIE1: -fpie
+// CHECK-PIE2: -fPIE
+// CHECK-NO-PIC: -fno-pic
|
|
@llvm/pr-subscribers-clang-driver Author: Simon Tatham (statham-arm) ChangesIf a multilib collection contains libraries built for different methods of accessing global data (via absolute address, or via a GOT in -fPIC style, or as an offset from a fixed register in Arm -frwpi style), then Even if a multilib collection only supports one relocation model, it's still useful for In this commit we determine the PIC / ROPI / RWPI status using the existing logic in Full diff: https://github.com/llvm/llvm-project/pull/149132.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 3f9b808b2722e..5738c0aa9b1d2 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -343,6 +343,7 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
std::vector<std::string> Result;
const llvm::Triple Triple(ComputeEffectiveClangTriple(Args));
Result.push_back("--target=" + Triple.str());
+ bool IsARM = false;
switch (Triple.getArch()) {
case llvm::Triple::aarch64:
@@ -355,6 +356,7 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
case llvm::Triple::thumb:
case llvm::Triple::thumbeb:
getARMMultilibFlags(D, Triple, Args, Result);
+ IsARM = true; // for ROPI/RWPI below
break;
case llvm::Triple::riscv32:
case llvm::Triple::riscv64:
@@ -376,6 +378,41 @@ ToolChain::getMultilibFlags(const llvm::opt::ArgList &Args) const {
else
Result.push_back("-fexceptions");
+ // A difference of relocation model (absolutely addressed data, PIC, Arm
+ // ROPI/RWPI) is likely to change whether a particular multilib variant is
+ // compatible with a given link. Determine the relocation model of the
+ // current link, and add appropriate
+ {
+ RegisterEffectiveTriple TripleRAII(
+ *this, llvm::Triple(ComputeEffectiveClangTriple(Args)));
+
+ auto [RelocationModel, PICLevel, IsPIE] = tools::ParsePICArgs(*this, Args);
+
+ // ROPI and RWPI are only meaningful on Arm, so for other architectures, we
+ // expect never to find out they're enabled. But it seems confusing to add
+ // -fno-ropi and -fno-rwpi unconditionally to every other architecture's
+ // multilib flags, so instead we leave them out completely.
+ if (IsARM) {
+ if (RelocationModel == llvm::Reloc::ROPI ||
+ RelocationModel == llvm::Reloc::ROPI_RWPI)
+ Result.push_back("-fropi");
+ else
+ Result.push_back("-fno-ropi");
+
+ if (RelocationModel == llvm::Reloc::RWPI ||
+ RelocationModel == llvm::Reloc::ROPI_RWPI)
+ Result.push_back("-frwpi");
+ else
+ Result.push_back("-fno-rwpi");
+ }
+
+ if (RelocationModel == llvm::Reloc::PIC_)
+ Result.push_back(IsPIE ? (PICLevel > 1 ? "-fPIE" : "-fpie")
+ : (PICLevel > 1 ? "-fPIC" : "-fpic"));
+ else
+ Result.push_back("-fno-pic");
+ }
+
// Sort and remove duplicates.
std::sort(Result.begin(), Result.end());
Result.erase(llvm::unique(Result), Result.end());
diff --git a/clang/test/Driver/print-multi-selection-flags.c b/clang/test/Driver/print-multi-selection-flags.c
index 5f9383fbed8f4..1d794091970d3 100644
--- a/clang/test/Driver/print-multi-selection-flags.c
+++ b/clang/test/Driver/print-multi-selection-flags.c
@@ -107,3 +107,22 @@
// CHECK-AARCH64-MULTILIB-CUSTOM-FLAG: --target=aarch64-unknown-none-eabi
// CHECK-MULTILIB-CUSTOM-FLAG-DAG: -fmultilib-flag=foo
// CHECK-MULTILIB-CUSTOM-FLAG-DAG: -fmultilib-flag=bar
+
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fropi | FileCheck --check-prefixes=CHECK-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -frwpi | FileCheck --check-prefixes=CHECK-RWPI,CHECK-NO-ROPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fropi -frwpi | FileCheck --check-prefixes=CHECK-ROPI,CHECK-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fno-ropi -fno-rwpi | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-NO-PIC %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fpic | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIC1 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fPIC | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIC2 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fpie | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIE1 %s
+// RUN: %clang -multi-lib-config=%S/Inputs/multilib/empty.yaml -print-multi-flags-experimental --target=arm-none-eabi -march=armv7a -fPIE | FileCheck --check-prefixes=CHECK-NO-ROPI,CHECK-NO-RWPI,CHECK-PIE2 %s
+// CHECK-ROPI: -fropi
+// CHECK-NO-ROPI: -fno-ropi
+// CHECK-RWPI: -frwpi
+// CHECK-NO-RWPI: -fno-rwpi
+// CHECK-PIC1: -fpic
+// CHECK-PIC2: -fPIC
+// CHECK-PIE1: -fpie
+// CHECK-PIE2: -fPIE
+// CHECK-NO-PIC: -fno-pic
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Note to reviewers: the thing I'm most uncertain of in this patch is whether I'm using the right strategy for finding out the information about relocation model.
But I'm not very confident of what an "effective triple" is at all, and I don't know for sure that it's right to call |
(Thanks to clang-format for pointing out the trailing space on that line, which was there because I meant to type a word after it!)
clang/lib/Driver/ToolChain.cpp
Outdated
| // expect never to find out they're enabled. But it seems confusing to add | ||
| // -fno-ropi and -fno-rwpi unconditionally to every other architecture's | ||
| // multilib flags, so instead we leave them out completely. | ||
| if (IsARM) { |
There was a problem hiding this comment.
Is there any way to move this Arm-only segment into getARMMultilibFlags? In theory, that function handles everything that is only relevant to Arm. So it's a more natural place to host this segment here.
There was a problem hiding this comment.
Done. In my early drafts I had the whole thing in getARMMultilibFlags, but moved it out once I realised that PIC needed to be handled as well as ROPI/RWPI. I've kept the calculation of the relocation model in the top-level function, and just passed the answer down into getARMMultilibFlags where it can add the Arm-specific options.
Answering my own question: I had failed to spot that |
All the CHECK lines are checked in a single run of FileCheck, so the options they mention must appear in the same order that the flags are shown in the -print-multi-flags output, which is ASCII sorting order.
ATfE doesn't currently provide any library variants built for RWPI. And if you link RWPI with non-RWPI code, nothing good will happen – they have incompatible ABIs. So we ought to give an error message rather than silently choose an incompatible library. Using llvm/llvm-project#149132 which allowed multilib selection to be aware of the relocation model at all, this commit does the simplest possible thing: adds a catch-all error message that will trigger on _any_ use of `-frwpi`, and report that there's no suitable library variant. (Another possibility would have been to add `-fno-rwpi` to the validity criteria for each existing library, but that seemed less likely to give a good error message. It would allow RWPI variants to be present for some cases but not all, but at the moment, we don't need that.) Since the error message works the same everywhere, I've just added one test.
If a multilib collection contains libraries built for different methods of accessing global data (via absolute address, or via a GOT in -fPIC style, or as an offset from a fixed register in Arm -frwpi style), then
multilib.yamlwill need to know which relocation model an application is using in order to select the right library.Even if a multilib collection only supports one relocation model, it's still useful for
multilib.yamlto be able to tell if the user has selected the right one, so as to give a useful error message if they haven't, instead of silently selecting a library that won't work.In this commit we determine the PIC / ROPI / RWPI status using the existing logic in
ParsePICArgs, and translate it back into a canonical set of multilib selection flags.