[PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Alex Richardson]

I was getting "I'm sorry, Dave, I can't do that.  Symbol format `elf64-littleriscv' unknown."
errors after updating from GDB 8.3 to 10. Bisecting showed that since
commit 1ff6de031241c59d0ff9fa01d3c0a4049b0e97c9, bfd.h depends on strncmp()
being present, so configuring with -Werror results in the check for ELF
support in BFD failing:
.../gdb/gdb/../bfd/elf-bfd.h: In function 'bfd_section_is_ctf':
.../gdb/gdb/../bfd/elf-bfd.h:3086:10: error: implicit declaration of function 'strncmp' [-Werror=implicit-function-declaration]
   return strncmp (name, ".ctf", 4) == 0 && (name[4] == 0 || name[4] == '.');
---
 gdb/acinclude.m4 | 1 +
 gdb/configure    | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 64574e26314..68520d6d938 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -266,6 +266,7 @@ AC_DEFUN([GDB_AC_CHECK_BFD], [
     [AC_LINK_IFELSE(
        [AC_LANG_PROGRAM(
 	  [#include <stdlib.h>
+	   #include <string.h>
 	   #include "bfd.h"
 	   #include "$4"],
 	  [return $3;]
diff --git a/gdb/configure b/gdb/configure
index 4a03cd9c3ec..ddbeefe426e 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -16745,6 +16745,7 @@ else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdlib.h>
+	   #include <string.h>
 	   #include "bfd.h"
 	   #include "elf-bfd.h"
 int
@@ -16858,6 +16859,7 @@ else
   cat confdefs.h - <<_ACEOF >conftest.$ac_ext
 /* end confdefs.h.  */
 #include <stdlib.h>
+	   #include <string.h>
 	   #include "bfd.h"
 	   #include "mach-o.h"
 int
-- 
2.29.1

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Simon Marchi]

On 2020-11-11 4:20 a.m., Alex Richardson via Gdb-patches wrote:
> I was getting "I'm sorry, Dave, I can't do that.  Symbol format `elf64-littleriscv' unknown."
> errors after updating from GDB 8.3 to 10. Bisecting showed that since
> commit 1ff6de031241c59d0ff9fa01d3c0a4049b0e97c9, bfd.h depends on strncmp()
> being present, so configuring with -Werror results in the check for ELF
> support in BFD failing:
> .../gdb/gdb/../bfd/elf-bfd.h: In function 'bfd_section_is_ctf':
> .../gdb/gdb/../bfd/elf-bfd.h:3086:10: error: implicit declaration of function 'strncmp' [-Werror=implicit-function-declaration]
>    return strncmp (name, ".ctf", 4) == 0 && (name[4] == 0 || name[4] == '.');
> ---
>  gdb/acinclude.m4 | 1 +
>  gdb/configure    | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
> index 64574e26314..68520d6d938 100644
> --- a/gdb/acinclude.m4
> +++ b/gdb/acinclude.m4
> @@ -266,6 +266,7 @@ AC_DEFUN([GDB_AC_CHECK_BFD], [
>      [AC_LINK_IFELSE(
>         [AC_LANG_PROGRAM(
>  	  [#include <stdlib.h>
> +	   #include <string.h>
>  	   #include "bfd.h"
>  	   #include "$4"],
>  	  [return $3;]
> diff --git a/gdb/configure b/gdb/configure
> index 4a03cd9c3ec..ddbeefe426e 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -16745,6 +16745,7 @@ else
>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  /* end confdefs.h.  */
>  #include <stdlib.h>
> +	   #include <string.h>
>  	   #include "bfd.h"
>  	   #include "elf-bfd.h"
>  int
> @@ -16858,6 +16859,7 @@ else
>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>  /* end confdefs.h.  */
>  #include <stdlib.h>
> +	   #include <string.h>
>  	   #include "bfd.h"
>  	   #include "mach-o.h"
>  int
> -- 
> 2.29.1
> 

Since elf-bfd.h uses strncmp, I think it should include string.h.  Is there a good reason not to do that?

Simon

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Alexander Richardson]

On Wed, 11 Nov 2020 at 20:48, Simon Marchi <simark@simark.ca> wrote:
>
> On 2020-11-11 4:20 a.m., Alex Richardson via Gdb-patches wrote:
> > I was getting "I'm sorry, Dave, I can't do that.  Symbol format `elf64-littleriscv' unknown."
> > errors after updating from GDB 8.3 to 10. Bisecting showed that since
> > commit 1ff6de031241c59d0ff9fa01d3c0a4049b0e97c9, bfd.h depends on strncmp()
> > being present, so configuring with -Werror results in the check for ELF
> > support in BFD failing:
> > .../gdb/gdb/../bfd/elf-bfd.h: In function 'bfd_section_is_ctf':
> > .../gdb/gdb/../bfd/elf-bfd.h:3086:10: error: implicit declaration of function 'strncmp' [-Werror=implicit-function-declaration]
> >    return strncmp (name, ".ctf", 4) == 0 && (name[4] == 0 || name[4] == '.');
> > ---
> >  gdb/acinclude.m4 | 1 +
> >  gdb/configure    | 2 ++
> >  2 files changed, 3 insertions(+)
> >
> > diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
> > index 64574e26314..68520d6d938 100644
> > --- a/gdb/acinclude.m4
> > +++ b/gdb/acinclude.m4
> > @@ -266,6 +266,7 @@ AC_DEFUN([GDB_AC_CHECK_BFD], [
> >      [AC_LINK_IFELSE(
> >         [AC_LANG_PROGRAM(
> >         [#include <stdlib.h>
> > +        #include <string.h>
> >          #include "bfd.h"
> >          #include "$4"],
> >         [return $3;]
> > diff --git a/gdb/configure b/gdb/configure
> > index 4a03cd9c3ec..ddbeefe426e 100755
> > --- a/gdb/configure
> > +++ b/gdb/configure
> > @@ -16745,6 +16745,7 @@ else
> >    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> >  /* end confdefs.h.  */
> >  #include <stdlib.h>
> > +        #include <string.h>
> >          #include "bfd.h"
> >          #include "elf-bfd.h"
> >  int
> > @@ -16858,6 +16859,7 @@ else
> >    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> >  /* end confdefs.h.  */
> >  #include <stdlib.h>
> > +        #include <string.h>
> >          #include "bfd.h"
> >          #include "mach-o.h"
> >  int
> > --
> > 2.29.1
> >
>
> Since elf-bfd.h uses strncmp, I think it should include string.h.  Is there a good reason not to do that?
>

Hi Simon,

that's what I originally planned, but it seems like elf-bfd.h (and the
headers it includes) don't include any system headers. Since I'm not
familiar with any of this code I assumed this was intentional.

Alex

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Tom Tromey]

>>>>> "Alexander" == Alexander Richardson via Gdb-patches <gdb-patches@sourceware.org> writes:

Alexander> that's what I originally planned, but it seems like elf-bfd.h (and the
Alexander> headers it includes) don't include any system headers. Since I'm not
Alexander> familiar with any of this code I assumed this was intentional.

It seems like a question for the binutils list.

Maybe the answer is that configuring with -Werror shouldn't be done,
though.  I don't know the current state, but in the past, Autoconf
sometimes generated somewhat questionable test programs, so it's not
clear to me that the parts of configure that are out of gdb's control
are -Werror-clean.

Tom

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Simon Marchi]

On 2020-11-12 4:25 a.m., Alexander Richardson wrote:
> On Wed, 11 Nov 2020 at 20:48, Simon Marchi <simark@simark.ca> wrote:
>>
>> On 2020-11-11 4:20 a.m., Alex Richardson via Gdb-patches wrote:
>>> I was getting "I'm sorry, Dave, I can't do that.  Symbol format `elf64-littleriscv' unknown."
>>> errors after updating from GDB 8.3 to 10. Bisecting showed that since
>>> commit 1ff6de031241c59d0ff9fa01d3c0a4049b0e97c9, bfd.h depends on strncmp()
>>> being present, so configuring with -Werror results in the check for ELF
>>> support in BFD failing:
>>> .../gdb/gdb/../bfd/elf-bfd.h: In function 'bfd_section_is_ctf':
>>> .../gdb/gdb/../bfd/elf-bfd.h:3086:10: error: implicit declaration of function 'strncmp' [-Werror=implicit-function-declaration]
>>>    return strncmp (name, ".ctf", 4) == 0 && (name[4] == 0 || name[4] == '.');
>>> ---
>>>  gdb/acinclude.m4 | 1 +
>>>  gdb/configure    | 2 ++
>>>  2 files changed, 3 insertions(+)
>>>
>>> diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
>>> index 64574e26314..68520d6d938 100644
>>> --- a/gdb/acinclude.m4
>>> +++ b/gdb/acinclude.m4
>>> @@ -266,6 +266,7 @@ AC_DEFUN([GDB_AC_CHECK_BFD], [
>>>      [AC_LINK_IFELSE(
>>>         [AC_LANG_PROGRAM(
>>>         [#include <stdlib.h>
>>> +        #include <string.h>
>>>          #include "bfd.h"
>>>          #include "$4"],
>>>         [return $3;]
>>> diff --git a/gdb/configure b/gdb/configure
>>> index 4a03cd9c3ec..ddbeefe426e 100755
>>> --- a/gdb/configure
>>> +++ b/gdb/configure
>>> @@ -16745,6 +16745,7 @@ else
>>>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>>>  /* end confdefs.h.  */
>>>  #include <stdlib.h>
>>> +        #include <string.h>
>>>          #include "bfd.h"
>>>          #include "elf-bfd.h"
>>>  int
>>> @@ -16858,6 +16859,7 @@ else
>>>    cat confdefs.h - <<_ACEOF >conftest.$ac_ext
>>>  /* end confdefs.h.  */
>>>  #include <stdlib.h>
>>> +        #include <string.h>
>>>          #include "bfd.h"
>>>          #include "mach-o.h"
>>>  int
>>> --
>>> 2.29.1
>>>
>>
>> Since elf-bfd.h uses strncmp, I think it should include string.h.  Is there a good reason not to do that?
>>
>
> Hi Simon,
>
> that's what I originally planned, but it seems like elf-bfd.h (and the
> headers it includes) don't include any system headers. Since I'm not
> familiar with any of this code I assumed this was intentional.
>
> Alex
>

Hi binutils@,

Could you check the discussion above?  Is there a reason elf-bfd.h
doesn't include the header file it needs to use the functions it uses?

Simon

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Nick Clifton]

Hi Simon,

>> that's what I originally planned, but it seems like elf-bfd.h (and the
>> headers it includes) don't include any system headers. Since I'm not
>> familiar with any of this code I assumed this was intentional.
>>
>> Alex
>>
> 
> Hi binutils@,
> 
> Could you check the discussion above?  Is there a reason elf-bfd.h
> doesn't include the header file it needs to use the functions it uses?

Essentially this is because elf-bfd.h is internal to the binutils
sources, and not expected to be used elsewhere.  So any code that
includes it is also expected to include the sysdep.h header which
does then include the needed system headers.

The idea is that all of the configuration time decisions about which
system headers to include are confined to one file - sysdep.h - rather
than having to be copied into all header files.

Cheers
   Nick

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Simon Marchi]

On 2020-11-13 7:09 a.m., Nick Clifton wrote:
> Hi Simon,
>
>>> that's what I originally planned, but it seems like elf-bfd.h (and the
>>> headers it includes) don't include any system headers. Since I'm not
>>> familiar with any of this code I assumed this was intentional.
>>>
>>> Alex
>>>
>>
>> Hi binutils@,
>>
>> Could you check the discussion above?  Is there a reason elf-bfd.h
>> doesn't include the header file it needs to use the functions it uses?
>
> Essentially this is because elf-bfd.h is internal to the binutils
> sources, and not expected to be used elsewhere.  So any code that
> includes it is also expected to include the sysdep.h header which
> does then include the needed system headers.
>
> The idea is that all of the configuration time decisions about which
> system headers to include are confined to one file - sysdep.h - rather
> than having to be copied into all header files.
>
> Cheers
>   Nick
>
>

Ah ok I remember this, we started discussing this in:

https://sourceware.org/pipermail/gdb-patches/2020-September/172041.html

In an ideal world, GDB would stop using BFD's internal functions.  But
for this, BFD would need to expose some ELF-specific bits, such as
program headers.  I don't know if this is against BFD's design
principles to abstract things across various executable formats.

I think Alexander's patch is fine, I don't think the complexity of the
patch linked above is necessary.  Are there even any systems today that
GDB supports for which strncmp isn't found in string.h?

Alexander, may I ask what particular configuration you are using?  It
would be good to document it in the commit log.

Simon

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Alexander Richardson]

On Fri, 13 Nov 2020 at 13:46, Simon Marchi <simark@simark.ca> wrote:
>
>
> On 2020-11-13 7:09 a.m., Nick Clifton wrote:
> > Hi Simon,
> >
> >>> that's what I originally planned, but it seems like elf-bfd.h (and the
> >>> headers it includes) don't include any system headers. Since I'm not
> >>> familiar with any of this code I assumed this was intentional.
> >>>
> >>> Alex
> >>>
> >>
> >> Hi binutils@,
> >>
> >> Could you check the discussion above?  Is there a reason elf-bfd.h
> >> doesn't include the header file it needs to use the functions it uses?
> >
> > Essentially this is because elf-bfd.h is internal to the binutils
> > sources, and not expected to be used elsewhere.  So any code that
> > includes it is also expected to include the sysdep.h header which
> > does then include the needed system headers.
> >
> > The idea is that all of the configuration time decisions about which
> > system headers to include are confined to one file - sysdep.h - rather
> > than having to be copied into all header files.
> >
> > Cheers
> >   Nick
> >
> >
>
> Ah ok I remember this, we started discussing this in:
>
> https://sourceware.org/pipermail/gdb-patches/2020-September/172041.html
>
> In an ideal world, GDB would stop using BFD's internal functions.  But
> for this, BFD would need to expose some ELF-specific bits, such as
> program headers.  I don't know if this is against BFD's design
> principles to abstract things across various executable formats.
>
> I think Alexander's patch is fine, I don't think the complexity of the
> patch linked above is necessary.  Are there even any systems today that
> GDB supports for which strncmp isn't found in string.h?
>
> Alexander, may I ask what particular configuration you are using?  It
> would be good to document it in the commit log.
>
Hi Simon,

I was configuring GDB with -Werror=implicit-function-definition in
CFLAGS and the --disable-werror configure argument.
I've dropped that flag from my build scripts for now, but I would like
to add it again if this patch is accepted.

Thanks,
Alex

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Tom Tromey]

>>>>> "Alex" == Alexander Richardson via Binutils <binutils@sourceware.org> writes:

Alex> I was configuring GDB with -Werror=implicit-function-definition in
Alex> CFLAGS and the --disable-werror configure argument.
Alex> I've dropped that flag from my build scripts for now, but I would like
Alex> to add it again if this patch is accepted.

It's kind of lame that we can't (AFAIK) specify one set of flags for
configure and another set for the build itself.  Maybe this could be
added to the top-level Makefile somehow.

Tom

Re: [PATCH] GDB: Fix detection of ELF support when configuring with -Werror

[Simon Marchi]

On 2020-11-20 10:54 a.m., Tom Tromey wrote:
>>>>>> "Alex" == Alexander Richardson via Binutils <binutils@sourceware.org> writes:
>
> Alex> I was configuring GDB with -Werror=implicit-function-definition in
> Alex> CFLAGS and the --disable-werror configure argument.
> Alex> I've dropped that flag from my build scripts for now, but I would like
> Alex> to add it again if this patch is accepted.
>
> It's kind of lame that we can't (AFAIK) specify one set of flags for
> configure and another set for the build itself.  Maybe this could be
> added to the top-level Makefile somehow.

I don't know, having multiple set of flags would make it even more
confusing I think.  And it would be risky to have configure take some
decisions based on some set of flags and compile the code using some
different set of flags.

Anyhow, I've pushed Alexander's patch, since it can't really hurt and
it fixes the problem for now.  I adjusted the commit message a bit to
mention explicitly that it was caused by -Wimplicit-function-declaration
(it's mentioned in the compiler error message, but I think it's good to
make it clear, in case someone tries to reproduce the problem).

Simon