[PATCH] Partially revert "build: -no-pie is no functional linker flag"

[PATCH] Partially revert "build: -no-pie is no functional linker flag"

[Jessica Clarke]

This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.

This commit was misguided and broke using --disable-pie on any distro
that enables PIE by default in their compiler driver, including Debian
and its derivatives. Whilst -no-pie is not a linker flag, it is a
compiler driver flag that ensures -pie is not automatically passed by it
to the linker. Without it, all compile_prog checks will fail as any code
built with the explicit -fno-pie will fail to link with the implicit
default -pie due to trying to use position-dependent relocations. The
only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
the linker itself in pc-bios/optionrom/Makefile.

Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
since the only previous use was the one that should not have existed. I
have also updated the comment for the -fno-pie and -no-pie checks to
reflect what they're actually needed for.

Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index 9a79a004d7..b8b29e1eee 100755
--- a/configure
+++ b/configure
@@ -2249,6 +2249,7 @@ EOF
 # Check we support --no-pie first; we will need this for building ROMs.
 if compile_prog "-Werror -fno-pie" "-no-pie"; then
   CFLAGS_NOPIE="-fno-pie"
+  LDFLAGS_NOPIE="-no-pie"
 fi
 
 if test "$static" = "yes"; then
@@ -2264,6 +2265,7 @@ if test "$static" = "yes"; then
   fi
 elif test "$pie" = "no"; then
   CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS"
+  CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS"
 elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
   CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS"
   CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS"
-- 
2.17.1

[PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Jessica Clarke]

This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.

This commit was misguided and broke using --disable-pie on any distro
that enables PIE by default in their compiler driver, including Debian
and its derivatives. Whilst -no-pie is not a linker flag, it is a
compiler driver flag that ensures -pie is not automatically passed by it
to the linker. Without it, all compile_prog checks will fail as any code
built with the explicit -fno-pie will fail to link with the implicit
default -pie due to trying to use position-dependent relocations. The
only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
the linker itself in pc-bios/optionrom/Makefile.

Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
since the only previous use was the one that should not have existed. I
have also updated the comment for the -fno-pie and -no-pie checks to
reflect what they're actually needed for.

Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
---
Changes in v2:
  * Actually include the comment change; didn't add the hunk when
    amending...

 configure | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 9a79a004d7..8aecd277ed 100755
--- a/configure
+++ b/configure
@@ -2246,9 +2246,11 @@ static THREAD int tls_var;
 int main(void) { return tls_var; }
 EOF
 
-# Check we support --no-pie first; we will need this for building ROMs.
+# Check we support -fno-pie and -no-pie first; we will need the former for
+# building ROMs, and both for everything if --disable-pie is passed.
 if compile_prog "-Werror -fno-pie" "-no-pie"; then
   CFLAGS_NOPIE="-fno-pie"
+  LDFLAGS_NOPIE="-no-pie"
 fi
 
 if test "$static" = "yes"; then
@@ -2264,6 +2266,7 @@ if test "$static" = "yes"; then
   fi
 elif test "$pie" = "no"; then
   CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS"
+  CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS"
 elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
   CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS"
   CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS"
-- 
2.17.1

Re: [PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Michael Roth]

Quoting Jessica Clarke (2021-08-05 14:25:45)
> This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
> 
> This commit was misguided and broke using --disable-pie on any distro
> that enables PIE by default in their compiler driver, including Debian
> and its derivatives. Whilst -no-pie is not a linker flag, it is a
> compiler driver flag that ensures -pie is not automatically passed by it
> to the linker. Without it, all compile_prog checks will fail as any code
> built with the explicit -fno-pie will fail to link with the implicit
> default -pie due to trying to use position-dependent relocations. The
> only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
> the linker itself in pc-bios/optionrom/Makefile.
> 
> Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
> since the only previous use was the one that should not have existed. I
> have also updated the comment for the -fno-pie and -no-pie checks to
> reflect what they're actually needed for.
> 
> Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>

Ping. --disable-pie builds are broken on Ubuntu 20.04 without this regression
fix. Looking to include it for v6.0.1/v6.1.1.

> ---
> Changes in v2:
>   * Actually include the comment change; didn't add the hunk when
>     amending...
> 
>  configure | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 9a79a004d7..8aecd277ed 100755
> --- a/configure
> +++ b/configure
> @@ -2246,9 +2246,11 @@ static THREAD int tls_var;
>  int main(void) { return tls_var; }
>  EOF
>  
> -# Check we support --no-pie first; we will need this for building ROMs.
> +# Check we support -fno-pie and -no-pie first; we will need the former for
> +# building ROMs, and both for everything if --disable-pie is passed.
>  if compile_prog "-Werror -fno-pie" "-no-pie"; then
>    CFLAGS_NOPIE="-fno-pie"
> +  LDFLAGS_NOPIE="-no-pie"
>  fi
>  
>  if test "$static" = "yes"; then
> @@ -2264,6 +2266,7 @@ if test "$static" = "yes"; then
>    fi
>  elif test "$pie" = "no"; then
>    CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS"
> +  CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS"
>  elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
>    CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS"
>    CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS"
> -- 
> 2.17.1
> 
>

Re: [PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Richard Henderson]

On 10/14/21 4:30 PM, Michael Roth wrote:
> Quoting Jessica Clarke (2021-08-05 14:25:45)
>> This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
>>
>> This commit was misguided and broke using --disable-pie on any distro
>> that enables PIE by default in their compiler driver, including Debian
>> and its derivatives. Whilst -no-pie is not a linker flag, it is a
>> compiler driver flag that ensures -pie is not automatically passed by it
>> to the linker. Without it, all compile_prog checks will fail as any code
>> built with the explicit -fno-pie will fail to link with the implicit
>> default -pie due to trying to use position-dependent relocations. The
>> only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
>> the linker itself in pc-bios/optionrom/Makefile.
>>
>> Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
>> since the only previous use was the one that should not have existed. I
>> have also updated the comment for the -fno-pie and -no-pie checks to
>> reflect what they're actually needed for.
>>
>> Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> 
> Ping. --disable-pie builds are broken on Ubuntu 20.04 without this regression
> fix. Looking to include it for v6.0.1/v6.1.1.

Sorry, missed this the first time around.

I think a better fix is to remove the direct invocation of ld in 
pc-bios/optionrom/Makefile, and instead rely on the compiler driver.  All of the local 
ldflags can be passed through via -Wl, but we'd get no-pie via CFLAGS_NOPIE.


r~

Re: [PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Jessica Clarke]

On 15 Oct 2021, at 01:12, Richard Henderson <richard.henderson@linaro.org> wrote:
> 
> On 10/14/21 4:30 PM, Michael Roth wrote:
>> Quoting Jessica Clarke (2021-08-05 14:25:45)
>>> This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
>>> 
>>> This commit was misguided and broke using --disable-pie on any distro
>>> that enables PIE by default in their compiler driver, including Debian
>>> and its derivatives. Whilst -no-pie is not a linker flag, it is a
>>> compiler driver flag that ensures -pie is not automatically passed by it
>>> to the linker. Without it, all compile_prog checks will fail as any code
>>> built with the explicit -fno-pie will fail to link with the implicit
>>> default -pie due to trying to use position-dependent relocations. The
>>> only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
>>> the linker itself in pc-bios/optionrom/Makefile.
>>> 
>>> Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
>>> since the only previous use was the one that should not have existed. I
>>> have also updated the comment for the -fno-pie and -no-pie checks to
>>> reflect what they're actually needed for.
>>> 
>>> Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
>>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
>> Ping. --disable-pie builds are broken on Ubuntu 20.04 without this regression
>> fix. Looking to include it for v6.0.1/v6.1.1.
> 
> Sorry, missed this the first time around.
> 
> I think a better fix is to remove the direct invocation of ld in pc-bios/optionrom/Makefile, and instead rely on the compiler driver.  All of the local ldflags can be passed through via -Wl, but we'd get no-pie via CFLAGS_NOPIE.

I am not changing anything about pc-bios/optionrom/Makefile. Whether
you want to remove direct ld invocation or not, these changes are
needed to reinstate --disable-pie support for everything else, which is
currently broken. The only reason to mention pc-bios/optionrom/Makefile
in the commit message is to explain why the original commit did what it
did and what part of it is legitimate (even if there are better ways to
have done it) and thus not reverted in this patch.

Jess

Re: [PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Richard Henderson]

On 10/14/21 5:22 PM, Jessica Clarke wrote:
> On 15 Oct 2021, at 01:12, Richard Henderson <richard.henderson@linaro.org> wrote:
>>
>> On 10/14/21 4:30 PM, Michael Roth wrote:
>>> Quoting Jessica Clarke (2021-08-05 14:25:45)
>>>> This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
>>>>
>>>> This commit was misguided and broke using --disable-pie on any distro
>>>> that enables PIE by default in their compiler driver, including Debian
>>>> and its derivatives. Whilst -no-pie is not a linker flag, it is a
>>>> compiler driver flag that ensures -pie is not automatically passed by it
>>>> to the linker. Without it, all compile_prog checks will fail as any code
>>>> built with the explicit -fno-pie will fail to link with the implicit
>>>> default -pie due to trying to use position-dependent relocations. The
>>>> only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
>>>> the linker itself in pc-bios/optionrom/Makefile.
>>>>
>>>> Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
>>>> since the only previous use was the one that should not have existed. I
>>>> have also updated the comment for the -fno-pie and -no-pie checks to
>>>> reflect what they're actually needed for.
>>>>
>>>> Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
>>>> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Cc: qemu-stable@nongnu.org
>>>> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
>>> Ping. --disable-pie builds are broken on Ubuntu 20.04 without this regression
>>> fix. Looking to include it for v6.0.1/v6.1.1.
>>
>> Sorry, missed this the first time around.
>>
>> I think a better fix is to remove the direct invocation of ld in pc-bios/optionrom/Makefile, and instead rely on the compiler driver.  All of the local ldflags can be passed through via -Wl, but we'd get no-pie via CFLAGS_NOPIE.
> 
> I am not changing anything about pc-bios/optionrom/Makefile. Whether
> you want to remove direct ld invocation or not, these changes are
> needed to reinstate --disable-pie support for everything else, which is
> currently broken. The only reason to mention pc-bios/optionrom/Makefile
> in the commit message is to explain why the original commit did what it
> did and what part of it is legitimate (even if there are better ways to
> have done it) and thus not reverted in this patch.

Ah, quite right -- currently configure fails.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v2] Partially revert "build: -no-pie is no functional linker flag"

[Paolo Bonzini]

On 05/08/21 21:25, Jessica Clarke wrote:
> This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
> 
> This commit was misguided and broke using --disable-pie on any distro
> that enables PIE by default in their compiler driver, including Debian
> and its derivatives. Whilst -no-pie is not a linker flag, it is a
> compiler driver flag that ensures -pie is not automatically passed by it
> to the linker. Without it, all compile_prog checks will fail as any code
> built with the explicit -fno-pie will fail to link with the implicit
> default -pie due to trying to use position-dependent relocations. The
> only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
> the linker itself in pc-bios/optionrom/Makefile.
> 
> Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
> since the only previous use was the one that should not have existed. I
> have also updated the comment for the -fno-pie and -no-pie checks to
> reflect what they're actually needed for.
> 
> Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
> Cc: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> ---
> Changes in v2:
>    * Actually include the comment change; didn't add the hunk when
>      amending...
> 
>   configure | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 9a79a004d7..8aecd277ed 100755
> --- a/configure
> +++ b/configure
> @@ -2246,9 +2246,11 @@ static THREAD int tls_var;
>   int main(void) { return tls_var; }
>   EOF
>   
> -# Check we support --no-pie first; we will need this for building ROMs.
> +# Check we support -fno-pie and -no-pie first; we will need the former for
> +# building ROMs, and both for everything if --disable-pie is passed.
>   if compile_prog "-Werror -fno-pie" "-no-pie"; then
>     CFLAGS_NOPIE="-fno-pie"
> +  LDFLAGS_NOPIE="-no-pie"
>   fi
>   
>   if test "$static" = "yes"; then
> @@ -2264,6 +2266,7 @@ if test "$static" = "yes"; then
>     fi
>   elif test "$pie" = "no"; then
>     CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS"
> +  CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS"
>   elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
>     CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS"
>     CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS"
> 

Queued, thanks.

Paolo