Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Fri, Aug 17, 2012 at 2:42 AM, Chupin, Pavel V wrote: > Submitted patch here: https://android-review.googlesource.com/#/c/41705 > > -- Pavel > link.h has been added to AOSP. I am closing PR 54209. Thanks. -- H.J.
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Tue, Aug 14, 2012 at 4:27 PM, Ian Lance Taylor wrote: > On Tue, Aug 14, 2012 at 3:47 PM, Maxim Kuvyrkov > wrote: > >> I think this patch will break MIPS Android build due to mismatch of >> ElfW(type) when _MIPS_SZPTR == 64. I think the right way to fix this is to >> make Bionic export link.h or already-existing linker.h, but I differ to Ian >> for final judgement. > > I think it would be better to export . I don't know how > feasible that is or how long it would take to become available. Pavel, how long does it take to export for Android/x86? >> FWIW, I'm OK with using hard-coded definitions if link.h is absent, and >> using definitions from link.h if it is there. I.e., >> >> #ifdef HAVE_LINK_H >> # include >> #else >> >> #endif > > This is conceptually fine as long as we are clear that we are testing > for the presence of link.h on the target, not the host. It can be > hard for libgcc to reliably test for the presence of target-specific > header files. > That is also my concern. -- H.J.
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Tue, Aug 14, 2012 at 3:47 PM, Maxim Kuvyrkov wrote:
> On 15/08/2012, at 7:39 AM, H.J. Lu wrote:
>
>> On Tue, Aug 14, 2012 at 12:38 PM, H.J. Lu wrote:
>>> On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote:
On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote:
>
> Bionic C library doesn't provide link.h.
Does Bionic provide dl_iterate_phdr? If it does, I'll just note in
passing that it would be straightforward to simply incorporate the
required types and constants in unwind-dw2-fde-dip.c directly, and
avoid the #include. If it doesn't, then of course nothing will make
this code work correctly.
>>>
>>> dl_iterate_phdr is provided in libdl.so, which is always linked with
>>> dynamic executables:
>>>
>>> #define ANDROID_LIB_SPEC \
>>> "%{!static: -ldl}"
>>>
>>>
>>> This patch fixes Android/x86 build on trunk. OK to install?
>
> [Adding David Turner to CC as the main Bionic expert. Also reattaching HJ's
> current patch so that David can easily look at it. Link to the bug report:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54209]
>
> I think this patch will break MIPS Android build due to mismatch of
> ElfW(type) when _MIPS_SZPTR == 64. I think the right way to fix this is to
> make Bionic export link.h or already-existing linker.h, but I differ to Ian
> for final judgement.
>
> FWIW, I'm OK with using hard-coded definitions if link.h is absent, and using
> definitions from link.h if it is there. I.e.,
>
> #ifdef HAVE_LINK_H
> # include
> #else
>
> #endif
>
> This would allow Bionic to eventually catch up and provide link.h uniformly
> across all targets.
>
> I've looked into latest Android NDK distribution, and the situation with
> link.h is not uniform across targets. ARM and x86 don't have link.h, while
> MIPS does:
> ---
> /*
>For building unwind-dw2-fde-glibc.c for MIPS frame unwinding,
>we need to have that defines struct dl_phdr_info,
>ELFW(type), and dl_iterate_phdr().
> */
>
> #include
> #include
>
> struct dl_phdr_info
> {
> Elf32_Addr dlpi_addr;
> const char *dlpi_name;
> const Elf32_Phdr *dlpi_phdr;
> Elf32_Half dlpi_phnum;
> };
>
> #if _MIPS_SZPTR == 32
> #define ElfW(type) Elf32_##type
> #elif _MIPS_SZPTR == 64
> #define ElfW(type) Elf64_##type
> #endif
>
> int
> dl_iterate_phdr(int (*cb)(struct dl_phdr_info *info, size_t size, void *data),
> void *data);
> ---
>
> I'm not 100% sure where the above link.h comes from for MIPS, but since it's
> not present in Bionic sources, my guess is kernel's arch/mips/include
> directory. Checking ... No, not from the kernel sources. Hm...
>
> --
Bionic is a 32-bit library. I don't know how _MIPS_SZPTR == 64 works
with Bionic on mips.
--
H.J.
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Tue, Aug 14, 2012 at 3:47 PM, Maxim Kuvyrkov wrote: > I think this patch will break MIPS Android build due to mismatch of > ElfW(type) when _MIPS_SZPTR == 64. I think the right way to fix this is to > make Bionic export link.h or already-existing linker.h, but I differ to Ian > for final judgement. I think it would be better to export . I don't know how feasible that is or how long it would take to become available. > FWIW, I'm OK with using hard-coded definitions if link.h is absent, and using > definitions from link.h if it is there. I.e., > > #ifdef HAVE_LINK_H > # include > #else > > #endif This is conceptually fine as long as we are clear that we are testing for the presence of link.h on the target, not the host. It can be hard for libgcc to reliably test for the presence of target-specific header files. Ian
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On 15/08/2012, at 7:39 AM, H.J. Lu wrote:
> On Tue, Aug 14, 2012 at 12:38 PM, H.J. Lu wrote:
>> On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote:
>>> On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote:
Bionic C library doesn't provide link.h.
>>>
>>> Does Bionic provide dl_iterate_phdr? If it does, I'll just note in
>>> passing that it would be straightforward to simply incorporate the
>>> required types and constants in unwind-dw2-fde-dip.c directly, and
>>> avoid the #include. If it doesn't, then of course nothing will make
>>> this code work correctly.
>>>
>>
>> dl_iterate_phdr is provided in libdl.so, which is always linked with
>> dynamic executables:
>>
>> #define ANDROID_LIB_SPEC \
>> "%{!static: -ldl}"
>>
>>
>> This patch fixes Android/x86 build on trunk. OK to install?
[Adding David Turner to CC as the main Bionic expert. Also reattaching HJ's
current patch so that David can easily look at it. Link to the bug report:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54209]
I think this patch will break MIPS Android build due to mismatch of ElfW(type)
when _MIPS_SZPTR == 64. I think the right way to fix this is to make Bionic
export link.h or already-existing linker.h, but I differ to Ian for final
judgement.
FWIW, I'm OK with using hard-coded definitions if link.h is absent, and using
definitions from link.h if it is there. I.e.,
#ifdef HAVE_LINK_H
# include
#else
#endif
This would allow Bionic to eventually catch up and provide link.h uniformly
across all targets.
I've looked into latest Android NDK distribution, and the situation with link.h
is not uniform across targets. ARM and x86 don't have link.h, while MIPS does:
---
/*
For building unwind-dw2-fde-glibc.c for MIPS frame unwinding,
we need to have that defines struct dl_phdr_info,
ELFW(type), and dl_iterate_phdr().
*/
#include
#include
struct dl_phdr_info
{
Elf32_Addr dlpi_addr;
const char *dlpi_name;
const Elf32_Phdr *dlpi_phdr;
Elf32_Half dlpi_phnum;
};
#if _MIPS_SZPTR == 32
#define ElfW(type) Elf32_##type
#elif _MIPS_SZPTR == 64
#define ElfW(type) Elf64_##type
#endif
int
dl_iterate_phdr(int (*cb)(struct dl_phdr_info *info, size_t size, void *data),
void *data);
---
I'm not 100% sure where the above link.h comes from for MIPS, but since it's
not present in Bionic sources, my guess is kernel's arch/mips/include
directory. Checking ... No, not from the kernel sources. Hm...
--
Maxim Kuvyrkov
CodeSourcery / Mentor Graphics
>>
>> 2012-08-14 H.J. Lu
>>
>>PR bootstrap/54209
>>* unwind-dw2-fde-dip.c (dl_phdr_info): New struct for Bionic C
>>library.
>>(ElfW): New macro for Bionic C library.
>>Don't include for Bionic C library.
>
> Wrong patch. Here is the right one.
>
> --
> H.J.
>
gcc-pr54209.patch
Description: Binary data
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Tue, Aug 14, 2012 at 12:38 PM, H.J. Lu wrote:
> On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote:
>> On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote:
>>>
>>> Bionic C library doesn't provide link.h.
>>
>> Does Bionic provide dl_iterate_phdr? If it does, I'll just note in
>> passing that it would be straightforward to simply incorporate the
>> required types and constants in unwind-dw2-fde-dip.c directly, and
>> avoid the #include. If it doesn't, then of course nothing will make
>> this code work correctly.
>>
>
> dl_iterate_phdr is provided in libdl.so, which is always linked with
> dynamic executables:
>
> #define ANDROID_LIB_SPEC \
> "%{!static: -ldl}"
>
>
> This patch fixes Android/x86 build on trunk. OK to install?
>
> Thanks.
>
> --
> H.J.
>
> 2012-08-14 H.J. Lu
>
> PR bootstrap/54209
> * unwind-dw2-fde-dip.c (dl_phdr_info): New struct for Bionic C
> library.
> (ElfW): New macro for Bionic C library.
> Don't include for Bionic C library.
Wrong patch. Here is the right one.
--
H.J.
gcc-pr54209.patch
Description: Binary data
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote:
> On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote:
>>
>> Bionic C library doesn't provide link.h.
>
> Does Bionic provide dl_iterate_phdr? If it does, I'll just note in
> passing that it would be straightforward to simply incorporate the
> required types and constants in unwind-dw2-fde-dip.c directly, and
> avoid the #include. If it doesn't, then of course nothing will make
> this code work correctly.
>
dl_iterate_phdr is provided in libdl.so, which is always linked with
dynamic executables:
#define ANDROID_LIB_SPEC \
"%{!static: -ldl}"
This patch fixes Android/x86 build on trunk. OK to install?
Thanks.
--
H.J.
2012-08-14 H.J. Lu
PR bootstrap/54209
* unwind-dw2-fde-dip.c (dl_phdr_info): New struct for Bionic C
library.
(ElfW): New macro for Bionic C library.
Don't include for Bionic C library.
gcc-pr54157.patch
Description: Binary data
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, Aug 9, 2012 at 4:01 PM, H.J. Lu wrote: > On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote: >> On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote: >>> >>> Bionic C library doesn't provide link.h. >> >> Does Bionic provide dl_iterate_phdr? If it does, I'll just note in >> passing that it would be straightforward to simply incorporate the >> required types and constants in unwind-dw2-fde-dip.c directly, and >> avoid the #include. If it doesn't, then of course nothing will make >> this code work correctly. > > That is a good idea. Pavel, can you look into it? You may find libiberty/simple-object-elf.c to be a useful guide. Ian
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, Aug 9, 2012 at 3:17 PM, Ian Lance Taylor wrote: > On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote: >> >> Bionic C library doesn't provide link.h. > > Does Bionic provide dl_iterate_phdr? If it does, I'll just note in > passing that it would be straightforward to simply incorporate the > required types and constants in unwind-dw2-fde-dip.c directly, and > avoid the #include. If it doesn't, then of course nothing will make > this code work correctly. That is a good idea. Pavel, can you look into it? Thanks. -- H.J.
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, Aug 9, 2012 at 9:39 AM, H.J. Lu wrote: > > Bionic C library doesn't provide link.h. Does Bionic provide dl_iterate_phdr? If it does, I'll just note in passing that it would be straightforward to simply incorporate the required types and constants in unwind-dw2-fde-dip.c directly, and avoid the #include. If it doesn't, then of course nothing will make this code work correctly. Ian
RE: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, 9 Aug 2012, Fu, Chao-Ying wrote: > How about this patch? Just enable it for MIPS that provides link.h in > Android NDK. > Thanks a lot! Please don't put this sort of architecture conditional in an architecture-independent source file. In this case it should be fine for libgcc's configure to try compiling a file that #includes (obviously, make sure the configure test gets the right results both when it's present and when it's absent), and use the results of that configure test instead of defined(__mips__). (In a bootstrap where libc headers aren't yet present, inhibit_libc should be defined anyway to disable those libgcc features depending on system headers from libc.) -- Joseph S. Myers jos...@codesourcery.com
RE: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
> > Where does mips link.h come from? I didn't see it in AOSP > Bionic C library. > > -- > H.J. > It's from development/ndk/platforms/android-9/arch-mips/include/link.h from AOSP checkout. Regards, Chao-ying
Re: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
On Thu, Aug 9, 2012 at 11:11 AM, Fu, Chao-Ying wrote: >> > Hi, >> > >> > Bionic C library doesn't provide link.h. This patch >> reverts revision >> > 186788: >> > >> > http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00740.html >> > >> > OK to install? >> > >> > Thanks. >> > >> > H.J. >> > --- >> > 2012-08-09 H.J. Lu >> > >> > PR bootstrap/54209 >> > * unwind-dw2-fde-dip.c (USE_PT_GNU_EH_FRAME): Don't define for >> > Bionic C library. >> > >> > diff --git a/libgcc/unwind-dw2-fde-dip.c >> b/libgcc/unwind-dw2-fde-dip.c >> > index 92f8ab5..f57dc8c 100644 >> > --- a/libgcc/unwind-dw2-fde-dip.c >> > +++ b/libgcc/unwind-dw2-fde-dip.c >> > @@ -54,11 +54,6 @@ >> > #endif >> > >> > #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ >> > -&& defined(__BIONIC__) >> > -# define USE_PT_GNU_EH_FRAME >> > -#endif >> > - >> > -#if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ >> > && defined(__FreeBSD__) && __FreeBSD__ >= 7 >> > # define ElfW __ElfN >> > # define USE_PT_GNU_EH_FRAME >> > >> >> How about this patch? Just enable it for MIPS that >> provides link.h in Android NDK. >> Thanks a lot! >> >> Regards, >> Chao-ying >> >> Index: unwind-dw2-fde-dip.c >> === >> --- unwind-dw2-fde-dip.c(revision 190260) >> +++ unwind-dw2-fde-dip.c(working copy) >> @@ -55,6 +55,7 @@ >> >> #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ >> && defined(__BIONIC__) >> +&& defined(__mips__) >> # define USE_PT_GNU_EH_FRAME >> #endif >> > > Sorry, I forgot \ in the previous patch. > Ex: > Index: unwind-dw2-fde-dip.c > === > --- unwind-dw2-fde-dip.c(revision 190260) > +++ unwind-dw2-fde-dip.c(working copy) > @@ -54,7 +54,8 @@ > #endif > > #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > -&& defined(__BIONIC__) > +&& defined(__BIONIC__) \ > +&& defined(__mips__) > # define USE_PT_GNU_EH_FRAME > #endif Where does mips link.h come from? I didn't see it in AOSP Bionic C library. -- H.J.
RE: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
> Hi, > > Bionic C library doesn't provide link.h. This patch reverts revision > 186788: > > http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00740.html > > OK to install? > > Thanks. > > H.J. > --- > 2012-08-09 H.J. Lu > > PR bootstrap/54209 > * unwind-dw2-fde-dip.c (USE_PT_GNU_EH_FRAME): Don't define for > Bionic C library. > > diff --git a/libgcc/unwind-dw2-fde-dip.c b/libgcc/unwind-dw2-fde-dip.c > index 92f8ab5..f57dc8c 100644 > --- a/libgcc/unwind-dw2-fde-dip.c > +++ b/libgcc/unwind-dw2-fde-dip.c > @@ -54,11 +54,6 @@ > #endif > > #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > -&& defined(__BIONIC__) > -# define USE_PT_GNU_EH_FRAME > -#endif > - > -#if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > && defined(__FreeBSD__) && __FreeBSD__ >= 7 > # define ElfW __ElfN > # define USE_PT_GNU_EH_FRAME > How about this patch? Just enable it for MIPS that provides link.h in Android NDK. Thanks a lot! Regards, Chao-ying Index: unwind-dw2-fde-dip.c === --- unwind-dw2-fde-dip.c(revision 190260) +++ unwind-dw2-fde-dip.c(working copy) @@ -55,6 +55,7 @@ #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ && defined(__BIONIC__) +&& defined(__mips__) # define USE_PT_GNU_EH_FRAME #endif
RE: PATCH: PR bootstrap/54209: [4.8 Regression] Failed to build gcc for Android/x86
> > Hi, > > > > Bionic C library doesn't provide link.h. This patch > reverts revision > > 186788: > > > > http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00740.html > > > > OK to install? > > > > Thanks. > > > > H.J. > > --- > > 2012-08-09 H.J. Lu > > > > PR bootstrap/54209 > > * unwind-dw2-fde-dip.c (USE_PT_GNU_EH_FRAME): Don't define for > > Bionic C library. > > > > diff --git a/libgcc/unwind-dw2-fde-dip.c > b/libgcc/unwind-dw2-fde-dip.c > > index 92f8ab5..f57dc8c 100644 > > --- a/libgcc/unwind-dw2-fde-dip.c > > +++ b/libgcc/unwind-dw2-fde-dip.c > > @@ -54,11 +54,6 @@ > > #endif > > > > #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > > -&& defined(__BIONIC__) > > -# define USE_PT_GNU_EH_FRAME > > -#endif > > - > > -#if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > > && defined(__FreeBSD__) && __FreeBSD__ >= 7 > > # define ElfW __ElfN > > # define USE_PT_GNU_EH_FRAME > > > > How about this patch? Just enable it for MIPS that > provides link.h in Android NDK. > Thanks a lot! > > Regards, > Chao-ying > > Index: unwind-dw2-fde-dip.c > === > --- unwind-dw2-fde-dip.c(revision 190260) > +++ unwind-dw2-fde-dip.c(working copy) > @@ -55,6 +55,7 @@ > > #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ > && defined(__BIONIC__) > +&& defined(__mips__) > # define USE_PT_GNU_EH_FRAME > #endif > Sorry, I forgot \ in the previous patch. Ex: Index: unwind-dw2-fde-dip.c === --- unwind-dw2-fde-dip.c(revision 190260) +++ unwind-dw2-fde-dip.c(working copy) @@ -54,7 +54,8 @@ #endif #if !defined(inhibit_libc) && defined(HAVE_LD_EH_FRAME_HDR) \ -&& defined(__BIONIC__) +&& defined(__BIONIC__) \ +&& defined(__mips__) # define USE_PT_GNU_EH_FRAME #endif
