[BOLT][AArch64] Add base_cflags for PI flags

[paschalis-mpeis]

By default, %cflags/%cxxflags include -fPIE and -pie, which may be overridden by position-independent (PI) flags in tests. On some systems, tests fail as those defaults are not inverted correctly.

This patch adds %base_cflags/%base_cxxflags for cases where tests need to specify PI flags explicitly.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Paschalis Mpeis (paschalis-mpeis)

Changes

By default, %cflags/%cxxflags include -fPIE and -pie, which may be overridden by position-independent (PI) flags in tests. On some systems, tests fail as those defaults are not inverted correctly.

This patch adds %base_cflags/%base_cxxflags for cases where tests need to specify PI flags explicitly.

---

Full diff: https://github.com/llvm/llvm-project/pull/145502.diff

10 Files Affected:

• (modified) bolt/test/AArch64/array_end.c (+1-1)
• (modified) bolt/test/AArch64/asm-func-debug.test (+1-1)
• (modified) bolt/test/AArch64/data-in-code.s (+1-1)
• (modified) bolt/test/AArch64/hook-fini.s (+1-1)
• (modified) bolt/test/AArch64/ifunc.test (+3-3)
• (modified) bolt/test/AArch64/jmp-table-unsupported.s (+1-1)
• (modified) bolt/test/AArch64/lit.local.cfg (+4-1)
• (modified) bolt/test/AArch64/tls.c (+1-1)
• (modified) bolt/test/AArch64/veneer-lld-abs.s (+1-1)
• (modified) bolt/test/lit.local.cfg (+12-4)

diff --git a/bolt/test/AArch64/array_end.c b/bolt/test/AArch64/array_end.c
index 6f4f9800c3903..de8e052288648 100644
--- a/bolt/test/AArch64/array_end.c
+++ b/bolt/test/AArch64/array_end.c
@@ -3,7 +3,7 @@
 // __init_array_end address would not be owned by any section.
 
 // REQUIRES: system-linux
-// RUN: %clang %cflags -no-pie %s -o %t.exe -Wl,-q \
+// RUN: %clang %base_cflags -no-pie %s -o %t.exe -Wl,-q \
 // RUN:   -Wl,-T %S/Inputs/array_end.lld_script
 // RUN: llvm-bolt %t.exe -o %t.bolt --print-disasm \
 // RUN:  --print-only="callFini" | FileCheck %s
diff --git a/bolt/test/AArch64/asm-func-debug.test b/bolt/test/AArch64/asm-func-debug.test
index 546add5ade02f..6bf85e301930c 100644
--- a/bolt/test/AArch64/asm-func-debug.test
+++ b/bolt/test/AArch64/asm-func-debug.test
@@ -3,7 +3,7 @@
 #
 # The input test case foo() contains nops that we remove.
 
-RUN: %clang %cflags -gdwarf-5 -no-pie %p/../Inputs/asm_foo.s %p/../Inputs/asm_main.c -o %t.exe
+RUN: %clang %base_cflags -gdwarf-5 -no-pie %p/../Inputs/asm_foo.s %p/../Inputs/asm_main.c -o %t.exe
 RUN: llvm-bolt %t.exe -o %t --update-debug-sections
 RUN: llvm-dwarfdump -all %t | FileCheck %s
 
diff --git a/bolt/test/AArch64/data-in-code.s b/bolt/test/AArch64/data-in-code.s
index 1df5d4568542f..bc25337b32057 100644
--- a/bolt/test/AArch64/data-in-code.s
+++ b/bolt/test/AArch64/data-in-code.s
@@ -1,7 +1,7 @@
 ## Check that llvm-bolt prints data embedded in code.
 
 # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
-# RUN: %clang %cflags -fno-PIC -no-pie %t.o -o %t.exe -nostdlib \
+# RUN: %clang %base_cflags -fno-PIC -no-pie %t.o -o %t.exe -nostdlib \
 # RUN:    -fuse-ld=lld -Wl,-q
 
 ## Check disassembly of BOLT input.
diff --git a/bolt/test/AArch64/hook-fini.s b/bolt/test/AArch64/hook-fini.s
index 4f321d463ef32..e5a473cde8474 100644
--- a/bolt/test/AArch64/hook-fini.s
+++ b/bolt/test/AArch64/hook-fini.s
@@ -27,7 +27,7 @@
 
 ## Create a dummy shared library to link against to force creation of the dynamic section.
 # RUN: %clang %cflags %p/../Inputs/stub.c -fPIC -shared -o %t-stub.so
-# RUN: %clang %cflags %s -no-pie -Wl,-q,-fini=0 %t-stub.so -o %t-no-pie-no-fini.exe
+# RUN: %clang %base_cflags %s -no-pie -Wl,-q,-fini=0 %t-stub.so -o %t-no-pie-no-fini.exe
 # RUN: llvm-readelf -r %t-no-pie-no-fini.exe | FileCheck --check-prefix=RELOC-NO-PIE %s
 # RUN: llvm-bolt %t-no-pie-no-fini.exe -o %t-no-pie-no-fini --instrument
 # RUN: llvm-readelf -ds -x .fini_array %t-no-pie-no-fini | FileCheck --check-prefix=CHECK-NO-PIE-NO-FINI %s
diff --git a/bolt/test/AArch64/ifunc.test b/bolt/test/AArch64/ifunc.test
index 3da42c67c5a0a..c7f890275da46 100644
--- a/bolt/test/AArch64/ifunc.test
+++ b/bolt/test/AArch64/ifunc.test
@@ -1,6 +1,6 @@
 // With -O0 indirect call is performed on IPLT trampoline. IPLT trampoline
 // has IFUNC symbol.
-// RUN: %clang %cflags -nostdlib -O0 -no-pie %p/../Inputs/ifunc.c -fuse-ld=lld \
+// RUN: %clang %base_cflags -nostdlib -O0 -no-pie %p/../Inputs/ifunc.c -fuse-ld=lld \
 // RUN:    -o %t.O0.exe -Wl,-q
 // RUN: llvm-bolt %t.O0.exe -o %t.O0.bolt.exe \
 // RUN:   --print-disasm --print-only=_start | \
@@ -9,8 +9,8 @@
 // RUN:   FileCheck --check-prefix=REL_CHECK %s
 
 // Non-pie static executable doesn't generate PT_DYNAMIC, check relocation
-// is readed successfully and IPLT trampoline has been identified by bolt.
-// RUN: %clang %cflags -nostdlib -O3 %p/../Inputs/ifunc.c -fuse-ld=lld -no-pie \
+// is read successfully and IPLT trampoline has been identified by bolt.
+// RUN: %clang %base_cflags -nostdlib -O3 %p/../Inputs/ifunc.c -fuse-ld=lld -no-pie \
 // RUN:   -o %t.O3_nopie.exe -Wl,-q
 // RUN: llvm-readelf -l %t.O3_nopie.exe | \
 // RUN:   FileCheck --check-prefix=NON_DYN_CHECK %s
diff --git a/bolt/test/AArch64/jmp-table-unsupported.s b/bolt/test/AArch64/jmp-table-unsupported.s
index 1228149430449..d3cc4c6ed395d 100644
--- a/bolt/test/AArch64/jmp-table-unsupported.s
+++ b/bolt/test/AArch64/jmp-table-unsupported.s
@@ -92,7 +92,7 @@ SECTIONS {
 # JT-BOLT-FIXED-BR: failed to match indirect branch
 
 ## Prepare binary (6)
-# RUN: %clang %cflags -no-pie %t/jt_type_normal.c \
+# RUN: %clang %base_cflags -no-pie %t/jt_type_normal.c \
 # RUN:   -Wl,-q -Wl,-z,now -Wl,--no-relax \
 # RUN:   -o %t/jt_type_normal.exe
 # RUN: llvm-objdump --no-show-raw-insn -d %t/jt_type_normal.exe | FileCheck \
diff --git a/bolt/test/AArch64/lit.local.cfg b/bolt/test/AArch64/lit.local.cfg
index 9432240469c7b..dcca5c4c9ab8b 100644
--- a/bolt/test/AArch64/lit.local.cfg
+++ b/bolt/test/AArch64/lit.local.cfg
@@ -1,7 +1,10 @@
 if "AArch64" not in config.root.targets:
     config.unsupported = True
 
-flags = "--target=aarch64-unknown-linux-gnu -nostartfiles -nostdlib -ffreestanding"
+flags = "-nostartfiles -nostdlib -ffreestanding"
 
 config.substitutions.insert(0, ("%cflags", f"%cflags {flags}"))
 config.substitutions.insert(0, ("%cxxflags", f"%cxxflags {flags}"))
+
+config.substitutions.insert(0, ("%base_cflags", f"%base_cflags {flags}"))
+config.substitutions.insert(0, ("%base_cxxflags", f"%base_cxxflags {flags}"))
diff --git a/bolt/test/AArch64/tls.c b/bolt/test/AArch64/tls.c
index b531811f679ff..f037b0ff9a38b 100644
--- a/bolt/test/AArch64/tls.c
+++ b/bolt/test/AArch64/tls.c
@@ -24,7 +24,7 @@ int main() {
 }
 
 // REQUIRES: system-linux
-// RUN: %clang %cflags -no-pie %s -o %t.exe -Wl,-q \
+// RUN: %clang %base_cflags -no-pie %s -o %t.exe -Wl,-q \
 // RUN:   -Wl,--unresolved-symbols=ignore-all \
 // RUN:   -fuse-ld=lld \
 // RUN:   -nostdlib
diff --git a/bolt/test/AArch64/veneer-lld-abs.s b/bolt/test/AArch64/veneer-lld-abs.s
index b22301db66c54..40d1476e1e36b 100644
--- a/bolt/test/AArch64/veneer-lld-abs.s
+++ b/bolt/test/AArch64/veneer-lld-abs.s
@@ -2,7 +2,7 @@
 ## generated by LLD.
 
 # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
-# RUN: %clang %cflags -fno-PIC -no-pie %t.o -o %t.exe -nostdlib \
+# RUN: %clang %base_cflags -fno-PIC -no-pie %t.o -o %t.exe -nostdlib \
 # RUN:    -fuse-ld=lld -Wl,-q
 # RUN: llvm-objdump -d %t.exe | FileCheck --check-prefix=CHECK-INPUT %s
 # RUN: llvm-objcopy --remove-section .rela.mytext %t.exe
diff --git a/bolt/test/lit.local.cfg b/bolt/test/lit.local.cfg
index 8a61d11f5825f..5b2a550ad8995 100644
--- a/bolt/test/lit.local.cfg
+++ b/bolt/test/lit.local.cfg
@@ -4,8 +4,16 @@ host_triple = config.target_triple
 if not "linux" in host_triple:
   host_triple = host_triple.split("-")[0] + "-unknown-linux-gnu"
 
-common_linker_flags = "-fuse-ld=lld -Wl,--unresolved-symbols=ignore-all -Wl,--build-id=none -pie"
-flags = f"--target={host_triple} -fPIE {common_linker_flags}"
+# By default, %cflags/%cxxflags enable position-independent code. To be explicit
+# in tests, use %base_cflags/%base_cxxflags and provide any relevant flags, ie
+# -fPIE/-fPIC/-pie, and -fno-PIE/-fno-PIC/-no-pie
 
-config.substitutions.insert(0, ("%cflags", f"%cflags {flags}"))
-config.substitutions.insert(0, ("%cxxflags", f"%cxxflags {flags}"))
+common_linker_flags = "-fuse-ld=lld -Wl,--unresolved-symbols=ignore-all -Wl,--build-id=none"
+
+flags_base = f"--target={host_triple} {common_linker_flags}"
+config.substitutions.insert(0, ("%base_cflags", f"%cflags {flags_base}"))
+config.substitutions.insert(0, ("%base_cxxflags", f"%cxxflags {flags_base}"))
+
+flags_pie = f"{flags_base} -fPIE -pie"
+config.substitutions.insert(0, ("%cflags", f"%cflags {flags_pie}"))
+config.substitutions.insert(0, ("%cxxflags", f"%cxxflags {flags_pie}"))

[aaupov]

Is PIE considered the base setting? I'd say it's better to have explicit names instead

[paschalis-mpeis]

Yep, it's included by default in %cflags/%cxxflags. I'd also prefer to be explicit, but I understand why it's the default.

As an alternative to %base_cflags, we could introduce %pie_cflags and %nopie_cflags (likewise for cxx).
What do you think?

[aaupov]

Sorry about the delay. PIE or no-PIE being the default is a choice that depends on the distro/toolchain. I'd vote for making it explicit: pie/nopie. If you have a different opinion, let's check with others: cc @maksfb @yota9

[paschalis-mpeis]

I'd vote for making it explicit: pie/nopie. If you have a different opinion, let's check with others: cc @maksfb @yota9

I also prefer making everything explicit. This change would require replacing every %cflags / %cxxflags with pie_* or nopie_* variants.

Uploaded a draft patch to see if is what we want. If we agree, I'll follow-up with a full patch.