Warnings in native MinGW32 build of GDB 7.8

Warnings in native MinGW32 build of GDB 7.8

[Eli Zaretskii]

I've built today a native MinGW32 GDB 7.8, and saw warnings about
incomplete argument types:

     In file included from defs.h:631,
		      from gdb.c:19:
     gdbarch.h:429: warning: parameter has incomplete type
     gdbarch.h:430: warning: parameter has incomplete type

     In file included from target-dcache.h:21,
		      from target-dcache.c:19:
     dcache.h:42: warning: parameter has incomplete type

I fixed that as below, but I wonder why no one else saw this.  is this
because I use an ancient version of GCC?

OK to commit the below (master and 7.8 branch), with suitable
ChangeLog entries?

--- gdb/dcache.c~0	2014-07-29 15:37:42.000000000 +0300
+++ gdb/dcache.c	2014-08-09 16:17:31.823000000 +0300
@@ -18,6 +18,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "target.h"	/* for 'enum target_xfer_status' */
 #include "dcache.h"
 #include "gdbcmd.h"
 #include <string.h>

--- gdb/defs.h~0	2014-07-29 15:37:42.000000000 +0300
+++ gdb/defs.h	2014-08-09 15:33:59.666750000 +0300
@@ -628,6 +628,7 @@
 #endif /* alloca not defined */
 
 /* Dynamic target-system-dependent parameters for GDB.  */
+#include "frame.h"	/* for 'struct frame_id' */
 #include "gdbarch.h"
 
 /* * Maximum size of a register.  Something small, but large enough for

--- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
+++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
@@ -16,6 +16,7 @@
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "defs.h"
+#include "target.h"	/* for 'enum target_xfer_status' */
 #include "target-dcache.h"
 #include "gdbcmd.h"
 #include "progspace.h"

Re: Warnings in native MinGW32 build of GDB 7.8

[Yao Qi]

On 08/09/2014 10:09 PM, Eli Zaretskii wrote:
> I fixed that as below, but I wonder why no one else saw this.  is this
> because I use an ancient version of GCC?

I don't see any warning in my mingw32 build.  I am using
i686-w64-mingw32-gcc 4.8.2.

> 
> OK to commit the below (master and 7.8 branch), with suitable
> ChangeLog entries?
> 
> --- gdb/dcache.c~0	2014-07-29 15:37:42.000000000 +0300
> +++ gdb/dcache.c	2014-08-09 16:17:31.823000000 +0300
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "target.h"	/* for 'enum target_xfer_status' */

I can see enum target_xfer_status is used in this c file.  This is good
to me.

>  #include "dcache.h"
>  #include "gdbcmd.h"
>  #include <string.h>
> 
> --- gdb/defs.h~0	2014-07-29 15:37:42.000000000 +0300
> +++ gdb/defs.h	2014-08-09 15:33:59.666750000 +0300
> @@ -628,6 +628,7 @@
>  #endif /* alloca not defined */
>  
>  /* Dynamic target-system-dependent parameters for GDB.  */
> +#include "frame.h"	/* for 'struct frame_id' */

It is unclear to me why do we need this include?

>  #include "gdbarch.h"
>  
>  /* * Maximum size of a register.  Something small, but large enough for
> 
> --- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
> +++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
> @@ -16,6 +16,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "target.h"	/* for 'enum target_xfer_status' */

enum target_xfer_status isn't used in target-dcache.c.  Do we really
need this?

-- 
Yao (齐尧)

Re: Warnings in native MinGW32 build of GDB 7.8

[Eli Zaretskii]

> Date: Wed, 13 Aug 2014 12:07:22 +0800
> From: Yao Qi <yao@codesourcery.com>
> 
> On 08/09/2014 10:09 PM, Eli Zaretskii wrote:
> > I fixed that as below, but I wonder why no one else saw this.  is this
> > because I use an ancient version of GCC?
> 
> I don't see any warning in my mingw32 build.  I am using
> i686-w64-mingw32-gcc 4.8.2.

Your GCC version is eons ahead of mine, and from a different distro on
top of that.

> > OK to commit the below (master and 7.8 branch), with suitable
> > ChangeLog entries?
> > 
> > --- gdb/dcache.c~0	2014-07-29 15:37:42.000000000 +0300
> > +++ gdb/dcache.c	2014-08-09 16:17:31.823000000 +0300
> > @@ -18,6 +18,7 @@
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >  
> >  #include "defs.h"
> > +#include "target.h"	/* for 'enum target_xfer_status' */
> 
> I can see enum target_xfer_status is used in this c file.  This is good
> to me.

Not sure what you mean here.  Do you agree with this change?  If not,
why not?

> >  #include "dcache.h"
> >  #include "gdbcmd.h"
> >  #include <string.h>
> > 
> > --- gdb/defs.h~0	2014-07-29 15:37:42.000000000 +0300
> > +++ gdb/defs.h	2014-08-09 15:33:59.666750000 +0300
> > @@ -628,6 +628,7 @@
> >  #endif /* alloca not defined */
> >  
> >  /* Dynamic target-system-dependent parameters for GDB.  */
> > +#include "frame.h"	/* for 'struct frame_id' */
> 
> It is unclear to me why do we need this include?

Because 'struct frame_id' is otherwise not defined, and I get warnings
like this one:

     In file included from defs.h:631,
		      from gdb.c:19:
     gdbarch.h:429: warning: parameter has incomplete type
     gdbarch.h:430: warning: parameter has incomplete type

> > --- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
> > +++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
> > @@ -16,6 +16,7 @@
> >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> >  
> >  #include "defs.h"
> > +#include "target.h"	/* for 'enum target_xfer_status' */
> 
> enum target_xfer_status isn't used in target-dcache.c.  Do we really
> need this?

It is used in dcache.h which target-dcache.c includes:

     In file included from target-dcache.h:21,
		      from target-dcache.c:19:
     dcache.h:42: warning: parameter has incomplete type

Thanks for reviewing the patch.

Re: Warnings in native MinGW32 build of GDB 7.8

[Joel Brobecker]

> > >  #include "defs.h"
> > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > 
> > I can see enum target_xfer_status is used in this c file.  This is good
> > to me.
> 
> Not sure what you mean here.  Do you agree with this change?  If not,
> why not?

I think Yao was agreeing with the patch. I agree with the change
as well, so you can commit that part right away.

> > >  /* Dynamic target-system-dependent parameters for GDB.  */
> > > +#include "frame.h"	/* for 'struct frame_id' */
> > 
> > It is unclear to me why do we need this include?
> 
> Because 'struct frame_id' is otherwise not defined, and I get warnings
> like this one:
> 
>      In file included from defs.h:631,
> 		      from gdb.c:19:
>      gdbarch.h:429: warning: parameter has incomplete type
>      gdbarch.h:430: warning: parameter has incomplete type

I think we should fix gdbarch.h to include frame.h instead, which
effectively means adjusting gdbarch.sh. The solution you chose seems
to be relying on an indirect include, which we really really try
to avoid. Yao's question is a good example of one of the reasons
why we avoid that; but there is also the fact that not everyone
will need frame.h's declarations.

> > > --- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
> > > +++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
> > > @@ -16,6 +16,7 @@
> > >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > >  
> > >  #include "defs.h"
> > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > 
> > enum target_xfer_status isn't used in target-dcache.c.  Do we really
> > need this?
> 
> It is used in dcache.h which target-dcache.c includes:
> 
>      In file included from target-dcache.h:21,
> 		      from target-dcache.c:19:
>      dcache.h:42: warning: parameter has incomplete type

Same here, dcache.h should be the one including target.h.

-- 
Joel

Re: Warnings in native MinGW32 build of GDB 7.8

[Eli Zaretskii]

> Date: Wed, 13 Aug 2014 10:42:27 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches@sourceware.org
> 
> > > >  #include "defs.h"
> > > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > > 
> > > I can see enum target_xfer_status is used in this c file.  This is good
> > > to me.
> > 
> > Not sure what you mean here.  Do you agree with this change?  If not,
> > why not?
> 
> I think Yao was agreeing with the patch. I agree with the change
> as well, so you can commit that part right away.
> 
> > > >  /* Dynamic target-system-dependent parameters for GDB.  */
> > > > +#include "frame.h"	/* for 'struct frame_id' */
> > > 
> > > It is unclear to me why do we need this include?
> > 
> > Because 'struct frame_id' is otherwise not defined, and I get warnings
> > like this one:
> > 
> >      In file included from defs.h:631,
> > 		      from gdb.c:19:
> >      gdbarch.h:429: warning: parameter has incomplete type
> >      gdbarch.h:430: warning: parameter has incomplete type
> 
> I think we should fix gdbarch.h to include frame.h instead, which
> effectively means adjusting gdbarch.sh. The solution you chose seems
> to be relying on an indirect include, which we really really try
> to avoid. Yao's question is a good example of one of the reasons
> why we avoid that; but there is also the fact that not everyone
> will need frame.h's declarations.
> 
> > > > --- gdb/target-dcache.c~0	2014-06-11 19:34:41.000000000 +0300
> > > > +++ gdb/target-dcache.c	2014-08-09 16:17:42.244875000 +0300
> > > > @@ -16,6 +16,7 @@
> > > >     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> > > >  
> > > >  #include "defs.h"
> > > > +#include "target.h"	/* for 'enum target_xfer_status' */
> > > 
> > > enum target_xfer_status isn't used in target-dcache.c.  Do we really
> > > need this?
> > 
> > It is used in dcache.h which target-dcache.c includes:
> > 
> >      In file included from target-dcache.h:21,
> > 		      from target-dcache.c:19:
> >      dcache.h:42: warning: parameter has incomplete type
> 
> Same here, dcache.h should be the one including target.h.

I can do the 1st and the 3rd parts, but I'd prefer not to touch
gdbarch.sh.  Could one of you please do that?

TIA

Re: Warnings in native MinGW32 build of GDB 7.8

[Joel Brobecker]

> I can do the 1st and the 3rd parts, but I'd prefer not to touch
> gdbarch.sh.  Could one of you please do that?

Let me do it now...

-- 
Joel

Re: Warnings in native MinGW32 build of GDB 7.8

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 205 bytes --]

> > I can do the 1st and the 3rd parts, but I'd prefer not to touch
> > gdbarch.sh.  Could one of you please do that?

Attached is a patch that does that. Can you confirm it fixes your
warnings?

-- 
Joel

[-- Attachment #2: 0001-Add-frame.h-include-in-gdbarch.h.patch --]
[-- Type: text/x-diff, Size: 1220 bytes --]

From ee8948e61a8047d67cf8ad32d3259dbe7951a527 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 13 Aug 2014 11:15:00 -0700
Subject: [PATCH] Add "frame.h" #include in gdbarch.h.

This include is needed because of gdbarch_dummy_id needs the full
definition of struct frame_id.

gdb/ChangeLog:

        * gdbarch.sh: #include "frame.h" in gdbarch.h.  Delete "struct
        frame_info" advance declaration.

Tested on x86_64-linux by rebuilding GDB.
---
 gdb/gdbarch.h  | 3 ++-
 gdb/gdbarch.sh | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5388e6e..0303b2e 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -35,9 +35,10 @@
 #ifndef GDBARCH_H
 #define GDBARCH_H
 
+#include "frame.h"
+
 struct floatformat;
 struct ui_file;
-struct frame_info;
 struct value;
 struct objfile;
 struct obj_section;
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 61d0781..2a8bca8 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1124,9 +1124,10 @@ cat <<EOF
 #ifndef GDBARCH_H
 #define GDBARCH_H
 
+#include "frame.h"
+
 struct floatformat;
 struct ui_file;
-struct frame_info;
 struct value;
 struct objfile;
 struct obj_section;
-- 
1.9.1

Re: Warnings in native MinGW32 build of GDB 7.8

[Eli Zaretskii]

> Date: Wed, 13 Aug 2014 11:26:55 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: yao@codesourcery.com, gdb-patches@sourceware.org
> 
> Attached is a patch that does that. Can you confirm it fixes your
> warnings?

Yes, it does.  Thanks!

Re: Warnings in native MinGW32 build of GDB 7.8

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

> > Attached is a patch that does that. Can you confirm it fixes your
> > warnings?
> 
> Yes, it does.  Thanks!

Thanks, Eli. I just pushed that patch.
And since I was missing one piece in the ChangeLog entry, I am
attaching the patch I ended up pushing.  The diff itself is the same
though.

-- 
Joel

[-- Attachment #2: 0001-Add-frame.h-include-in-gdbarch.h.patch --]
[-- Type: text/x-diff, Size: 1702 bytes --]

From eb7a547ad40c97ca306e29c94678e4eab1822089 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Wed, 13 Aug 2014 11:15:00 -0700
Subject: [PATCH] Add "frame.h" #include in gdbarch.h.

This include is needed because gdbarch_dummy_id needs the full
definition of struct frame_id.

gdb/ChangeLog:

        * gdbarch.sh: #include "frame.h" in gdbarch.h.  Delete "struct
        frame_info" partial declaration.
        * gdbarch.h: Regenerate.

Tested on x86_64-linux by rebuilding GDB.
---
 gdb/ChangeLog  | 6 ++++++
 gdb/gdbarch.h  | 3 ++-
 gdb/gdbarch.sh | 3 ++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 53cc615..6aa6592 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2014-08-15  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdbarch.sh: #include "frame.h" in gdbarch.h.  Delete "struct
+	frame_info" partial declaration.
+	* gdbarch.h: Regenerate.
+
 2014-08-15  Yao Qi  <yao@codesourcery.com>
 
 	* dwarf2read.c (dwarf_decode_lines_1): Remove parameter 'pst'.
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 5388e6e..0303b2e 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -35,9 +35,10 @@
 #ifndef GDBARCH_H
 #define GDBARCH_H
 
+#include "frame.h"
+
 struct floatformat;
 struct ui_file;
-struct frame_info;
 struct value;
 struct objfile;
 struct obj_section;
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 61d0781..2a8bca8 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1124,9 +1124,10 @@ cat <<EOF
 #ifndef GDBARCH_H
 #define GDBARCH_H
 
+#include "frame.h"
+
 struct floatformat;
 struct ui_file;
-struct frame_info;
 struct value;
 struct objfile;
 struct obj_section;
-- 
1.9.1

Re: Warnings in native MinGW32 build of GDB 7.8

[Eli Zaretskii]

> Date: Fri, 15 Aug 2014 05:33:00 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: yao@codesourcery.com, gdb-patches@sourceware.org
> 
> > > Attached is a patch that does that. Can you confirm it fixes your
> > > warnings?
> > 
> > Yes, it does.  Thanks!
> 
> Thanks, Eli. I just pushed that patch.

Thanks, I pushed the other part, and also cherry-picked both to the
7.8 branch.