diff mbox

PATCH: Include <link.h> only if USE_PT_GNU_EH_FRAME is defined

Message ID 20100907163838.GA13615@intel.com
State New
Headers show

Commit Message

H.J. Lu Sept. 7, 2010, 4:38 p.m. UTC
Hi,

Bionic C library doesn't have <link.h>.  But it uses
unwind-dw2-fde-glibc.c.  This patch includes <link.h> only if it is
really needed.  OK for trunk?

Thanks.


H.J.
---
2010-09-07  H.J. Lu  <hongjiu.lu@intel.com>

	* unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
	Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.

Comments

Richard Henderson Sept. 7, 2010, 5:09 p.m. UTC | #1
On 09/07/2010 09:38 AM, H.J. Lu wrote:
> 	* unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
> 	Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.

Why is Bionic using unwind-dw2-fde-glibc.c?  The *only* point
to using that file is PT_GNU_EH_FRAME.  Rather than make this
change, why not just point Bionic to unwind-dw2-fde.c?


r~
Joseph Myers Sept. 7, 2010, 7:58 p.m. UTC | #2
On Tue, 7 Sep 2010, Richard Henderson wrote:

> On 09/07/2010 09:38 AM, H.J. Lu wrote:
> > 	* unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
> > 	Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.
> 
> Why is Bionic using unwind-dw2-fde-glibc.c?  The *only* point
> to using that file is PT_GNU_EH_FRAME.  Rather than make this
> change, why not just point Bionic to unwind-dw2-fde.c?

The design is that you can have a multilib toolchain with multilibs for 
all of glibc, uClibc and Bionic.  Until the toplevel libgcc transition is 
more complete, actually selecting which files get built into libgcc based 
on the multilib being built can be tricky.
Richard Henderson Sept. 7, 2010, 8:02 p.m. UTC | #3
On 09/07/2010 12:58 PM, Joseph S. Myers wrote:
> On Tue, 7 Sep 2010, Richard Henderson wrote:
> 
>> On 09/07/2010 09:38 AM, H.J. Lu wrote:
>>> 	* unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
>>> 	Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.
>>
>> Why is Bionic using unwind-dw2-fde-glibc.c?  The *only* point
>> to using that file is PT_GNU_EH_FRAME.  Rather than make this
>> change, why not just point Bionic to unwind-dw2-fde.c?
> 
> The design is that you can have a multilib toolchain with multilibs for 
> all of glibc, uClibc and Bionic.  Until the toplevel libgcc transition is 
> more complete, actually selecting which files get built into libgcc based 
> on the multilib being built can be tricky.

Ah.  Well.  Perhaps HJ's original patch is better.
What do you think Joseph?


r~
Joseph Myers Sept. 7, 2010, 8:05 p.m. UTC | #4
On Tue, 7 Sep 2010, Richard Henderson wrote:

> On 09/07/2010 12:58 PM, Joseph S. Myers wrote:
> > On Tue, 7 Sep 2010, Richard Henderson wrote:
> > 
> >> On 09/07/2010 09:38 AM, H.J. Lu wrote:
> >>> 	* unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
> >>> 	Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.
> >>
> >> Why is Bionic using unwind-dw2-fde-glibc.c?  The *only* point
> >> to using that file is PT_GNU_EH_FRAME.  Rather than make this
> >> change, why not just point Bionic to unwind-dw2-fde.c?
> > 
> > The design is that you can have a multilib toolchain with multilibs for 
> > all of glibc, uClibc and Bionic.  Until the toplevel libgcc transition is 
> > more complete, actually selecting which files get built into libgcc based 
> > on the multilib being built can be tricky.
> 
> Ah.  Well.  Perhaps HJ's original patch is better.
> What do you think Joseph?

HJ's original patch does look like it ought to work in such a multilib 
configuration (i.e., solve the problem in such a configuration if it 
solves it in an Android-only configuration).
H.J. Lu Sept. 7, 2010, 8:08 p.m. UTC | #5
On Tue, Sep 7, 2010 at 1:05 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Tue, 7 Sep 2010, Richard Henderson wrote:
>
>> On 09/07/2010 12:58 PM, Joseph S. Myers wrote:
>> > On Tue, 7 Sep 2010, Richard Henderson wrote:
>> >
>> >> On 09/07/2010 09:38 AM, H.J. Lu wrote:
>> >>>   * unwind-dw2-fde-glibc.c: Include <elf.h> for DT_CONFIG.
>> >>>   Include <link.h> only if USE_PT_GNU_EH_FRAME is defined.
>> >>
>> >> Why is Bionic using unwind-dw2-fde-glibc.c?  The *only* point
>> >> to using that file is PT_GNU_EH_FRAME.  Rather than make this
>> >> change, why not just point Bionic to unwind-dw2-fde.c?
>> >
>> > The design is that you can have a multilib toolchain with multilibs for
>> > all of glibc, uClibc and Bionic.  Until the toplevel libgcc transition is
>> > more complete, actually selecting which files get built into libgcc based
>> > on the multilib being built can be tricky.
>>
>> Ah.  Well.  Perhaps HJ's original patch is better.
>> What do you think Joseph?
>
> HJ's original patch does look like it ought to work in such a multilib
> configuration (i.e., solve the problem in such a configuration if it
> solves it in an Android-only configuration).
>

Should I apply my original patch instead?

Thanks.
Richard Henderson Sept. 7, 2010, 8:09 p.m. UTC | #6
On 09/07/2010 01:08 PM, H.J. Lu wrote:
> On Tue, Sep 7, 2010 at 1:05 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
>> On Tue, 7 Sep 2010, Richard Henderson wrote:
>>> Ah.  Well.  Perhaps HJ's original patch is better.
>>> What do you think Joseph?
>>
>> HJ's original patch does look like it ought to work in such a multilib
>> configuration (i.e., solve the problem in such a configuration if it
>> solves it in an Android-only configuration).
>>
> 
> Should I apply my original patch instead?

Yes, I think so.  Sorry for the run around.


r~
diff mbox

Patch

diff --git a/gcc/unwind-dw2-fde-glibc.c b/gcc/unwind-dw2-fde-glibc.c
index b8a7312..a762d87 100644
--- a/gcc/unwind-dw2-fde-glibc.c
+++ b/gcc/unwind-dw2-fde-glibc.c
@@ -33,7 +33,7 @@ 
 #include "tconfig.h"
 #include "tsystem.h"
 #ifndef inhibit_libc
-#include <link.h>
+#include <elf.h>		/* Get DT_CONFIG.  */
 #endif
 #include "coretypes.h"
 #include "tm.h"
@@ -59,6 +59,8 @@ 
 
 #if defined(USE_PT_GNU_EH_FRAME)
 
+#include <link.h>
+
 #ifndef __RELOC_POINTER
 # define __RELOC_POINTER(ptr, base) ((ptr) + (base))
 #endif