bfd: add stdlib.h when using abort

Message ID 20210419011924.28338-1-vapier@gentoo.org
State New
Headers show
Series
  • bfd: add stdlib.h when using abort
Related show

Commit Message

Claudiu Zissulescu via Binutils April 19, 2021, 1:19 a.m.
Since this file calls abort() now, we need to include stdlib.h for
its prototype.
---
 bfd/elf-bfd.h | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.30.2

Comments

Andreas Schwab April 19, 2021, 9:15 a.m. | #1
On Apr 18 2021, Mike Frysinger via Binutils wrote:

> Since this file calls abort() now, we need to include stdlib.h for

> its prototype.


That should come from bfd/sysdep.h.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Claudiu Zissulescu via Binutils April 19, 2021, 2:40 p.m. | #2
On 19 Apr 2021 11:15, Andreas Schwab wrote:
> On Apr 18 2021, Mike Frysinger via Binutils wrote:

> > Since this file calls abort() now, we need to include stdlib.h for

> > its prototype.

> 

> That should come from bfd/sysdep.h.


what do you suggest for these ?

In file included from sim/cris/sim-if.c:26:
sim/cris/../../bfd/elf-bfd.h: In function ‘elf_link_hash_lookup’:
sim/cris/../../bfd/elf-bfd.h:729:5: warning: implicit declaration of function ‘abort’ [-Wimplicit-function-declaration]
  729 |     abort ();
      |     ^~~~~
sim/cris/../../bfd/elf-bfd.h:729:5: warning: incompatible implicit declaration of built-in function ‘abort’
sim/cris/../../bfd/elf-bfd.h:29:1: note: include ‘<stdlib.h>’ or provide a declaration of ‘abort’
   28 | #include "bfdlink.h"
  +++ |+#include <stdlib.h>
   29 | 
sim/cris/../../bfd/elf-bfd.h: In function ‘elf_link_hash_traverse’:
sim/cris/../../bfd/elf-bfd.h:742:5: warning: incompatible implicit declaration of built-in function ‘abort’
  742 |     abort ();
      |     ^~~~~
sim/cris/../../bfd/elf-bfd.h:742:5: note: include ‘<stdlib.h>’ or provide a declaration of ‘abort’


In file included from sim/m68hc11/../../bfd/elf32-m68hc1x.h:25,
                 from sim/m68hc11/interp.c:27:
sim/m68hc11/../../bfd/elf-bfd.h: In function ‘elf_link_hash_lookup’:
sim/m68hc11/../../bfd/elf-bfd.h:729:5: warning: implicit declaration of function ‘abort’ [-Wimplicit-function-declaration]
  729 |     abort ();
      |     ^~~~~
sim/m68hc11/../../bfd/elf-bfd.h:729:5: warning: incompatible implicit declaration of built-in function ‘abort’
sim/m68hc11/../../bfd/elf-bfd.h:29:1: note: include ‘<stdlib.h>’ or provide a declaration of ‘abort’
   28 | #include "bfdlink.h"
  +++ |+#include <stdlib.h>
   29 | 
sim/m68hc11/../../bfd/elf-bfd.h: In function ‘elf_link_hash_traverse’:
sim/m68hc11/../../bfd/elf-bfd.h:742:5: warning: incompatible implicit declaration of built-in function ‘abort’
  742 |     abort ();
      |     ^~~~~
sim/m68hc11/../../bfd/elf-bfd.h:742:5: note: include ‘<stdlib.h>’ or provide a declaration of ‘abort’
Andreas Schwab April 19, 2021, 2:53 p.m. | #3
On Apr 19 2021, Mike Frysinger wrote:

> On 19 Apr 2021 11:15, Andreas Schwab wrote:

>> On Apr 18 2021, Mike Frysinger via Binutils wrote:

>> > Since this file calls abort() now, we need to include stdlib.h for

>> > its prototype.

>> 

>> That should come from bfd/sysdep.h.

>

> what do you suggest for these ?


They need to include sysdep.h.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Claudiu Zissulescu via Binutils April 19, 2021, 3:18 p.m. | #4
On 19 Apr 2021 16:53, Andreas Schwab wrote:
> On Apr 19 2021, Mike Frysinger wrote:

> > On 19 Apr 2021 11:15, Andreas Schwab wrote:

> >> On Apr 18 2021, Mike Frysinger via Binutils wrote:

> >> > Since this file calls abort() now, we need to include stdlib.h for

> >> > its prototype.

> >> 

> >> That should come from bfd/sysdep.h.

> >

> > what do you suggest for these ?

> 

> They need to include sysdep.h.


bfd/sysdep.h is viral.  it requires it be required before anything else,
and instead of the standard config.h.  m68hc11 isn't even including this
directly, it's going through bfd/elf32-m68hc1x.h.  i don't think it's
reasonable for every project that might use bfd headers be forced to use
the internal bfd/sysdep.h instead of their own config.h setup.  that
header also pulls in a lot more stuff that assumes that bfd's configure
was used to probe settings.

this isn't specific to sim/.  grep the tree for elf-bfd.h and you'll find
a ton of hits.  none of them are using bfd/sysdep.h.

what is the problem with having bfd standalone headers be standalone and
include the right headers they use ?
-mike
Andreas Schwab April 19, 2021, 3:39 p.m. | #5
On Apr 19 2021, Mike Frysinger wrote:

> this isn't specific to sim/.  grep the tree for elf-bfd.h and you'll find

> a ton of hits.  none of them are using bfd/sysdep.h.


They all include the equivalent of it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Claudiu Zissulescu via Binutils April 19, 2021, 3:45 p.m. | #6
On 19 Apr 2021 17:39, Andreas Schwab wrote:
> On Apr 19 2021, Mike Frysinger wrote:

> > this isn't specific to sim/.  grep the tree for elf-bfd.h and you'll find

> > a ton of hits.  none of them are using bfd/sysdep.h.

> 

> They all include the equivalent of it.


i don't know what that means.  can you be less cryptic ?

i'm still not seeing why we're forcing consumers of headers to include all
the random things that the headers might use/need.  this is a strong anti-
pattern imo and impossible to reasonably maintain.  if a header needs the
defs from a standard header (like stdlib.h), it should be including it.
there's no penalty for doing so -- this is why #ifdef header guards exist.
-mike
Claudiu Zissulescu via Binutils April 24, 2021, 2:02 a.m. | #7
On 18 Apr 2021 21:19, Mike Frysinger via Binutils wrote:
> Since this file calls abort() now, we need to include stdlib.h for

> its prototype.

> ---

>  bfd/elf-bfd.h | 2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h

> index 5a2f64233f9c..fb2683724e11 100644

> --- a/bfd/elf-bfd.h

> +++ b/bfd/elf-bfd.h

> @@ -22,6 +22,8 @@

>  #ifndef _LIBELF_H_

>  #define _LIBELF_H_ 1

>  

> +#include <stdlib.h>

> +

>  #include "elf/common.h"

>  #include "elf/external.h"

>  #include "elf/internal.h"


OK to merge now ?
-mike
Claudiu Zissulescu via Binutils April 26, 2021, 11:56 a.m. | #8
Hi Mike,

>> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h

>> index 5a2f64233f9c..fb2683724e11 100644

>> --- a/bfd/elf-bfd.h

>> +++ b/bfd/elf-bfd.h

>> @@ -22,6 +22,8 @@

>>   #ifndef _LIBELF_H_

>>   #define _LIBELF_H_ 1

>>   

>> +#include <stdlib.h>

>> +


> OK to merge now ?


Go ahead punk - make my day.  Err, I mean, approved, please apply.

Cheers
   Nick

Patch

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 5a2f64233f9c..fb2683724e11 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -22,6 +22,8 @@ 
 #ifndef _LIBELF_H_
 #define _LIBELF_H_ 1
 
+#include <stdlib.h>
+
 #include "elf/common.h"
 #include "elf/external.h"
 #include "elf/internal.h"