[PATCH] configure: check whether linker works too

[PATCH] configure: check whether linker works too

[Laine Gholson]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

As a result of http://www.openwall.com/lists/musl/2016/10/22/3
it has been shown that musl's current compiler sanity checks
are inadequate. This patch checks if the compiler as a whole
works properly, and moves the CPPFLAGS/CFLAGS/LDFLAGS sanity
checks after the 'compiler works' check.

Patch and patch signature attached

Thanks,
  Laine Gholson

[-- Attachment #2: configure.patch --]
[-- Type: text/x-patch, Size: 1159 bytes --]

From 99fd54d6f2b7c84a02c1ca0d5f1397d709c14313 Mon Sep 17 00:00:00 2001
From: Laine Gholson <laine.gholson@gmail.com>
Date: Mon, 24 Oct 2016 17:49:58 -0500
Subject: [PATCH] configure: check whether linker works too

and move flag sanity checks to a seperate test
---
 configure | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 707eb12..e7d565c 100755
--- a/configure
+++ b/configure
@@ -236,8 +236,17 @@ printf "%s\n" "$CC"
 test -n "$CC" || { echo "$0: cannot find a C compiler" ; exit 1 ; }
 
 printf "checking whether C compiler works... "
-echo "typedef int x;" > "$tmpc"
-if output=$($CC $CPPFLAGS $CFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then
+echo 'int main(void) { return 0; }' > "$tmpc"
+if output=$($CC -o /dev/null "$tmpc" 2>&1) ; then
+printf "yes\n"
+else
+printf "no; compiler output follows:\n%s\n" "$output"
+exit 1
+fi
+
+printf 'checking whether *FLAGS are sane... '
+echo 'int main(void) { return 0; }' > "$tmpc"
+if output=$($CC $CFLAGS $CPPFLAGS $LDFLAGS -o /dev/null "$tmpc" 2>&1) ; then
 printf "yes\n"
 else
 printf "no; compiler output follows:\n%s\n" "$output"
-- 
2.10.1


[-- Attachment #3: configure.patch.asc --]
[-- Type: text/plain, Size: 801 bytes --]

-----BEGIN PGP SIGNATURE-----

iQIcBAABCgAGBQJYDpFPAAoJEK8T7CzTdRCcPzAP/RRxjdB7h3OhXEI2t+4Y5Ves
ih45z3NtcMgdgcR8UgCOj4nsknR4TxkeuzO3tlpdALTeZPiLBRvUZj1GqPThq6PG
N3b95fwRD6lbDE546QRZ2yy+CwRZtr8fqBPud8J+L9PgvUuN24tKMwuv6uK7xkwU
1CLAS05gvBRbooOb0aq+9v2blcigJvEZdJL9a7SJweSBCQJ1k0HAln9JNg4QPQBr
71/Vp+EsvB6CPf4JJjsW9gCdUgbapUZ4e+Crrb616EzPDrBbu+QRbOcWcBtXzFlg
qssPQB3qyVzMKt49mDxrlSSAr0hPzgV1520ODEdZbgKKA0NcyVthhWzHxQ5cDfai
+h4FhnCO20WZ1KiBRM5P5DdUFSaXE94Gg/NIih+aOo+y3uznLLrw1fb+byzPGhgy
U1dHywKwaq4QTRYbuJ6FhSqoc5GqPNJzKDg9wkibOdYEGQ9Z3hv6j273Wa1fEnfT
WYlh1grDtjpa5LmdI4jRuo6YD8uyEHiKP6NNMZQFZOA7tqt2C1/2/7wSmXzAk/JK
NRWzFY4bCRuTCHXQbyJjMyLHj726fKLb7GN+e2lPx9MF8amU+MdU16riFEPPeYj0
2HRvlijJ3Fai9QmIJlipHPyAFTxp1ET2paGwwTPcNZHVOyuS40ckLVEFJ6g06BPP
isrOCBx9mZkGLkd2ztnh
=4mAJ
-----END PGP SIGNATURE-----

Re: [PATCH] configure: check whether linker works too

[Rich Felker]

On Mon, Oct 24, 2016 at 06:05:02PM -0500, Laine Gholson wrote:
> As a result of http://www.openwall.com/lists/musl/2016/10/22/3
> it has been shown that musl's current compiler sanity checks
> are inadequate. This patch checks if the compiler as a whole
> works properly, and moves the CPPFLAGS/CFLAGS/LDFLAGS sanity
> checks after the 'compiler works' check.
> 
> Patch and patch signature attached
> 
> Thanks,
>  Laine Gholson

> >From 99fd54d6f2b7c84a02c1ca0d5f1397d709c14313 Mon Sep 17 00:00:00 2001
> From: Laine Gholson <laine.gholson@gmail.com>
> Date: Mon, 24 Oct 2016 17:49:58 -0500
> Subject: [PATCH] configure: check whether linker works too
> 
> and move flag sanity checks to a seperate test
> ---
>  configure | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 707eb12..e7d565c 100755
> --- a/configure
> +++ b/configure
> @@ -236,8 +236,17 @@ printf "%s\n" "$CC"
>  test -n "$CC" || { echo "$0: cannot find a C compiler" ; exit 1 ; }
>  
>  printf "checking whether C compiler works... "
> -echo "typedef int x;" > "$tmpc"
> -if output=$($CC $CPPFLAGS $CFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then
> +echo 'int main(void) { return 0; }' > "$tmpc"
> +if output=$($CC -o /dev/null "$tmpc" 2>&1) ; then
> +printf "yes\n"
> +else
> +printf "no; compiler output follows:\n%s\n" "$output"
> +exit 1
> +fi

As-is, this is not an acceptable change. When bootstrapping a new
toolchain, linking executables does not work because there is no libc
to link to. That's why we only test compiling and not linking.

The only need for a linker in the build process is building a shared
libc.so, and there are some linking tests that use -shared (which can
work without a libc.so, as long as -nostdlib is also being used). It
would be acceptable to add a linking test on which support for shared
library output depends; it should probably cause configure to fail if
--enable-shared was used (shared=yes) and otherwise should cause
shared=auto to become shared=no.

> +printf 'checking whether *FLAGS are sane... '
> +echo 'int main(void) { return 0; }' > "$tmpc"
> +if output=$($CC $CFLAGS $CPPFLAGS $LDFLAGS -o /dev/null "$tmpc" 2>&1) ; then
>  printf "yes\n"
>  else
>  printf "no; compiler output follows:\n%s\n" "$output"

I don't think this test is needed; you'll get a failure at make time
if you included bad flags. If there is a reason to prefer doing such a
test, there need to be separate ones for compiling and linking (since
linking might not work, e.g. if you have a compiler that can't produce
shared libraries).

Rich

Re: [PATCH] configure: check whether linker works too

[Laine Gholson]

[-- Attachment #1: Type: text/plain, Size: 2951 bytes --]

On 10/24/16 18:23, Rich Felker wrote:
> On Mon, Oct 24, 2016 at 06:05:02PM -0500, Laine Gholson wrote:
>> As a result of http://www.openwall.com/lists/musl/2016/10/22/3
>> it has been shown that musl's current compiler sanity checks
>> are inadequate. This patch checks if the compiler as a whole
>> works properly, and moves the CPPFLAGS/CFLAGS/LDFLAGS sanity
>> checks after the 'compiler works' check.
>>
>> Patch and patch signature attached
>>
>> Thanks,
>>  Laine Gholson
>
>> >From 99fd54d6f2b7c84a02c1ca0d5f1397d709c14313 Mon Sep 17 00:00:00 2001
>> From: Laine Gholson <laine.gholson@gmail.com>
>> Date: Mon, 24 Oct 2016 17:49:58 -0500
>> Subject: [PATCH] configure: check whether linker works too
>>
>> and move flag sanity checks to a seperate test
>> ---
>>  configure | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 707eb12..e7d565c 100755
>> --- a/configure
>> +++ b/configure
>> @@ -236,8 +236,17 @@ printf "%s\n" "$CC"
>>  test -n "$CC" || { echo "$0: cannot find a C compiler" ; exit 1 ; }
>>
>>  printf "checking whether C compiler works... "
>> -echo "typedef int x;" > "$tmpc"
>> -if output=$($CC $CPPFLAGS $CFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then
>> +echo 'int main(void) { return 0; }' > "$tmpc"
>> +if output=$($CC -o /dev/null "$tmpc" 2>&1) ; then
>> +printf "yes\n"
>> +else
>> +printf "no; compiler output follows:\n%s\n" "$output"
>> +exit 1
>> +fi
>
> As-is, this is not an acceptable change. When bootstrapping a new
> toolchain, linking executables does not work because there is no libc
> to link to. That's why we only test compiling and not linking.
>
> The only need for a linker in the build process is building a shared
> libc.so, and there are some linking tests that use -shared (which can
> work without a libc.so, as long as -nostdlib is also being used). It
> would be acceptable to add a linking test on which support for shared
> library output depends; it should probably cause configure to fail if
> --enable-shared was used (shared=yes) and otherwise should cause
> shared=auto to become shared=no.
Fixed patch attached: add -nostdlib and -shared, and only check if
linker works if $shared = yes
>
>> +printf 'checking whether *FLAGS are sane... '
>> +echo 'int main(void) { return 0; }' > "$tmpc"
>> +if output=$($CC $CFLAGS $CPPFLAGS $LDFLAGS -o /dev/null "$tmpc" 2>&1) ; then
>>  printf "yes\n"
>>  else
>>  printf "no; compiler output follows:\n%s\n" "$output"
>
> I don't think this test is needed; you'll get a failure at make time
> if you included bad flags. If there is a reason to prefer doing such a
> test, there need to be separate ones for compiling and linking (since
> linking might not work, e.g. if you have a compiler that can't produce
> shared libraries).
I insist on it, some users may use insane CFLAGS values that don't even
work, and it tells them that their CFLAGS are wrong.
>
> Rich
>
Laine Gholson

[-- Attachment #2: configure.patch --]
[-- Type: text/x-patch, Size: 1533 bytes --]

diff --git a/configure b/configure
index 707eb12..c363079 100755
--- a/configure
+++ b/configure
@@ -237,13 +237,49 @@ test -n "$CC" || { echo "$0: cannot find a C compiler" ; exit 1 ; }
 
 printf "checking whether C compiler works... "
 echo "typedef int x;" > "$tmpc"
-if output=$($CC $CPPFLAGS $CFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then
+if output=$($CC -c -o /dev/null "$tmpc" 2>&1) ; then
 printf "yes\n"
 else
 printf "no; compiler output follows:\n%s\n" "$output"
 exit 1
 fi
 
+if test x"$shared" != xno ; then
+  printf "checking whether linker works... "
+  echo "int main(void) { return 0; }" > "$tmpc"
+  if output=$($CC -nostdlib -shared -o /dev/null "$tmpc" 2>&1) ; then
+  printf "yes\n"
+  else
+  if test x"$shared" = xyes ; then
+  printf "no; compiler output follows:\n%s\n" "$output"
+  exit 1
+  else
+  printf "no; shared library disabled\n"
+  shared=no
+  fi
+  fi
+fi
+
+printf "checking whether CFLAGS and CPPFLAGS are sane... "
+echo "typedef int x;" > "$tmpc"
+if output=$($CC $CFLAGS $CPPFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then
+printf "yes\n"
+else
+printf "no; compiler output follows:\n%s\n" "$output"
+exit 1
+fi
+
+if test x"$shared" != xno ; then
+  printf "checking whether LDFLAGS are sane... "
+  echo "int main(void) { return 0; }" > "$tmpc"
+  if output=$($CC $LDFLAGS -nostdlib -shared -o /dev/null "$tmpc" 2>&1) ; then
+  printf "yes\n"
+  else
+  printf "no; compiler output follows:\n%s\n" "$output"
+  exit 1
+  fi
+fi
+
 #
 # Figure out options to force errors on unknown flags.
 #

Re: [PATCH] configure: check whether linker works too

[Szabolcs Nagy]

* Laine Gholson <laine.gholson@gmail.com> [2016-10-24 19:00:55 -0500]:
> +if test x"$shared" != xno ; then
> +  printf "checking whether linker works... "
> +  echo "int main(void) { return 0; }" > "$tmpc"
> +  if output=$($CC -nostdlib -shared -o /dev/null "$tmpc" 2>&1) ; then

a shared lib does not need main (it does not hurt though).

> +  printf "yes\n"
> +  else
> +  if test x"$shared" = xyes ; then
> +  printf "no; compiler output follows:\n%s\n" "$output"
> +  exit 1
> +  else
> +  printf "no; shared library disabled\n"
> +  shared=no
> +  fi

this silently succeeds with default options when the
linker is broken (does not match documented behaviour).

> +  fi
> +fi
> +
> +printf "checking whether CFLAGS and CPPFLAGS are sane... "
> +echo "typedef int x;" > "$tmpc"
> +if output=$($CC $CFLAGS $CPPFLAGS -c -o /dev/null "$tmpc" 2>&1) ; then

there is some copy-paste repetition in these checks,
i think it could be refactored in a cleaner way.
(like tryflag etc)