Closed slyon closed 2 years ago
Shouldn't all occurrences of the NO_PIE_LDFLAGS variable go away then? I mean remove it also from common.mak and review the use of NO_PIE_LINKFLAGS vs NO_PIE_LDFLAGS
Yes. The NO_PIE_LINKFLAGS vs NO_PIE_LDFLAGS should be reviewed, as their differentiation was not really clear to me.
But I saw that in some places the NO_PIE_LDFLAGS are also used as a compiler flag, where it is valid and needed. It's just invalid as a ld
flag.
As an external contributor I did not want do fiddle too much with the build system, but would rather leave this to somebody more involved with the project. This PR is just the smallest possible fix I could find and applied to the Ubuntu package.
I thought that -no-pie
is valid for some tooling, i.e. gcc
accepts it and does things with it? but not ld
?
gcc
accepts it and does things with it? but notld
?
Exactly.
So, using -no-pie for ld "worked" just by chance because ld was bugged and didn't properly check what was specified? Am I getting that right?
Anyway, the PR seems to be okay. Looking into the other NOPIE* stuff as well though before pulling it. Thanks for the hint!
Correct @hoeppnerj
ld
just ignored that flag earlier, but now it complains about it (as of binutils >= 3.35.50). And upstream tells us: "The linker does not have a -no-pie or a --no-pie command line option, so the error message is correct."
Thanks for looking into it!
@slyon I've prepared a patch internally to get rid of the NO_PIE_LDFLAGS entirely. I'll merge your patch with that and add you as Suggested-by, would that be okay with you? Nevermind, I'll come back to this, just noticed a couple of other things that might need an additional patch, so yours would stay as is.
a friendly ping :-) It's breaking builds in Fedora Rawhide with binutils 2.36.1-15.fc35
ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
@slyon sorry, it's been a while. We have a version internally now that fixes the build issues.
However, instead of removing the -no-pie
option we instead use GCC for the linking as it should be
done anyway. This particular part was the only one that used ld
directly.
With that we fix two things in one patch.
That also means that I won't take your patch here. But I would gladly mention you as Reported-by and reference this PR if that's okay for you.
The patch looks like this:
diff --git a/common.mak b/common.mak
index 78423dac4848..e08a9023b635 100644
--- a/common.mak
+++ b/common.mak
@@ -60,7 +60,6 @@ endef
$(eval $(call cmd_define, AS," AS ",$(CROSS_COMPILE)as))
$(eval $(call cmd_define, LINK," LINK ",$(CROSS_COMPILE)gcc))
-$(eval $(call cmd_define, LD," LD ",$(CROSS_COMPILE)ld))
$(eval $(call cmd_define, CC," CC ",$(CROSS_COMPILE)gcc))
$(eval $(call cmd_define, HOSTCC," HOSTCC ",gcc))
$(eval $(call cmd_define, LINKXX," LINKXX ",$(CROSS_COMPILE)g++))
diff --git a/zipl/boot/Makefile b/zipl/boot/Makefile
index 09403a787719..e0c041e78684 100644
--- a/zipl/boot/Makefile
+++ b/zipl/boot/Makefile
@@ -110,7 +110,7 @@ stage3.bin: stage3.exec
$< $@
data.o: $(FILES)
- $(LD) $(NO_PIE_LDFLAGS) -r -b binary -o data.o $(FILES)
+ $(LINK) $(NO_PIE_LDFLAGS) -static -nostdlib -Wl,--relocatable -Wl,--format,binary -o data.o $(FILES)
data.h: data.o
rm -f data.h
Sounds good to me, thank you!
The "-no-pie" option never has been a valid flag of ld. It breaks the build with newer binutils. See: https://sourceware.org/bugzilla/show_bug.cgi?id=27050 https://bugs.launchpad.net/ubuntu/+source/s390-tools/+bug/1907789