Re: [PATCH] Redesign pthread in LIB_SPEC for systems without libpthread

[Pavel Chupin <pavel dot v dot chupin at gmail dot com>]

On Fri, Aug 16, 2013 at 3:21 AM, Maxim Kuvyrkov <maxim@kugelworks.com> wrote:
> On 15/08/2013, at 10:49 PM, Alexander Ivchenko wrote:
>
>> Could anybody please take a look? This is important for building gcc for android.
>>
>> ping^4
>
> [Sorry for being cranky]
>
> There is a reason why people are ignoring your patch -- the submission is not very well structured.
>
> First, the subject -- it is not accurate.  "Redesign" implies a big change, you would be better off with a heading like "Fix LIB_SPEC for Android".
>
> Second, you do not fully describe the problem that you are trying to fix.
>
> Third, you do not say how your patch fixes the problem.
>
> Lastly, you do not mention which targets you have encountered the problem on and tested the fix on.  Since you are from Intel, I can guess that you use x86.
>
>
>> >>>> 2013-04-02  Pavel Chupin  <pavel.v.chupin@intel.com>
>> >>>>
>> >>>>         Redesign pthread in LIB_SPEC for systems without libpthread
>> >>>>         * gcc/config/gnu-user.h: Remove pthread from GNU_USER_TARGET_LIB_SPEC
>> >>>>         but keep in default LIB_SPEC
>> >>>>         * gcc/config/linux-android.h: Add pthread to ANDROID_LIB_SPEC
>> >>>>
>> >>>> Is it OK for trunk?
>
>
>> --- a/gcc/config/gnu-user.h
>> +++ b/gcc/config/gnu-user.h
>> @@ -74,11 +74,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>>  #define CPLUSPLUS_CPP_SPEC "-D_GNU_SOURCE %(cpp)"
>>
>>  #define GNU_USER_TARGET_LIB_SPEC \
>> -  "%{pthread:-lpthread} \
>> -   %{shared:-lc} \
>> +   "%{shared:-lc} \
>
> Indentation seems off here, please double-check.
>
>>     %{!shared:%{mieee-fp:-lieee} %{profile:-lc_p}%{!profile:-lc}}"
>>  #undef  LIB_SPEC
>> -#define LIB_SPEC GNU_USER_TARGET_LIB_SPEC
>> +#define LIB_SPEC "%{pthread:-lpthread} " GNU_USER_TARGET_LIB_SPEC
>
> You need to similarly add "%{pthread:-lpthread} " to LIB_SPEC in gcc/config/mips/gnu-user.h.
>
>>
>>  #if defined(HAVE_LD_EH_FRAME_HDR)
>>  #define LINK_EH_SPEC "%{!static:--eh-frame-hdr} "
>> diff --git a/gcc/config/linux-android.h b/gcc/config/linux-android.h
>> index 831a19c..aaf1d34 100644
>> --- a/gcc/config/linux-android.h
>> +++ b/gcc/config/linux-android.h
>> @@ -49,7 +49,8 @@
>>    "%{!frtti:%{!fno-rtti: -fno-rtti}}"
>>
>>  #define ANDROID_LIB_SPEC \
>> -  "%{!static: -ldl}"
>> +  "%{!static: -ldl}" \
>> +  "%{pthread: -lc}"
>
> Add a comment to explain why -pthread option triggers -lc for Android.
>
> Please post the updated patch along with notes on which targets you have tested it.  Test coverage should include at least x86_64-linux-gnu and one of android targets.
>
> Thank you,
>
> --
> Maxim Kuvyrkov
> www.kugelworks.com
>
>

Sorry for not being clear. Thanks for review.

It's late to change subj I think to avoid producing new thread but I
got your point.
Problem is that all 3 Android compilers (arm, x86, mips) are failed to
build on trunk due to libgomp and libatomic configure errors like (arm
example):

configure:14403:
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/xgcc
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/build-gcc-4.9-arm-linux-androideabi/./gcc/
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/bin/
-B/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/lib/
-isystem /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/include
-isystem /tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-arm-linux-androideabi-4.9/arm-linux-androideabi/sys-include
   -o conftest -g -Os -fno-sync-libcalls  -pthread   conftest.c  >&5
/tmp/ndk-pvchupin/build/host-gcc/i686-linux-gnu/temp-binutils-2.23-arm-linux-androideabi/bin/arm-linux-androideabi-ld:
error: cannot find -lpthread

There is no libpthread library in Bionic, pthreads is integrated into libc.
My fix removes -lpthread from GNU_USER_TARGET_LIB_SPEC definition
which is included in both bionic and linux but keeping it in linux
LIB_SPEC.

Please see modified patch attached.
I've built all 3 Android compilers, x86_64 and i686 and ran simple
test with -pthread.

Updated ChangeLog:

2013-08-16  Pavel Chupin  <pavel.v.chupin@intel.com>

        Fix LIB_SPEC for systems without libpthread
        * config/gnu-user.h: Remove pthread from
        GNU_USER_TARGET_LIB_SPEC but keep at LIB_SPEC.
        * config/arm/linux-eabi.h: Add pthread to linux LIB_SPEC.
        * config/i386/linux-common.h: Likewise.
        * config/mips/gnu-user.h: Likewise.
        * config/mips/linux-common.h: Likewise.


Is it OK now?

-- 
Pavel Chupin
Intel Corporation

Attachment: 0001-Fix-LIB_SPEC-for-systems-without-libpthread.patch
Description: Binary data