Re: pgsql: Replace our hacked version of ax_pthread.m4 with latest upstream

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: Replace our hacked version of ax_pthread.m4 with latest upstream
Date: 2015-07-26 07:49:37
Message-ID: 55B49111.9010508@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On 07/26/2015 12:19 AM, Tom Lane wrote:
> I've located another problem in the new improved ax_pthread.m4 script:
> prairiedog is now concluding that it should use -pthread, which leads
> to a whole bunch of build warnings along the lines of
>
> powerpc-apple-darwin8-gcc-4.0.1: unrecognized option '-pthread'
>
> Previously it concluded that it didn't need any special switch.
>
> As near as I can tell, the problem is that this bit of ax_pthread.m4
> is backwards:
>
> # Clang doesn't consider unrecognized options an error unless we specify
> # -Werror. We throw in some extra Clang-specific options to ensure that
> # this doesn't happen for GCC, which also accepts -Werror.
>
> AC_MSG_CHECKING([if compiler needs -Werror to reject unknown flags])
> save_CFLAGS="$CFLAGS"
> ax_pthread_extra_flags="-Werror"
> CFLAGS="$CFLAGS $ax_pthread_extra_flags -Wunknown-warning-option -Wsizeof-array-argument"
> AC_COMPILE_IFELSE([AC_LANG_PROGRAM([int foo(void);],[foo()])],
> [AC_MSG_RESULT([yes])],
> [ax_pthread_extra_flags=
> AC_MSG_RESULT([no])])
> CFLAGS="$save_CFLAGS"
>
> AFAICS, the only way that ax_pthread_extra_flags ends up containing -Werror
> is if this compile attempt *succeeds*, which surely ought not happen ever
> with any compiler; it certainly won't on any of mine. Thus, we never
> actually apply -Werror and so the subsequent thread-switch testing
> completely fails to notice if the compiler merely emits warnings.

Hmm. I don't think that's backwards. It's a strange way of testing "does
the compiler understand -Werror, and is it not GCC?". It succeeds on
clang, as the comment says:

~$ clang a.c -c -Werror -Wunknown-warning-option -Wsizeof-array-argument
[success]

The problem it's trying to work-around is that allegedly clang does not
throw an error if -pthread is given on command-line on a platform where
it's not required, merely a warning. That workaround is too narrow, as
we have the exact same problem on prairiedog, with gcc. (But as you
noted, -Werror wouldn't help with older gcc versions anyway)

Googling around, I found a ticket in the autoconf bug tracker for this:
https://savannah.gnu.org/patch/?8186. The discussion died out in 2013,
but it seems that the latest idea there was to change the order the
flags are tested on Darwin, so that "none" is tested before "-pthread".
That should fix the issue, and hopefully not create other problems on
other Darwin variants. So I suggest that we do:

@@ -160,7 +165,7 @@ case ${host_os} in
;;

darwin*)
- ax_pthread_flags="-pthread $ax_pthread_flags"
+ ax_pthread_flags="none -pthread $ax_pthread_flags"
;;
esac

It's sad that we have to hack this script again, I hoped we could use
the upstream version as is. I'll comment on the upstream ticket, let's
see if we can get the upstream version fixed too.

> And on top of that, at least with the versions of gcc I have laying about,
> an unknown or misspelled -p option does not result in nonzero exit status,
> whether or not you say -Werror:
>
> $ touch foo.c
> $ gcc -pthread -c foo.c
> $ gcc -pzthread -c foo.c
> gcc: unrecognized option '-pzthread'
> gcc: unrecognized option '-pzthread'
> $ echo $?
> 0
> $ gcc -Werror -pzthread -c foo.c
> gcc: unrecognized option '-pzthread'
> gcc: unrecognized option '-pzthread'
> $ echo $?
> 0
>
> (same results on gcc 4.0.1 or 4.4.7)
>
> Therefore, adding -Werror would fail to produce the desired results in the
> subsequent thread-flag checks even if the check about whether to add it
> had been done correctly.

This was fixed somewhere between 4.4 and 4.6. But yeah, it means that we
cannot rely on it.

- Heikki

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2015-07-26 15:12:37 pgsql: Allow to push down clauses from HAVING to WHERE when grouping se
Previous Message Joe Conway 2015-07-26 00:47:11 pgsql: Improve markup for row_security.