-fno-PIC targeting glibc should be legal

kubkon commented 11 months ago

Zig Version

0.12.0-dev.832+d5c19ecd5

Steps to Reproduce and Observed Behavior

This is currently illegal:

$ echo "int main() { return 0; }" > main.c
$ zig cc -c -fno-PIC main.c -target x86_64-linux-gnu
error: unable to create compilation: TargetRequiresPIC

but is actually a valid compilation combo.

$ clang -c -fno-PIC main.c -target x86_64-linux-unknown-gnu

Expected Behavior

Compiles successfully.

kubkon commented 11 months ago

OK, I've tried the proposed fix as follows:

diff --git a/src/Compilation.zig b/src/Compilation.zig
index 28b67ff7346..0afcd9f03f5 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -1034,7 +1034,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
         const must_pic: bool = b: {
             if (target_util.requiresPIC(options.target, link_libc))
                 break :b true;
-            break :b link_mode == .Dynamic;
+            break :b link_mode == .Dynamic and options.output_mode == .Lib;
         };
         const pic = if (options.want_pic) |explicit| pic: {
             if (!explicit) {
@@ -5162,8 +5162,12 @@ pub fn addCCArgs(
                 },
             }

-            if (target_util.supports_fpic(target) and comp.bin_file.options.pic) {
-                try argv.append("-fPIC");
+            if (target_util.supports_fpic(target)) {
+                if (comp.bin_file.options.pic) {
+                    try argv.append("-fPIC");
+                } else {
+                    try argv.append("-fno-PIC");
+                }
             }

             if (comp.unwind_tables) {
diff --git a/src/target.zig b/src/target.zig
index c137166543d..125f48f81b6 100644
--- a/src/target.zig
+++ b/src/target.zig
@@ -192,10 +192,10 @@ pub fn requiresPIE(target: std.Target) bool {

 /// This function returns whether non-pic code is completely invalid on the given target.
 pub fn requiresPIC(target: std.Target, linking_libc: bool) bool {
+    _ = linking_libc;
     return target.isAndroid() or
         target.os.tag == .windows or target.os.tag == .uefi or
-        osRequiresLibC(target) or
-        (linking_libc and target.isGnuLibC());
+        osRequiresLibC(target);
 }

 /// This is not whether the target supports Position Independent Code, but whether the -fPIC

However, LLD fails to link test-std tests with a failed relocation error:

$ ./stage3/bin/zig test ../lib/std/std.zig  -target aarch64-linux-gnu --zig-lib-dir /home/kubkon/dev/zig/lib/ -lc
...
ld.lld: error: /home/kubkon/dev/zig/zig-cache/o/e765335f0502c09a9967f8a26326652a/test.o:(function math.mul__anon_6094: .text+0x3f00): improper alignment for relocation R_AARCH64_LDST64_ABS_LO12_NC: 0x2340BB4 is not aligned to 8 bytes

From my brief investigation this looks like a reference to __stack_chk_guard/__stack_chk_fail stating that the symbol is misaligned. The funny thing is this symbol is dynamically exported by libc.so and it is aligned alright. Anyhow, passing -fno-stack-protector and -fno-stack-check makes the issue go away.

Also, linking the binary with mold works fine and emits a functional binary:

$ mold -z stack-size=16777216 --gc-sections --eh-frame-hdr -znow -m aarch64linux -o test /home/kubkon/.cache/zig/o/12dd35ea87cd033035d74567d6572a28/Scrt1.o /home/kubkon/.cache/zig/o/57eaab073ec0faaaf3679b7767e6d438/crti.o -dynamic-linker /lib/ld-linux-aarch64.so.1 /home/kubkon/dev/zig/zig-cache/o/e765335f0502c09a9967f8a26326652a/test.o --as-needed /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libm.so.6 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libpthread.so.0 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libc.so.6 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libdl.so.2 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/librt.so.1 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libld.so.2 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libutil.so.1 /home/kubkon/.cache/zig/o/7a770135ee851ffefd975b614f376e2a/libresolv.so.2 /home/kubkon/.cache/zig/o/50bd6a549a142bd1999535df0d05b1f7/libc_nonshared.a /home/kubkon/.cache/zig/o/8f0c3369fc70e2e54e065f49c0565fda/libcompiler_rt.a /home/kubkon/.cache/zig/o/c3840fa00c1dd2a0af5ee276cfe081f4/crtn.o --allow-shlib-undefined
$ qemu-aarch64 -L /home/kubkon/dev/glibc/multi/install/glibcs/aarch64-linux-gnu test
...
2527 passed; 77 skipped; 0 failed.

I suspect there might be a bug in LLD somewhere, but instead of trying to narrow it down at this stage, I will defer it until our linker is capable of linking aarch64 and see what's up then.

Jarred-Sumner commented 2 months ago

Currently, about 3.72 MB of Bun's executable is .data.rel.ro on Linux x64. I think most of that would be removed if zig build-obj didn't prevent disabling PIC. Each time we return a runtime pointer from a comptime function, it appears to create a new symbol to relocate in the .data.rel.ro section on Linux.

Section	Unit	Symbol	Filesize	VMSize	Percent of total file size
.rodata	icudt75_dat	[section .rodata]	30729184	30729184	31.71
.data.rel.ro	[section .data.rel.ro]		3734616	3759256	3.88
.rodata	[section .rodata]	[section .rodata]	2994946	2994946	3.09
.rodata	__anon_1124411	[section .rodata]	695727	695727	0.72
.rodata	hencs	[section .rodata]	524288	524288	0.54
.text	JSC::FTL::(anonymous namespace)::LowerDFGToB3::lower()	[section .text]	429088	429088	0.44
.rodata	Bun::InternalModuleRegistryConstants::NodeCryptoCodeBytes	[section .rodata]	428432	428432	0.44
.text	Bun::ProcessBindingUV::jsGetErrorMap(JSC::JSGlobalObject, JSC::CallFrame)	ProcessBindingTTYWrap.cpp	333728	333728	0.34
.rodata	hdecs	[section .rodata]	262144	262144	0.27
.rodata	__anon_1127446	[section .rodata]	224938	224938	0.23
.rodata	JSC::s_JSCCombinedCode	[section .rodata]	215020	215020	0.22
.gnu.hash	[section .gnu.hash]		190720	190720	0.2
.rodata	__anon_1919867	[section .rodata]	164286	164286	0.17
.rodata	Bun::InternalModuleRegistryConstants::NodeStreamCodeBytes	[section .rodata]	158336	158336	0.16
.rodata	ecp_nistz256_precomputed	[section .rodata]	151552	151552	0.16
.text	JSC::Yarr::createCharacterClass325()	[section .text]	150464	150464	0.16

ziglang / zig