gdb: fix IA64 build failure of linux-nat

Message ID 20200519212710.1417100-1-slyfox@gentoo.org
State New
Headers show
Series
  • gdb: fix IA64 build failure of linux-nat
Related show

Commit Message

Kevin Buettner via Gdb-patches May 19, 2020, 9:27 p.m.
From: Sergei Trofimovich <siarheit@google.com>


On IA64 built failed as:

```
ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope
  352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))
      |                             ^~~~~~~~~~~~~~~~
```

The fix includes "gdbarch.h" header where symbol is declared.

	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used
	'gdbarch_num_regs'.

Signed-off-by: Sergei Trofimovich <siarheit@google.com>

---
 gdb/ChangeLog        | 5 +++++
 gdb/ia64-linux-nat.c | 1 +
 2 files changed, 6 insertions(+)

-- 
2.26.2

Comments

Kevin Buettner via Gdb-patches May 19, 2020, 10 p.m. | #1
On Tue, 19 May 2020 22:27:10 +0100
Sergei Trofimovich via Gdb-patches <gdb-patches@sourceware.org> wrote:

> From: Sergei Trofimovich <siarheit@google.com>

> 

> On IA64 built failed as:

> 

> ```

> ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope

>   352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))

>       |                             ^~~~~~~~~~~~~~~~

> ```

> 

> The fix includes "gdbarch.h" header where symbol is declared.

> 

> 	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used

> 	'gdbarch_num_regs'.


Okay, but please capitalize "include" in the ChangeLog entry prior
to pushing this change.

Kevin
Kevin Buettner via Gdb-patches Aug. 16, 2020, 8:45 a.m. | #2
On Tue, 19 May 2020 15:00:41 -0700
Kevin Buettner <kevinb@redhat.com> wrote:

> On Tue, 19 May 2020 22:27:10 +0100

> Sergei Trofimovich via Gdb-patches <gdb-patches@sourceware.org> wrote:

> 

> > From: Sergei Trofimovich <siarheit@google.com>

> > 

> > On IA64 built failed as:

> > 

> > ```

> > ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope

> >   352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))

> >       |                             ^~~~~~~~~~~~~~~~

> > ```

> > 

> > The fix includes "gdbarch.h" header where symbol is declared.

> > 

> > 	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used

> > 	'gdbarch_num_regs'.  

> 

> Okay, but please capitalize "include" in the ChangeLog entry prior

> to pushing this change.


Attached v2-* patch with capitalization changes.

I don't have a 'gdb' write access yet (I think), but I do have GCC one.

Should I request 'gdb' access as well as specified in
    https://sourceware.org/cgi-bin/pdw/ps_form.cgi ?

Thank you for the review! 

-- 

  Sergei
From 0357a5913eb9232d9a29a7e1b2c90d3a03a1ae2b Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>

Date: Tue, 19 May 2020 22:19:45 +0100
Subject: [PATCH v2] gdb: fix IA64 build failure of linux-nat

On IA64 built failed as:

```
ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope
  352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))
      |                             ^~~~~~~~~~~~~~~~
```

The fix includes "gdbarch.h" header where symbol is declared.

	* gdb/ia64-linux-nat.c: Include "gdbarch.h" to declare used
	'gdbarch_num_regs'.

Signed-off-by: Sergei Trofimovich <siarheit@google.com>

---
 gdb/ChangeLog        | 5 +++++
 gdb/ia64-linux-nat.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9cc7e44cba7..8865e6949d6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -4564,6 +4564,11 @@
 
 	* sparc64-tdep.c (adi_tag_fd): Update call to target_fileio_open.
 
+2020-05-19  Sergei Trofimovich  <siarheit@google.com>
+
+	* gdb/ia64-linux-nat.c: Include "gdbarch.h" to declare used
+	'gdbarch_num_regs'.
+
 2020-05-19  Simon Marchi  <simon.marchi@efficios.com>
 
 	* dwarf2/read.c (quirk_rust_enum): Allocate enough fields.
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index 8f36ea78e76..b532e7a51c5 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -21,6 +21,7 @@
 #include "defs.h"
 #include "inferior.h"
 #include "target.h"
+#include "gdbarch.h"
 #include "gdbcore.h"
 #include "regcache.h"
 #include "ia64-tdep.h"
-- 
2.28.0
Simon Marchi Aug. 16, 2020, 9:51 p.m. | #3
On 2020-08-16 4:45 a.m., Sergei Trofimovich via Gdb-patches wrote:
> On Tue, 19 May 2020 15:00:41 -0700

> Kevin Buettner <kevinb@redhat.com> wrote:

> 

>> On Tue, 19 May 2020 22:27:10 +0100

>> Sergei Trofimovich via Gdb-patches <gdb-patches@sourceware.org> wrote:

>>

>>> From: Sergei Trofimovich <siarheit@google.com>

>>>

>>> On IA64 built failed as:

>>>

>>> ```

>>> ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope

>>>   352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))

>>>       |                             ^~~~~~~~~~~~~~~~

>>> ```

>>>

>>> The fix includes "gdbarch.h" header where symbol is declared.

>>>

>>> 	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used

>>> 	'gdbarch_num_regs'.  

>>

>> Okay, but please capitalize "include" in the ChangeLog entry prior

>> to pushing this change.

> 

> Attached v2-* patch with capitalization changes.

> 

> I don't have a 'gdb' write access yet (I think), but I do have GCC one.

> 

> Should I request 'gdb' access as well as specified in

>     https://sourceware.org/cgi-bin/pdw/ps_form.cgi ?


Yes, that would be useful if you plan on contributing regularly.  If it's just
a occasional patch, we can also push for you.  As you wish.

> 

> Thank you for the review! 


A few more styling nits:

> diff --git a/gdb/ChangeLog b/gdb/ChangeLog

> index 9cc7e44cba7..8865e6949d6 100644

> --- a/gdb/ChangeLog

> +++ b/gdb/ChangeLog

> @@ -4564,6 +4564,11 @@

>

>  	* sparc64-tdep.c (adi_tag_fd): Update call to target_fileio_open.

>

> +2020-05-19  Sergei Trofimovich  <siarheit@google.com>

> +

> +	* gdb/ia64-linux-nat.c: Include "gdbarch.h" to declare used

> +	'gdbarch_num_regs'.


The file path should be relative to the ChangeLog location, so here just "ia64-linux-nat.c".

Also, make sure that your new entry is at the top of the file (here, it's at line 4564), and
that you update the date to $TODAY when you push.  For these reasons, most people don't include
the ChangeLog bits in the patch directly, but just include it in the commit log (as you did).
Of course, when pushing the patch, then you need to insert it in the ChangeLog file.

Simon
Simon Marchi Aug. 17, 2020, 8:21 a.m. | #4
On 2020-08-17 4:54 a.m., Tom de Vries wrote:
> Hi,

> 

> IA64 support was just obsoleted in bfd (commit 73d0dc162e "Obsolete ia64").

> 

> So, AFAIU, this should now be built with --enable-obsolete.

> 

> What are the consequences for gdb IA64 patches ?

> 

> Thanks,

> - Tom


By transitivity, I'd say that the GDB port is also obsolete.  But as long as it's in
the tree, it's fine to accept patches to keep it building (otherwise it's useless to
keep it in the tree).

If you wanted to build GDB with --target=ia64-something-something, you'll need to pass
--enable-obsolete in order to build BFD, which is a required dependency of GDB.  When we
pass --enable-targets=all, it builds a GDB with ia64 support without requiring that
--enable-obsolete flag though.

Do you know what's the BFD policy for obsolete configurations?  If they wanted to get
rid of it completely, they would need to remove GDB support too, otherwise they'd break
the build.  So should we eventually take the lead and remove support for it first?

Sergei, quick survey: do you, or someone you know actually use GDB on ia64?  Or you just
noticed it not building because you package it?

Simon
Tom de Vries Aug. 17, 2020, 8:54 a.m. | #5
On 8/16/20 11:51 PM, Simon Marchi wrote:
> On 2020-08-16 4:45 a.m., Sergei Trofimovich via Gdb-patches wrote:

>> On Tue, 19 May 2020 15:00:41 -0700

>> Kevin Buettner <kevinb@redhat.com> wrote:

>>

>>> On Tue, 19 May 2020 22:27:10 +0100

>>> Sergei Trofimovich via Gdb-patches <gdb-patches@sourceware.org> wrote:

>>>

>>>> From: Sergei Trofimovich <siarheit@google.com>

>>>>

>>>> On IA64 built failed as:

>>>>

>>>> ```

>>>> ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope

>>>>   352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))

>>>>       |                             ^~~~~~~~~~~~~~~~

>>>> ```

>>>>

>>>> The fix includes "gdbarch.h" header where symbol is declared.

>>>>

>>>> 	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used

>>>> 	'gdbarch_num_regs'.  

>>>

>>> Okay, but please capitalize "include" in the ChangeLog entry prior

>>> to pushing this change.

>>

>> Attached v2-* patch with capitalization changes.

>>

>> I don't have a 'gdb' write access yet (I think), but I do have GCC one.

>>

>> Should I request 'gdb' access as well as specified in

>>     https://sourceware.org/cgi-bin/pdw/ps_form.cgi ?

> 

> Yes, that would be useful if you plan on contributing regularly.  If it's just

> a occasional patch, we can also push for you.  As you wish.

> 

>>

>> Thank you for the review! 

> 

> A few more styling nits:

> 

>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog

>> index 9cc7e44cba7..8865e6949d6 100644

>> --- a/gdb/ChangeLog

>> +++ b/gdb/ChangeLog

>> @@ -4564,6 +4564,11 @@

>>

>>  	* sparc64-tdep.c (adi_tag_fd): Update call to target_fileio_open.

>>

>> +2020-05-19  Sergei Trofimovich  <siarheit@google.com>

>> +

>> +	* gdb/ia64-linux-nat.c: Include "gdbarch.h" to declare used

>> +	'gdbarch_num_regs'.

> 

> The file path should be relative to the ChangeLog location, so here just "ia64-linux-nat.c".

> 

> Also, make sure that your new entry is at the top of the file (here, it's at line 4564), and

> that you update the date to $TODAY when you push.  For these reasons, most people don't include

> the ChangeLog bits in the patch directly, but just include it in the commit log (as you did).

> Of course, when pushing the patch, then you need to insert it in the ChangeLog file.

> 


Hi,

IA64 support was just obsoleted in bfd (commit 73d0dc162e "Obsolete ia64").

So, AFAIU, this should now be built with --enable-obsolete.

What are the consequences for gdb IA64 patches ?

Thanks,
- Tom
Kevin Buettner via Gdb-patches Aug. 17, 2020, 6:59 p.m. | #6
On Mon, 17 Aug 2020 04:21:40 -0400
Simon Marchi <simark@simark.ca> wrote:

> On 2020-08-17 4:54 a.m., Tom de Vries wrote:

> > Hi,

> > 

> > IA64 support was just obsoleted in bfd (commit 73d0dc162e "Obsolete ia64").

> > 

> > So, AFAIU, this should now be built with --enable-obsolete.

> > 

> > What are the consequences for gdb IA64 patches ?

> > 

> > Thanks,

> > - Tom  

> 

> By transitivity, I'd say that the GDB port is also obsolete.  But as long as it's in

> the tree, it's fine to accept patches to keep it building (otherwise it's useless to

> keep it in the tree).

> 

> If you wanted to build GDB with --target=ia64-something-something, you'll need to pass

> --enable-obsolete in order to build BFD, which is a required dependency of GDB.  When we

> pass --enable-targets=all, it builds a GDB with ia64 support without requiring that

> --enable-obsolete flag though.

> 

> Do you know what's the BFD policy for obsolete configurations?  If they wanted to get

> rid of it completely, they would need to remove GDB support too, otherwise they'd break

> the build.  So should we eventually take the lead and remove support for it first?

> 

> Sergei, quick survey: do you, or someone you know actually use GDB on ia64?  Or you just

> noticed it not building because you package it?


I'm occasionally using gdb on ia64 for real debugging.

I'm part of the team that still keeps ia64 port alive in Gentoo.
(And also a part of team that maintains binutils and gdb).
Debian also has a functioning ia64 port and are interested to have gdb running.

gdb still comes in handy for bug squashing and poking at disassembly.

I'd like to tackle a few slightly more substantial bugs on binutils-gdb-ia64
in hopes to prolong ia64's life a bit more (if it's not much of a maintenance
burden for other ports). But I don't promise anything :)

Basic gdb support still works fine:

"""
# uname -a
Linux guppy 4.19.86-gentoo #4 SMP Fri Dec 6 23:07:34 UTC 2019 ia64 Dual-Core Intel(R) Itanium(R) Processor 9040 GenuineIntel GNU/Linux

# gdb ./a
GNU gdb (Gentoo 9.2 vanilla) 9.2
...
(gdb) start
Temporary breakpoint 1 at 0x7f1
Starting program: /root/a
Failed to read a valid object file image from memory.

Temporary breakpoint 1, 0x20000008000007f1 in main ()
(gdb) disassemble
Dump of assembler code for function main:
   0x20000008000007f0 <+0>:     [MII]       mov r2=r12
=> 0x20000008000007f1 <+1>:                 mov r14=r0;;
   0x20000008000007f2 <+2>:                 mov r8=r14
   0x2000000800000800 <+16>:    [MIB]       mov r12=r2
   0x2000000800000801 <+17>:                nop.i 0x0
   0x2000000800000802 <+18>:                br.ret.sptk.many b0;;
End of assembler dump.
"""

-- 

  Sergei
Kevin Buettner via Gdb-patches Aug. 17, 2020, 8:57 p.m. | #7
On Sun, 16 Aug 2020 17:51:28 -0400
Simon Marchi <simark@simark.ca> wrote:

> On 2020-08-16 4:45 a.m., Sergei Trofimovich via Gdb-patches wrote:

> > On Tue, 19 May 2020 15:00:41 -0700

> > Kevin Buettner <kevinb@redhat.com> wrote:

> >   

> >> On Tue, 19 May 2020 22:27:10 +0100

> >> Sergei Trofimovich via Gdb-patches <gdb-patches@sourceware.org> wrote:

> >>  

> >>> From: Sergei Trofimovich <siarheit@google.com>

> >>>

> >>> On IA64 built failed as:

> >>>

> >>> ```

> >>> ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope

> >>>   352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))

> >>>       |                             ^~~~~~~~~~~~~~~~

> >>> ```

> >>>

> >>> The fix includes "gdbarch.h" header where symbol is declared.

> >>>

> >>> 	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used

> >>> 	'gdbarch_num_regs'.    

> >>

> >> Okay, but please capitalize "include" in the ChangeLog entry prior

> >> to pushing this change.  

> > 

> > Attached v2-* patch with capitalization changes.

> > 

> > I don't have a 'gdb' write access yet (I think), but I do have GCC one.

> > 

> > Should I request 'gdb' access as well as specified in

> >     https://sourceware.org/cgi-bin/pdw/ps_form.cgi ?  

> 

> Yes, that would be useful if you plan on contributing regularly.  If it's just

> a occasional patch, we can also push for you.  As you wish.


Sounds good! Got write access granted.

> > 

> > Thank you for the review!   

> 

> A few more styling nits:

> 

> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog

> > index 9cc7e44cba7..8865e6949d6 100644

> > --- a/gdb/ChangeLog

> > +++ b/gdb/ChangeLog

> > @@ -4564,6 +4564,11 @@

> >

> >  	* sparc64-tdep.c (adi_tag_fd): Update call to target_fileio_open.

> >

> > +2020-05-19  Sergei Trofimovich  <siarheit@google.com>

> > +

> > +	* gdb/ia64-linux-nat.c: Include "gdbarch.h" to declare used

> > +	'gdbarch_num_regs'.  

> 

> The file path should be relative to the ChangeLog location, so here just "ia64-linux-nat.c".

> 

> Also, make sure that your new entry is at the top of the file (here, it's at line 4564), and

> that you update the date to $TODAY when you push.  For these reasons, most people don't include

> the ChangeLog bits in the patch directly, but just include it in the commit log (as you did).

> Of course, when pushing the patch, then you need to insert it in the ChangeLog file.


All makes sense! Attached v3-* patch with date and path in ChanegLog change.

-- 

  Sergei
From f9b11e6b1833a6dc06a0af03bcb6bd42d7a2f009 Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <siarheit@google.com>

Date: Tue, 19 May 2020 22:19:45 +0100
Subject: [PATCH v3] gdb: fix IA64 build failure of linux-nat

On IA64 built failed as:

```
ia64-linux-nat.c:352:29: error: 'gdbarch_num_regs' was not declared in this scope
  352 |   if (regno < 0 || regno >= gdbarch_num_regs (gdbarch))
      |                             ^~~~~~~~~~~~~~~~
```

The fix includes "gdbarch.h" header where symbol is declared.

	* ia64-linux-nat.c: Include "gdbarch.h" to declare used
	'gdbarch_num_regs'.

Signed-off-by: Sergei Trofimovich <siarheit@google.com>

---
 gdb/ChangeLog        | 5 +++++
 gdb/ia64-linux-nat.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b097b325a22..3f681e19e5a 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-08-17  Sergei Trofimovich  <siarheit@google.com>
+
+	* ia64-linux-nat.c: Include "gdbarch.h" to declare used
+	'gdbarch_num_regs'.
+
 2020-08-17  Tom Tromey  <tromey@adacore.com>
 
 	* ada-varobj.c (ada_varobj_decode_var): Handle case where
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index 8f36ea78e76..b532e7a51c5 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -21,6 +21,7 @@
 #include "defs.h"
 #include "inferior.h"
 #include "target.h"
+#include "gdbarch.h"
 #include "gdbcore.h"
 #include "regcache.h"
 #include "ia64-tdep.h"
-- 
2.28.0
Simon Marchi Aug. 17, 2020, 9:04 p.m. | #8
On 2020-08-17 4:57 p.m., Sergei Trofimovich wrote:
> All makes sense! Attached v3-* patch with date and path in ChanegLog change.


Thanks, that LGTM, please go ahead and push.

Simon
Tom de Vries Aug. 18, 2020, 8:46 a.m. | #9
On 8/17/20 10:21 AM, Simon Marchi wrote:
> On 2020-08-17 4:54 a.m., Tom de Vries wrote:

>> Hi,

>>

>> IA64 support was just obsoleted in bfd (commit 73d0dc162e "Obsolete ia64").

>>

>> So, AFAIU, this should now be built with --enable-obsolete.

>>

>> What are the consequences for gdb IA64 patches ?

>>

>> Thanks,

>> - Tom

> 

> By transitivity, I'd say that the GDB port is also obsolete.  But as long as it's in

> the tree, it's fine to accept patches to keep it building (otherwise it's useless to

> keep it in the tree).

> 


Btw, I just found in gdb/configure.tgt:
...
    echo "*** Configuration $targ is obsolete." >&2
    echo "*** Support has been REMOVED." >&2
    exit 1
...
So, maybe in gdb a target is obsolete once it's removed?  There seems to
be no separate means to declare a target obsolete before it's removed.

> If you wanted to build GDB with --target=ia64-something-something, you'll need to pass

> --enable-obsolete in order to build BFD, which is a required dependency of GDB.  When we

> pass --enable-targets=all, it builds a GDB with ia64 support without requiring that

> --enable-obsolete flag though.

> 


Hmm, I find that last bit surprising.

> Do you know what's the BFD policy for obsolete configurations?  If they wanted to get

> rid of it completely, they would need to remove GDB support too, otherwise they'd break

> the build.  So should we eventually take the lead and remove support for it first?

> 


I don't know the policy, but the discussion about ia64 took place here (
https://sourceware.org/pipermail/binutils/2020-August/112825.html ,
helpfully titled "Time to obsolete arm-symbian?" ).  It was mentioned
that there was a segfault since at least binutils 2.31 on ia64, which
had not been addressed by anybody, and "Assuming no one cares enough
about ia64 to contribute fixes for the segfaults, ia64 would remain in
binutils until after the next release, at which point the ia64 support
code might be removed".

Thanks,
- Tom

> Sergei, quick survey: do you, or someone you know actually use GDB on ia64?  Or you just

> noticed it not building because you package it?

> 

> Simon

>

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ac0beef5ad..0d921da325 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@ 
+2020-05-19  Sergei Trofimovich  <siarheit@google.com>
+
+	* gdb/ia64-linux-nat.c: include "gdbarch.h" to declare used
+	'gdbarch_num_regs'.
+
 2020-05-19  Simon Marchi  <simon.marchi@efficios.com>
 
 	* dwarf2/read.c (quirk_rust_enum): Allocate enough fields.
diff --git a/gdb/ia64-linux-nat.c b/gdb/ia64-linux-nat.c
index 8f36ea78e7..b532e7a51c 100644
--- a/gdb/ia64-linux-nat.c
+++ b/gdb/ia64-linux-nat.c
@@ -21,6 +21,7 @@ 
 #include "defs.h"
 #include "inferior.h"
 #include "target.h"
+#include "gdbarch.h"
 #include "gdbcore.h"
 #include "regcache.h"
 #include "ia64-tdep.h"