Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Mon, 24 Oct 2016 19:00:55 -0500
From: Laine Gholson <laine.gholson@...il.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] configure: check whether linker works too

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@...il.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

View attachment "configure.patch" of type "text/x-patch" (1534 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.