patchGNU Autoconf Archive - Patches: patch #8186, ax_pthread adds -pthread when it...

 
 

patch #8186: ax_pthread adds -pthread when it should not on OSX when compiling with clang

Submitter:  None
Submitted:  Sat 21 Sep 2013 01:48:31 AM UTC
   
 
Priority:  5 - Normal Status:  Done
Assigned to:  simons Originator Email:  -email is unavailable-
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 15 Jun 2016 04:23:15 AM UTC, comment #25: 

Hi Bernd,

The _REENTRANT macro is supposed to be #defined as a side effect of the -pthread option when passed to GCC on Linux. This is part of how AX_PTHREAD determines that it has found the correct option.

As a workaround for now, you may call AC_DEFINE(_REENTRANT) before AX_PTHREAD, so that the macro is unconditionally defined. This may interfere with the pthread check, but it is better than the check failing.

I will need you to file a bug against the blackfin and arc compilers. Unless they have a good reason, it is incorrect for them not to #define _REENTRANT when the -pthread option is given. This is the standard behavior for GCC on Linux, because support for POSIX threads implies that re-entrant function variants should be used, which is specified by this macro.

(This #define is supposed to be in the GCC machine spec files, and as these architectures are more specialized, it is plausible that they have not received as much attention on minor details like this.)

Daniel Richard G. <iskunk>
Mon 13 Jun 2016 07:26:42 PM UTC, comment #24: 

Hi,

the latest version bump of the libmicrohttpd package to 0.9.50 included bumping the serial of ax_pthread.m4 from 21 to 23.

This bump includes changes to the way configure looks for pthread support.

On Linux the macro _REENTRANT is now being searched for:
https://github.com/peti/autoconf-archive/commit/39683064bbccb4008f239262cb681a970bf53603#diff-851a07ee321dc42003ac94cf4a9628faR211
which fails at least on arc and blackfin.

Before the autoconf update _REENTRANT was not searched on Linux:
https://github.com/peti/autoconf-archive/commit/39683064bbccb4008f239262cb681a970bf53603#diff-851a07ee321dc42003ac94cf4a9628faL273

This leads to problems detected by the buildroot autobuilders on blackfin and arc:

http://autobuild.buildroot.net/results/593/593b5050473a83a9ddcada9de39f8f14ab38d554//libmicrohttpd-0.9.50/config.log

configure:13932: checking whether pthreads work with -pthread
configure:14026: /home/test/autobuild/instance-3/output/host/usr/bin/bfin-linux-uclibc-gcc -std=gnu99 -o conftest -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os  -pthread -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  conftest.c   >&5
conftest.c:30:26: error: #error "_REENTRANT must be defined"


http://autobuild.buildroot.net/results/ff0/ff058a378b91740b5b399f32a6b375fbc3c8df06//libmicrohttpd-0.9.50/config.log

configure:13984: checking for the pthreads library -lpthread
configure:14026: /home/test/autobuild/instance-0/output/host/usr/bin/arc-linux-gcc -std=gnu99 -o conftest -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -matomic -Os   -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  conftest.c -lpthread  >&5
conftest.c:30:26: error: #error "_REENTRANT must be defined"

Disabling ax_pthread_check_macro="_REENTRANT" for Linux fixed the problem.

Bernd Kuhls <bkuhls>
Mon 29 Feb 2016 08:24:18 PM UTC, comment #23: 

Oh, Peter, I wish you'd asked first :>

Draft 3 breaks on FreeBSD. It's an easy fix, but I was hoping to come back here with that and a bit more.

Draft 4 is attached. It has that change, and the serial is bumped to 23.

That aside, it's just as well to push this. The macro isn't quite yet what I'd call "done," but it's certainly past the point of diminishing returns by now.

(file #36513)

Daniel Richard G. <iskunk>
Mon 29 Feb 2016 10:24:16 AM UTC, comment #22: 

I've pushed the draft-3 version to Git in 566ebc30b1f82d9e4ae20b336c22ca837fbd44e3. I hope that's alright with everyone?

Peter Simons <simons>
Group administrator
Tue 09 Feb 2016 08:33:28 AM UTC, comment #21: 

Hi everyone,

I've been working on this macro again lately, and finally have a few things figured out---not least the ongoing Clang/Darwin issue.

First, I would like to clarify that even though Clang explicitly does not require -pthread at link time, and gives no trouble on Darwin if this is not specified at compile time, this flag is not being silently ignored during compilation.

Specifically, -pthread causes _REENTRANT to be defined, which by convention is used to enable re-entrant declarations of variables/functions. It does happen that on Darwin, the system headers do not conditionalize anything on _REENTRANT, so this symbol has no effect there. But that is a property of Apple's headers, not of Clang, and third-party headers could very well make use of _REENTRANT. It's worth noting that "gcc -pthread" on Darwin defines this too.

So PTHREAD_CFLAGS="-pthread" is the correct solution on Darwin (and for Clang in general) after all. As for the warning that [older versions of] Clang gives when linking with this flag, it can be quashed with one of the following flags:

    -Qunused-arguments
    -Wno-unused-command-line-argument

AX_PTHREAD will check if the compiler is Clang, and if it's a Clang that gives the link-time warning, the macro will add either of the above to PTHREAD_CFLAGS to kill the warning. So projects that choose to build with -Werror won't need to carve out an exception for this.

All this is handled in new special-case code for Clang. What we have with Clang is a compiler that's sufficiently different from most others, and whose popularity as a free project will only increase with time, that handling it separately and specifically makes sense.

That's not the only change I have here, however. There's a grab bag of other improvements, gathered up over the last three years that this macro has been used in my product development work. Here's a (hopefully exhaustive) summary:

  • Minor doc/comment text tweaks


  • Formally require AC_PROG_CC and AC_PROG_SED


  • Use properly namespaced shell variable names


  • Made 'test "x$foo" = "xbar"' syntax consistent


  • Replaced obsolete AC_TRY_LINK_FUNC() call with AC_LINK_IFELSE()


  • Try -kthread and -llthread only on FreeBSD, as the notes indicate these are only used there (but I can't find a version of FreeBSD that uses either of these; anyone know?)


  • Added some observations from Solaris, Tru64 and HP-UX to the threads options notes


  • Added special-case handling for HP-UX


  • Added rudimentary support for z/OS Unix System Services ("openedition")


  • On Solaris, try the "-mt" + "-lpthread" combination first, and moved -pthreads to a general GCC handler


  • Removed the Darwin special case, as it is now handled as general GCC


  • Added a mechanism to verify correct pthreads support on certain platforms by checking that _REENTRANT or _THREAD_SAFE is defined by the option we're testing. I'd like to see what other platforms can be supported here, because IMO it's a good way of sussing out if we're really in "pthreads mode" or not


  • Totally rewrote the Clang support!


  • Added a check: Is the compiler Clang?


  • If compiling with Clang, use -pthread; don't bother with the general check, because (AFAIK) Clang has never supported any other option


  • If Clang give a warning when -pthread is specified in a link-only invocation, then find an option that will shut it up---implemented by temporarily redefining the ac_link() shell function


  • Use AC_CACHE_CHECK() for the PTHREAD_CREATE_JOINABLE, PTHREAD_PRIO_INHERIT and special-flags check, plus extra variables to ensure idempotency



Everyone who's interested, please review these changes, try them out, kick the tires, let me know if anything breaks. Comments and feedback will be appreciated. (I'll need more third-party testing than previous iterations as a few of my older exotic test systems have bit the dust in recent years.)

(file #36281)

Daniel Richard G. <iskunk>
Tue 04 Aug 2015 04:57:04 AM UTC, comment #20: 

Hi Heikki,

-D_REENTRANT and -D_THREAD_SAFE can be removed from your build scripts; AX_PTHREAD now takes care of adding these to PTHREAD_CFLAGS on certain platforms. It's possible that some platforms will then lose one/both of these flags (do note that they can be defined implicitly by -mt et al.) but this is correct behavior if the system headers do not make use of those cpp symbols.

-D_POSIX_PTHREAD_SEMANTICS is, of course, not handled by AX_PTHREAD. It appears to be applicable only to Solaris. Should I add this to the special-flags section of the macro?

The build farms logs are extraordinarily helpful! Thank you for running this draft version through there!

I see a number of Clang builds now use no pthreads option flag at all (though retaining -D flags). None of those builds broke, but please let me know if there is any negative effect from dropping the options.

smew is running a fairly old version of Clang that my draft did not handle correctly. I'm surprised that build didn't break. I've fixed the issue there.

Daniel Richard G. <iskunk>
Sun 02 Aug 2015 01:18:33 PM UTC, comment #19: 

Thanks Daniel. I've pushed that version to PostgreSQL git master, and the buildfarm is happy with it. I collected the PTHREAD_CFLAGS values chosen on each platform in the buildfarm, before and after this change, and they all look good to me.

For reference, the buildfarm results can be seen at http://pgbuildfarm.org/cgi-bin/show_status.pl, and my comparison of the chosen PTHREAD_CFLAGS can be seen at: http://www.postgresql.org/message-id/55BE176F.3070901@iki.fi

Heikki Linnakangas <hlinnaka>
Thu 30 Jul 2015 06:41:45 AM UTC, comment #18: 

The fix for this issue specifically is to remove the Darwin special case, but my changes go beyond that.

The Clang check is still needed to support older versions properly, but that's minimal. (I've also made a fix there, to account for the saner behavior of newer Clang releases.)

I've attached a "draft2" revision of the macro, which is my current bleeding-edge code. Diffing will show you everything that's going on in there (I will, of course, have to write up a proper changelog entry).

Please give this a try, Heikki and anyone else still interested, and let me know if you see any problems. I've tested it across the breadth of systems available to me, and everything seems to check out so far.

(file #34553)

Daniel Richard G. <iskunk>
Wed 29 Jul 2015 07:37:36 AM UTC, comment #17: 

Thanks Daniel! I don't have direct access to the machine, but I can commit a modified version to our git repository, and the Postgres buildfarm will run it on many platforms.

Which fix are you planning to apply for this?

1) Remove the Darwin-special case altogether, or

2) Put "none" before "-pthread" on Darwin

And were you planning to revert the clang-specific check, or is that still required? Could you post the exact version of the script you'd like me to test, please?

Heikki Linnakangas <hlinnaka>
Wed 29 Jul 2015 07:18:08 AM UTC, comment #16: 

Hello Heikki,

Another developer recently contacted me as well about this just a day before your inquiry, and I have been working on the macro lately. I have a number of changes that I've been sitting on, not least the tweak worked out with Brian here, and haven't had the wherewithal to get those committed for some time.

I have here what appears to be a Darwin 11.4.2 system that I will be testing on. Would you be willing to give the revised macro a try on your machine?

Daniel Richard G. <iskunk>
Sun 26 Jul 2015 08:50:32 AM UTC, comment #15: 

Any news on this? We're seeing the same issue that Brian reported, after updating to the latest version of acx_pthread.m4, in the PostgreSQL buildfarm. It happens on a Darwin 8.11.0 system running on PowerPC, using gcc.

Heikki Linnakangas <hlinnaka>
Thu 21 Nov 2013 01:20:40 AM UTC, comment #14: 

Brian and Alistair,

Thanks for checking back in. I'm currently testing the macro in my employer's stable of variegated Unix systems, and once it checks out, I'll post an update here for further testing.

Daniel Richard G. <iskunk>
Wed 20 Nov 2013 03:23:04 PM UTC, comment #13: 

Daniel,

Removing the darwin special case did indeed give the correct result.

Thanks for this. I'm happy to test anything you need.

Alistair King <alistairking>
Wed 20 Nov 2013 12:30:34 PM UTC, comment #12: 

Sorry, emails on this have been being caught by my spam filter.

By "cc", I was referring to the case where:

CC=cc

At which point you need to dig a bit deeper in order to determine which compiler is being used.

I have faint memory of this being an issue on a few other platforms where cc could be an alias to either gcc or some other vendor's C compiler.

Brian Aker <brianaker>
Wed 20 Nov 2013 07:19:59 AM UTC, comment #11: 

Alistair,

If you remove the special case for Darwin on line 162 of the macro file, does it give the correct result?

I'm doing some heavy maintenance on this macro, and will be happy to ensure that it works right on newer MacOS X systems.

Daniel Richard G. <iskunk>
Tue 19 Nov 2013 04:28:03 PM UTC, comment #10: 

I am seeing the same issue that Brian mentioned.
Using this patch on OSX 10.9, with CC set to either gcc or clang (it appears they are both llvm front-ends):

alistair@gibi:~$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk/usr/include/c++/4.2.1
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin13.0.0
Thread model: posix

alistair@gibi:~$ clang --version
Apple LLVM version 5.0 (clang-500.2.79) (based on LLVM 3.3svn)
Target: x86_64-apple-darwin13.0.0
Thread model: posix

When I use the patched ax_pthread macro it adds -pthread to CFLAGS.
This works just fine during the compilation phase, but when linking yields this (misleading) warning:

clang: warning: argument unused during compilation: '-pthread'

The fix suggested by Brian does indeed make this go away.
My guess as to why this fixes the problem is because while clang will accept -pthread during compilation, it is not needed; whereas during linking it throws a warning if it is used.

From man pthread(3):
INSTALLATION
     The default system libraries include pthread functions.  No additional libraries or CFLAGS are necessary to use these interfaces.

Hope this helps.

Alistair King <alistairking>
Sun 27 Oct 2013 10:28:47 AM UTC, comment #9: 

Brian, is there still a problem with this macro on your setup? Or do you think this issue can be closed?

Peter Simons <simons>
Group administrator
Sat 19 Oct 2013 06:16:42 AM UTC, comment #8: 

Brian,

What do you mean by "if you call it via CC?" How was the macro failing?

Also, could you explain the intent behind changing the order of the flags that are tested? What was the incorrect result it was giving before, and the correct result that your change now returns?

Daniel Richard G. <iskunk>
Fri 18 Oct 2013 06:53:19 AM UTC, comment #7: 

And I did a bad copy and paste...


diff m4/ax_pthread.m4  ~/Projects/ddm4/m4/ax_pthread.m4
85c85
< #serial 21
---

> #serial 22

163c163
<         ax_pthread_flags="-pthread $ax_pthread_flags"
---

>         ax_pthread_flags="none -pthread $ax_pthread_flags"



Adjust the order for OSX.

Brian Aker <brianaker>
Fri 18 Oct 2013 06:41:52 AM UTC, comment #6: 

Hi!

Sorry about not logging in when I left the original bug report, I was in a hurry working over someone's shoulder and just copy and pasted what I had done :)

This patch works if you call clang via "clang" but if you call it via CC it will fail. I took what you all did in trunk and applied the following:


diff --git a/m4/ax_pthread.m4 b/m4/ax_pthread.m4
index 4537934..ebea7fb 100644
--- a/m4/ax_pthread.m4
+++ b/m4/ax_pthread.m4
@@ -123,7 +123,7 @@ fi
 # which indicates that we try without any flags at all, and "pthread-config"
 # which is a program returning the flags for the Pth emulation library.
 
-ax_pthread_flags="none pthreads -Kthread -kthread lthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config"
+ax_pthread_flags="pthreads none -Kthread -kthread lthread -pthread -pthreads -mthreads pthread --thread-safe -mt pthread-config"
 
 # The ordering is (sometimes) important.  Some notes on the
 # individual items follow:
@@ -160,7 +160,7 @@ case ${host_os} in
         ;;
 
         darwin*)
-        ax_pthread_flags="-pthread $ax_pthread_flags"
+        ax_pthread_flags="none -pthread $ax_pthread_flags"
         ;;
 esac
 
Cheers,
    -Brian

Brian Aker <brianaker>
Thu 03 Oct 2013 04:58:28 PM UTC, comment #5: 

FYI, I have pushed Daniel's patch in revision dd94691.

The original poster may not even notice that this update is available, I'm afraid, because he or she didn't leave an e-mail address with this issue and thus cannot be notified of the new development.

Peter Simons <simons>
Group administrator
Fri 27 Sep 2013 05:01:50 AM UTC, comment #4: 

Okay, attached is a patch against git master that adds support for Clang to AX_PTHREAD, and a copy of the patched macro. Can the original poster give this a try?

(And if anyone else here uses Clang, your input will be much appreciated...)

(file #29240, file #29241)

Daniel Richard G. <iskunk>
Sat 21 Sep 2013 04:36:10 PM UTC, comment #3: 

I haven't worked with Clang yet, but from a bit of searching around, it seems that -pthread will either cause the compiler to complain harmlessly, or be recognized as a proper option in newer versions.

What's the specific failure mode that the patch intends to avoid? Is it a hard error? Warning noise? It would be helpful to see some copy-and-pasted messages from the compiler.

Daniel Richard G. <iskunk>
Sat 21 Sep 2013 07:43:11 AM UTC, comment #2: 

Hey Steven, hi Daniel, do you agree with this patch?

Peter Simons <simons>
Group administrator
Sat 21 Sep 2013 04:42:09 AM UTC, comment #1: 

Surely a better test for whether the compiler is clang is needed, for example in MacPorts clang is installed as clang-mp-3.3 etc... and with this check this compiler will not be identified as clang.

In one of my projects I use the following to see if clang is being used:

  # check for clang
  AS_IF([test "x$GCC" = xyes],
    [AS_IF([test "`$CC -v 2>&1 | grep -c 'clang'`" != "0"],[CLANG_CC=1])],
    [CLANG_CC=])
  AC_SUBST(CLANG_CC)

So could something like that be used here?

Anonymous
Sat 21 Sep 2013 01:48:31 AM UTC, original submission:  

@@ -160,7 +160,11 @@ case ${host_os} in
         ;;
 
         darwin*)
-        ax_pthread_flags="-pthread $ax_pthread_flags"
+        if test "$CC" = "clang"; then
+            ax_pthread_flags="$ax_pthread_flags"
+        else
+            ax_pthread_flags="-pthread $ax_pthread_flags"
+        fi
         ;;
 esac


Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #36513:  ax_pthread-draft4.m4 added by iskunk (18KiB - application/x-m4)
file #36281:  ax_pthread-draft3.m4 added by iskunk (18KiB - application/x-m4)
file #34553:  ax_pthread-draft2.m4 added by iskunk (16KiB - application/x-m4)
file #29240:  ax_pthread_clang.patch added by iskunk (5KiB - text/x-patch - Patch to add rudimentary Clang support to AX_PTHREAD)
file #29241:  ax_pthread-draft1.m4 added by iskunk (13KiB - application/x-m4 - Patch to add rudimentary Clang support to AX_PTHREAD)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bkuhls (Posted a comment)
  • -email is unavailable- added by hlinnaka (Posted a comment)
  • -email is unavailable- added by alistairking (Posted a comment)
  • -email is unavailable- added by brianaker (Posted a comment)
  • -email is unavailable- added by iskunk (Posted a comment)
  • -email is unavailable- added by simons (Posted a comment)
  • -email is unavailable- added by simons
  • -email is unavailable- added by simons
  •  

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-29 iskunk Attached File- Added ax_pthread-draft4.m4, #36513
    2016-02-09 iskunk Attached File- Added ax_pthread-draft3.m4, #36281
    2015-07-30 iskunk Attached File- Added ax_pthread-draft2.m4, #34553
    2013-11-01 simons StatusReady For Test Done
        Open/ClosedOpen Closed
    2013-10-03 simons StatusNeed Info Ready For Test
    2013-09-27 iskunk Attached File- Added ax_pthread_clang.patch, #29240
        Attached File- Added ax_pthread-draft1.m4, #29241
    2013-09-21 simons StatusNone Need Info
        Assigned toNone simons
    2013-09-21 simons Carbon-Copy- Added steven g. johnson <stevenj@alum.mit.edu>
        Carbon-Copy- Added daniel richard g. <skunk@iskunk.org>

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code