Please define thread_info as struct thread_info (and other stuff)

  Hello,

When building gdb version 8.2 on GNU/Hurd several problems arise that need to be
solved. One problem is that most files in gdb/ have sloppy definitions of struct
thread_info, causing a build stop fore every occurrence. They are excluding the
struct part, making the build (and port) on Hurd very cumbersome. Can you please
fix all bad references to thread_info upstream? I don't really understand how
gcc can let these errors pass on GNU/Linux.

Attached are a lot of patches and files fixing these problems, 61 in total. A
test build of Debian version gdb-8.2-1 on GNU/Linux with the attached patches
(and patched files) applied works fine! 

Furthermore the script gdbarch.sh needs patching and the resulting files
gdbarch.{c,h} are used in the Linux build. They are not included here due to
size constraints. The generated file target-delegates.c was also used after
patching target.h and generating it with: make-target-delegates target.h >
target-delegates.c. Not included here either, also due to size constraints.

Attached are also four old patches needed to build the 8.1 version of gdb:
gdb_configure.nat.patch
gdb-PATH_MAX.patch
gnu-nat.c.patch
struct-thread_info.patch
and a very old Debian PATH_MAX patch: solve_PATH_MAX_issue.patch. Includes is
also the original and new Debian 8.2-1 patch file: series.

More patches will be needed for a successful port of gdb-8.2 to GNU/hurd, but
please fix the above problems upstream first.

Thanks!
gdb-fortran-main.patch
solve_PATH_MAX_issue.patch
gdb-6.5-bz185337-resolve-tls-without-debuginfo-v2.patch
python-config.patch
gdb-glibc-vdso-workaround.patch
load-versioned-libcc1.patch
gdb_configure.nat.patch
gdb-PATH_MAX.patch
gnu-nat.c.patch
struct-thread_info.patch
gdb_gdbarch.sh.patch
gdb_regcache.h.patch
gdb_dummy-frame.h.patch
#gdb_gdbarch.h.patch
gdb_breakpoint.h.patch
gdb_target.h.patch
gdb_gnu-nat.h.patch
gdb_gnu-nat.c.patch
gdb_ada-lang.h.patch
gdb_mi_mi-cmd-var.c.patch
gdb_mi_mi-interp.c.patch
gdb_mi_mi-main.c.patch
gdb_python_python-internal.h.patch
gdb_python_py-inferior.c.patch
gdb_python_py-record.h.patch
gdb_python_py-record.c.patch
gdb_python_py-record-btrace.c.patch
gdb_python_py-threadevent.c.patch
gdb_elfread.c.patch
gdb_ada-tasks.c.patch
gdb_inline-frame.h.patch
gdb_breakpoint.c.patch
gdb_btrace.c.patch
gdb_corelow.c.patch
gdb_dummy-frame.c.patch
gdb_frame.c.patch
gdb_infcall.c.patch
gdb_infcmd.c.patch
gdb_inferior.c.patch
gdb_infrun.c.patch
gdb_inline-frame.c.patch
gdb_record-btrace.c.patch
gdb_regcache.c.patch
gdb_remote.c.patch
gdb_target.c.patch
gdb_thread.c.patch
gdb_top.c.patch
gdb_varobj.c.patch
gdb_compile_compile-object-load.c.patch
gdb_inflow.c.patch
gdb_aarch64-linux-tdep.c.patch
gdb_arm-linux-tdep.c.patch
gdb_amd64-linux-tdep.c.patch
gdb_i386-linux-tdep.c.patch
gdb_mips-linux-tdep.c.patch
gdb_ppc-linux-tdep.c.patch
gdb_ravenscar-thread.c.patch
gdb_s390-linux-tdep.c.patch
gdb_sparc64-linux-tdep.c.patch
gdb_sparc-linux-tdep.c.patch
gdb_i386-gnu-nat.c.patch
gdb-fortran-main.patch
solve_PATH_MAX_issue.patch
gdb-6.5-bz185337-resolve-tls-without-debuginfo-v2.patch
python-config.patch
gdb-glibc-vdso-workaround.patch
load-versioned-libcc1.patch
gdb_configure.nat.patch
gdb-PATH_MAX.patch
gnu-nat.c.patch
struct-thread_info.patch
  

>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:

Svante> When building gdb version 8.2 on GNU/Hurd several problems arise that need to be
Svante> solved. One problem is that most files in gdb/ have sloppy definitions of struct
Svante> thread_info, causing a build stop fore every occurrence. They are excluding the
Svante> struct part, making the build (and port) on Hurd very cumbersome. Can you please
Svante> fix all bad references to thread_info upstream? I don't really understand how
Svante> gcc can let these errors pass on GNU/Linux.

In C++ there's an implicit typedef for a struct tag, so "struct
thread_info" can also just be referred to as "thread_info".  So, I tend
to think the code is fine and something else is going wrong.

You didn't post the text of the error or the version of the compiler
that you are using, so it's hard to say more about what the problem is.
Could you send that info?

Svante> More patches will be needed for a successful port of gdb-8.2 to GNU/hurd, but
Svante> please fix the above problems upstream first.

I didn't go through all the patches, but I suggest sending a series
rather than one large message; following the contribution checklist; and
finally, working off git master rather than a branch.

thanks,
Tom
  

On Sat, 2018-12-15 at 15:48 -0700, Tom Tromey wrote:
> > > > > > "Svante" == Svante Signell <svante.signell@gmail.com> writes:
> 
> Svante> When building gdb version 8.2 on GNU/Hurd several problems arise that
> need to be
> Svante> solved. One problem is that most files in gdb/ have sloppy definitions
> of struct
> Svante> thread_info, causing a build stop fore every occurrence. They are
> excluding the
> Svante> struct part, making the build (and port) on Hurd very cumbersome. Can
> you please
> Svante> fix all bad references to thread_info upstream? I don't really
> understand how
> Svante> gcc can let these errors pass on GNU/Linux.
> 
> In C++ there's an implicit typedef for a struct tag, so "struct
> thread_info" can also just be referred to as "thread_info".  So, I tend
> to think the code is fine and something else is going wrong.
> 
> You didn't post the text of the error or the version of the compiler
> that you are using, so it's hard to say more about what the problem is.
> Could you send that info?

A typical error output:
CXX    target.o
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.c: In function ‘int
dispose_inferior(inferior*, void*)’:
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: error: ‘thread’ was not
declared 
in this scope
   thread_info *thread = any_thread_of_inferior (inf);
                ^~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: note: suggested alternative:
‘myread’
   thread_info *thread = any_thread_of_inferior (inf);
                ^~~~~~
                myread

Sorry, but the compiler directive is hidden from the output (by libtool??).

ii  gcc            4:8.2.0-2    hurd-i386    GNU C compiler
ii  g++            4:8.2.0-2    hurd-i386    GNU C++ compiler

I would be very happy if you could be consistent in the coding: Either use
struct thread_info everywhere or not.

Thanks!
  

On 2018-12-15 6:01 p.m., Svante Signell wrote:
> A typical error output:
> CXX    target.o
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c: In function ‘int
> dispose_inferior(inferior*, void*)’:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: error: ‘thread’ was not
> declared 
> in this scope
>    thread_info *thread = any_thread_of_inferior (inf);
>                 ^~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: note: suggested alternative:
> ‘myread’
>    thread_info *thread = any_thread_of_inferior (inf);
>                 ^~~~~~
>                 myread
> 
> Sorry, but the compiler directive is hidden from the output (by libtool??).

You can do "make V=1" to show the compiler invocation (same as if we would use automake).
> 
> ii  gcc            4:8.2.0-2    hurd-i386    GNU C compiler
> ii  g++            4:8.2.0-2    hurd-i386    GNU C++ compiler
> 
> I would be very happy if you could be consistent in the coding: Either use
> struct thread_info everywhere or not.

Well, we started with the "struct" keyword everywhere (because C), and many people get
rid of it in the parts of the code they touch (though not everyone, and I don't turn
down a patch just for this).  We also won't make a big change to remove the use of
"struct" throughout, as it would be a lot of work for nothing, it would create
unnecessary conflicts for people who maintain external patches and it would pollute
the history when git-blaming.

What you compiler is doing is very strange, as Tom mentioned the use of the "struct"
or "class" keyword when declaring a variable in C++ is optional.  What does your
compiler say when compiling the following code (g++ -c foo.cpp)?

struct Foo
{
	int a;
};

void func()
{
	Foo f2;
}


Simon
  

On Sat, 2018-12-15 at 23:31 -0500, Simon Marchi wrote:
> On 2018-12-15 6:01 p.m., Svante Signell wrote:
> > A typical error output:
> > CXX    target.o
> > /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c: In function ‘int
> > dispose_inferior(inferior*, void*)’:
> > /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: error: ‘thread’ was not
> > declared 
> > in this scope
> >    thread_info *thread = any_thread_of_inferior (inf);
> >                 ^~~~~~
> > /home/srs/DEBs/gdb/gdb-8.2/gdb/target.c:2028:16: note: suggested
> > alternative:
> > ‘myread’
> >    thread_info *thread = any_thread_of_inferior (inf);
> >                 ^~~~~~
> >                 myread
> > 
> > Sorry, but the compiler directive is hidden from the output (by libtool??).
> 
> You can do "make V=1" to show the compiler invocation (same as if we would use
> automake).

Here is an example output with make make V=1 -C build/objdir/gdb/ elfread.o

i686-gnu-g++ -x c++  -g -O2 -fdebug-prefix-map=/home/srs/DEBs/gdb/gdb-8.2=.
-fstack-protector-strong -Wformat -Werror=format-security -D_GNU_SOURCE  -I.
-I/home/srs/DEBs/gdb/gdb-8.2/gdb -I/home/srs/DEBs/gdb/gdb-8.2/gdb/common
-I/home/srs/DEBs/gdb/gdb-8.2/gdb/config -DLOCALEDIR="\"/usr/share/locale\""
-DHAVE_CONFIG_H -I/home/srs/DEBs/gdb/gdb-8.2/gdb/../include/opcode
-I/home/srs/DEBs/gdb/gdb-8.2/gdb/../opcodes/..   -I../bfd
-I/home/srs/DEBs/gdb/gdb-8.2/gdb/../bfd -I/home/srs/DEBs/gdb/gdb-
8.2/gdb/../include -I../libdecnumber -I/home/srs/DEBs/gdb/gdb-
8.2/gdb/../libdecnumber  -I/home/srs/DEBs/gdb/gdb-8.2/gdb/gnulib/import -Ibuild-
gnulib/import   -DTUI=1  -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC  -
I/usr/include/python3.7m -I/usr/include/python3.7m -Wall -Wpointer-arith -Wno-
unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts
-Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-
compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wsuggest-override
-Wimplicit-fallthrough=3 -Wduplicated-cond -Wformat-nonliteral  -c -o elfread.o
-MT elfread.o -MMD -MP -MF ./.deps/elfread.Tpo /home/srs/DEBs/gdb/gdb-
8.2/gdb/elfread.c
In file included from ./nm.h:25,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/defs.h:420,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:22:
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:46: warning: ‘get_thread_regcache’
initialized and declared ‘extern’
 extern struct regcache *get_thread_regcache (thread_info *thread);
                                              ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:46: error: ‘regcache*
get_thread_regcache’ redeclared as different kind of symbol
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:32:25: note: previous declaration
‘regcache* get_thread_regcache(ptid_t)’
 extern struct regcache *get_thread_regcache (ptid_t ptid);
                         ^~~~~~~~~~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:59: error: ‘thread’ was not
declared in this scope
 extern struct regcache *get_thread_regcache (thread_info *thread);
                                                           ^~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:59: note: suggested alternative:
‘thread_t’
 extern struct regcache *get_thread_regcache (thread_info *thread);
                                                           ^~~~~~
                                                           thread_t
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:144:14: error: ‘function_view’ in
namespace ‘gdb’ does not name a template type
 typedef gdb::function_view<register_status (int regnum, gdb_byte *buf)>
              ^~~~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:228:14: error: ‘register_read_ftype’
has not been declared
   void save (register_read_ftype cooked_read);
              ^~~~~~~~~~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:422:49: error: ‘register_read_ftype’
has not been declared
   readonly_detached_regcache (gdbarch *gdbarch, register_read_ftype
cooked_read)
                                                 ^~~~~~~~~~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:39: error: variable or field
‘registers_changed_thread’ declared void
 extern void registers_changed_thread (thread_info *thread);
                                       ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:52: error: ‘thread’ was not
declared in this scope
 extern void registers_changed_thread (thread_info *thread);
                                                    ^~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:52: note: suggested alternative:
‘thread_t’
 extern void registers_changed_thread (thread_info *thread);
                                                    ^~~~~~
                                                    thread_t
In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:44,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/progspace.h:24,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/objfiles.h:27,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:30:
/home/srs/DEBs/gdb/gdb-8.2/gdb/breakpoint.h:950:21: error: ‘thread_info’ is not
a type
       CORE_ADDR pc, thread_info *thread,
                     ^~~~~~~~~~~
In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/progspace.h:24,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/objfiles.h:27,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:30:
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:44: error: ‘extra_thread_info’
declared as a ‘virtual’ field
     virtual const char *extra_thread_info (thread_info *)
                                            ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:25: error: expected ‘;’ at end of
member declaration
     virtual const char *extra_thread_info (thread_info *)
                         ^~~~~~~~~~~~~~~~~
                                          ;
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:55: error: expected ‘)’ before ‘*’
token
     virtual const char *extra_thread_info (thread_info *)
                                           ~           ^~
                                                       )
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:38: error: ‘thread_name’ declared as
a ‘virtual’ field
     virtual const char *thread_name (thread_info *)
                                      ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:25: error: expected ‘;’ at end of
member declaration
     virtual const char *thread_name (thread_info *)
                         ^~~~~~~~~~~
                                    ;
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:49: error: expected ‘)’ before ‘*’
token
     virtual const char *thread_name (thread_info *)
                                     ~           ^~
                                                 )
/home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:656:13: error: ‘thread_info’ does not
name a type
     virtual thread_info *thread_handle_to_thread_info (const gdb_byte *,
             ^~~~~~~~~~~
In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/infcall.h:23,
                 from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:42:
/home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:38:10: error: ‘thread_info’ is not
a type
          thread_info *thread);
          ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:49:49: error: ‘thread_info’ is not
a type
 extern void dummy_frame_pop (frame_id dummy_id, thread_info *thread);
                                                 ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:51:53: error: ‘thread_info’ is not
a type
 extern void dummy_frame_discard (frame_id dummy_id, thread_info *thread);
                                                     ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:67:12: error: ‘thread_info’ is not
a type
            thread_info *thread,
            ^~~~~~~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c: In function ‘void
elf_gnu_ifunc_resolver_return_stop(breakpoint*)’:
/home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:975:16: error: ‘thread’ was not
declared in this scope
   thread_info *thread = inferior_thread ();
                ^~~~~~
/home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:975:16: note: suggested alternative:
‘myread’
   thread_info *thread = inferior_thread ();
                ^~~~~~
                myread
Makefile:1651: recipe for target 'elfread.o' failed
make: *** [elfread.o] Error 1

In gdb/regcache.h we have:
extern struct regcache *get_thread_regcache (ptid_t ptid);
extern struct regcache *get_thread_regcache (struct thread_info *thread);

> What you compiler is doing is very strange, as Tom mentioned the use of the
> "struct" or "class" keyword when declaring a variable in C++ is
> optional.  What does your
> compiler say when compiling the following code (g++ -c foo.cpp)?
> 
> struct Foo
> {
> 	int a;
> };
> 
> void func()
> {
> 	Foo f2;
> }

g++ -c -Wall foo.cpp 
foo.cpp: In function ‘void func()’:
foo.cpp:8:13: warning: unused variable ‘f2’ [-Wunused-variable]
         Foo f2;
  

On 2018-12-16 12:15 a.m., Svante Signell wrote:
> Here is an example output with make make V=1 -C build/objdir/gdb/ elfread.o
> 
> i686-gnu-g++ -x c++  -g -O2 -fdebug-prefix-map=/home/srs/DEBs/gdb/gdb-8.2=.
> -fstack-protector-strong -Wformat -Werror=format-security -D_GNU_SOURCE  -I.
> -I/home/srs/DEBs/gdb/gdb-8.2/gdb -I/home/srs/DEBs/gdb/gdb-8.2/gdb/common
> -I/home/srs/DEBs/gdb/gdb-8.2/gdb/config -DLOCALEDIR="\"/usr/share/locale\""
> -DHAVE_CONFIG_H -I/home/srs/DEBs/gdb/gdb-8.2/gdb/../include/opcode
> -I/home/srs/DEBs/gdb/gdb-8.2/gdb/../opcodes/..   -I../bfd
> -I/home/srs/DEBs/gdb/gdb-8.2/gdb/../bfd -I/home/srs/DEBs/gdb/gdb-
> 8.2/gdb/../include -I../libdecnumber -I/home/srs/DEBs/gdb/gdb-
> 8.2/gdb/../libdecnumber  -I/home/srs/DEBs/gdb/gdb-8.2/gdb/gnulib/import -Ibuild-
> gnulib/import   -DTUI=1  -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC  -
> I/usr/include/python3.7m -I/usr/include/python3.7m -Wall -Wpointer-arith -Wno-
> unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts
> -Wempty-body -Wunused-but-set-parameter -Wunused-but-set-variable -Wno-sign-
> compare -Wno-narrowing -Wno-error=maybe-uninitialized -Wsuggest-override
> -Wimplicit-fallthrough=3 -Wduplicated-cond -Wformat-nonliteral  -c -o elfread.o
> -MT elfread.o -MMD -MP -MF ./.deps/elfread.Tpo /home/srs/DEBs/gdb/gdb-
> 8.2/gdb/elfread.c
> In file included from ./nm.h:25,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/defs.h:420,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:22:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:46: warning: ‘get_thread_regcache’
> initialized and declared ‘extern’
>  extern struct regcache *get_thread_regcache (thread_info *thread);
>                                               ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:46: error: ‘regcache*
> get_thread_regcache’ redeclared as different kind of symbol
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:32:25: note: previous declaration
> ‘regcache* get_thread_regcache(ptid_t)’
>  extern struct regcache *get_thread_regcache (ptid_t ptid);
>                          ^~~~~~~~~~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:59: error: ‘thread’ was not
> declared in this scope
>  extern struct regcache *get_thread_regcache (thread_info *thread);
>                                                            ^~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:59: note: suggested alternative:
> ‘thread_t’
>  extern struct regcache *get_thread_regcache (thread_info *thread);
>                                                            ^~~~~~
>                                                            thread_t
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:144:14: error: ‘function_view’ in
> namespace ‘gdb’ does not name a template type
>  typedef gdb::function_view<register_status (int regnum, gdb_byte *buf)>
>               ^~~~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:228:14: error: ‘register_read_ftype’
> has not been declared
>    void save (register_read_ftype cooked_read);
>               ^~~~~~~~~~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:422:49: error: ‘register_read_ftype’
> has not been declared
>    readonly_detached_regcache (gdbarch *gdbarch, register_read_ftype
> cooked_read)
>                                                  ^~~~~~~~~~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:39: error: variable or field
> ‘registers_changed_thread’ declared void
>  extern void registers_changed_thread (thread_info *thread);
>                                        ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:52: error: ‘thread’ was not
> declared in this scope
>  extern void registers_changed_thread (thread_info *thread);
>                                                     ^~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:439:52: note: suggested alternative:
> ‘thread_t’
>  extern void registers_changed_thread (thread_info *thread);
>                                                     ^~~~~~
>                                                     thread_t
> In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:44,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/progspace.h:24,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/objfiles.h:27,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:30:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/breakpoint.h:950:21: error: ‘thread_info’ is not
> a type
>        CORE_ADDR pc, thread_info *thread,
>                      ^~~~~~~~~~~
> In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/progspace.h:24,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/objfiles.h:27,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:30:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:44: error: ‘extra_thread_info’
> declared as a ‘virtual’ field
>      virtual const char *extra_thread_info (thread_info *)
>                                             ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:25: error: expected ‘;’ at end of
> member declaration
>      virtual const char *extra_thread_info (thread_info *)
>                          ^~~~~~~~~~~~~~~~~
>                                           ;
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:652:55: error: expected ‘)’ before ‘*’
> token
>      virtual const char *extra_thread_info (thread_info *)
>                                            ~           ^~
>                                                        )
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:38: error: ‘thread_name’ declared as
> a ‘virtual’ field
>      virtual const char *thread_name (thread_info *)
>                                       ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:25: error: expected ‘;’ at end of
> member declaration
>      virtual const char *thread_name (thread_info *)
>                          ^~~~~~~~~~~
>                                     ;
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:654:49: error: expected ‘)’ before ‘*’
> token
>      virtual const char *thread_name (thread_info *)
>                                      ~           ^~
>                                                  )
> /home/srs/DEBs/gdb/gdb-8.2/gdb/target.h:656:13: error: ‘thread_info’ does not
> name a type
>      virtual thread_info *thread_handle_to_thread_info (const gdb_byte *,
>              ^~~~~~~~~~~
> In file included from /home/srs/DEBs/gdb/gdb-8.2/gdb/infcall.h:23,
>                  from /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:42:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:38:10: error: ‘thread_info’ is not
> a type
>           thread_info *thread);
>           ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:49:49: error: ‘thread_info’ is not
> a type
>  extern void dummy_frame_pop (frame_id dummy_id, thread_info *thread);
>                                                  ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:51:53: error: ‘thread_info’ is not
> a type
>  extern void dummy_frame_discard (frame_id dummy_id, thread_info *thread);
>                                                      ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/dummy-frame.h:67:12: error: ‘thread_info’ is not
> a type
>             thread_info *thread,
>             ^~~~~~~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c: In function ‘void
> elf_gnu_ifunc_resolver_return_stop(breakpoint*)’:
> /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:975:16: error: ‘thread’ was not
> declared in this scope
>    thread_info *thread = inferior_thread ();
>                 ^~~~~~
> /home/srs/DEBs/gdb/gdb-8.2/gdb/elfread.c:975:16: note: suggested alternative:
> ‘myread’
>    thread_info *thread = inferior_thread ();
>                 ^~~~~~
>                 myread
> Makefile:1651: recipe for target 'elfread.o' failed
> make: *** [elfread.o] Error 1
> 
> In gdb/regcache.h we have:
> extern struct regcache *get_thread_regcache (ptid_t ptid);
> extern struct regcache *get_thread_regcache (struct thread_info *thread);
> 
>> What you compiler is doing is very strange, as Tom mentioned the use of the
>> "struct" or "class" keyword when declaring a variable in C++ is
>> optional.  What does your
>> compiler say when compiling the following code (g++ -c foo.cpp)?
>>
>> struct Foo
>> {
>> 	int a;
>> };
>>
>> void func()
>> {
>> 	Foo f2;
>> }
> 
> g++ -c -Wall foo.cpp 
> foo.cpp: In function ‘void func()’:
> foo.cpp:8:13: warning: unused variable ‘f2’ [-Wunused-variable]
>          Foo f2;
> 
> 

Sorry, I don't have anything to add, except that it looks like a compiler
problem.  I'm compiling the same code with the same compiler version (gcc 8.2),
so I'm at lost here.  I'd try to reduce to a minimal reproducer to try to understand
the problem better...

Simon
  

On Dez 15 2018, Svante Signell <svante.signell@gmail.com> wrote:

> Index: gdb-8.2/gdb/regcache.h
> ===================================================================
> --- gdb-8.2.orig/gdb/regcache.h
> +++ gdb-8.2/gdb/regcache.h
> @@ -21,6 +21,7 @@
>  #define REGCACHE_H
>  
>  #include "common-regcache.h"
> +#include "function-view.h"
>  #include <forward_list>
>  
>  struct regcache;
> @@ -32,7 +33,7 @@ extern struct regcache *get_current_regc
>  extern struct regcache *get_thread_regcache (ptid_t ptid);
>  
>  /* Get the regcache of THREAD.  */
> -extern struct regcache *get_thread_regcache (thread_info *thread);
> +extern struct regcache *get_thread_regcache (struct thread_info *thread);
>  
>  extern struct regcache *get_thread_arch_regcache (ptid_t, struct gdbarch *);
>  extern struct regcache *get_thread_arch_aspace_regcache (ptid_t,
> @@ -436,7 +437,7 @@ extern void registers_changed_ptid (ptid
>  
>  /* Indicate that registers of THREAD may have changed, so invalidate
>     the cache.  */
> -extern void registers_changed_thread (thread_info *thread);
> +extern void registers_changed_thread (struct thread_info *thread);
>  
>  /* An abstract base class for register dump.  */

Try adding a forward declaration of struct thead_info.  Note that
config/i386/nm-i386gnu.h includes "regcache.h", making it unique among
the nm.h files.

Andreas.
  

>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:

Svante> /home/srs/DEBs/gdb/gdb-8.2/gdb/regcache.h:35:46: warning: ‘get_thread_regcache’
Svante> initialized and declared ‘extern’

Ok, I was able to reproduce this error with:

    int Foo;

    struct Foo
    {
      int a;
    };

    extern int f(Foo x);

I didn't see a definition of thread_info anywhere, but maybe somehow it
comes from one of your system headers.  You could compile with 'gcc -E'
and search for it.

clang will tell you where the conflicting definition appears; I've filed
a gcc bug about this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88520

Tom
  

On Sun, 2018-12-16 at 10:20 +0100, Andreas Schwab wrote:
> On Dez 15 2018, Svante Signell <svante.signell@gmail.com> wrote:

> 
> Try adding a forward declaration of struct thead_info.  Note that
> config/i386/nm-i386gnu.h includes "regcache.h", making it unique among
> the nm.h files.

From what I've leraned forward declarations is bad coding, an should be avoided
as much as possible. Right or wrong?

Furthermore, not defining thread_info as struct everywhere is in my opinion very
lazy coding. Another issue is to compile C-code (and C++-code) in *.c files.
Please rename these to *.cpp (and eventually the header files to *.hpp)! As it
is now it is very confusing.

Finally, I've found the problem (but no workaround yet): thread_info is an RPC
on GNU/Hurs, and including mach.h in gdb/config/i386/nm-i386gnu.h:#include
<mach.h> further includes <mach/mach_interface.h> which has the conflicting name
of that RPC:
kern_return_t thread_info
(
        mach_port_t target_thread,
        int flavor,
        thread_info_t thread_info_out,
        mach_msg_type_number_t *thread_info_outCnt
);

Please reconsider your coding stype for gdb.

Thanks!
  

>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:

>> Try adding a forward declaration of struct thead_info.  Note that
>> config/i386/nm-i386gnu.h includes "regcache.h", making it unique among
>> the nm.h files.

Svante> From what I've leraned forward declarations is bad coding, an should be avoided
Svante> as much as possible. Right or wrong?

Opinions vary -- I think the Google style has a rule against it? -- but
gdb generally does not avoid it.

Svante> Furthermore, not defining thread_info as struct everywhere is in my opinion very
Svante> lazy coding.

I disagree here, C++ provides the typedef, there's nothing really wrong
with using it.

Svante> Another issue is to compile C-code (and C++-code) in *.c files.
Svante> Please rename these to *.cpp (and eventually the header files to *.hpp)! As it
Svante> is now it is very confusing.

I think this was discussed When we made this switch, and the conclusion
was that a mass rename would be worse.

Svante> Finally, I've found the problem (but no workaround yet): thread_info is an RPC
Svante> on GNU/Hurs, and including mach.h in gdb/config/i386/nm-i386gnu.h:#include
Svante> <mach.h> further includes <mach/mach_interface.h> which has the conflicting name
Svante> of that RPC:
Svante> kern_return_t thread_info
Svante> (

Typical answers for this kind of thing are either to segregate the use
of the system header somehow, or maybe namespacing or some other kind of
renaming.  I haven't looked into the details much in this case I'm
afraid.

Tom
  

On Sun, 2018-12-16 at 16:10 -0700, Tom Tromey wrote:
> > > > > > "Svante" == Svante Signell <svante.signell@gmail.com> writes:
> > > Try adding a forward declaration of struct thead_info.  Note that
> > > config/i386/nm-i386gnu.h includes "regcache.h", making it unique among
> > > the nm.h files.
> 
> Svante> From what I've learned forward declarations is bad coding, an should
> be avoided
> Svante> as much as possible. Right or wrong?
> 
> Opinions vary -- I think the Google style has a rule against it? -- but
> gdb generally does not avoid it.
> 
> Svante> Furthermore, not defining thread_info as struct everywhere is in my
> opinion very
> Svante> lazy coding.
> 
> I disagree here, C++ provides the typedef, there's nothing really wrong
> with using it.

Well, the basis is C-code, where that is a requirement.

> Svante> Another issue is to compile C-code (and C++-code) in *.c files.
> Svante> Please rename these to *.cpp (and eventually the header files to
> *.hpp)! As it is now it is very confusing.
> 
> I think this was discussed When we made this switch, and the conclusion
> was that a mass rename would be worse.

Do you really need to compile all files with a C++ compiler? Pure C-code could
still be compiled with a C compiler.
 
> Svante> Finally, I've found the problem (but no workaround yet): thread_info
> is an RPC on GNU/Hurd, and including mach.h in gdb/config/i386/nm-
> i386gnu.h:#include <mach.h> further includes <mach/mach_interface.h> which has
> the conflicting name Svante> of that RPC: kern_return_t thread_info
> 
> Typical answers for this kind of thing are either to segregate the use
> of the system header somehow, or maybe namespacing or some other kind of
> renaming.  I haven't looked into the details much in this case I'm
> afraid.

As I see it you need to:

1) Apply the patches submitted earlier in this thread using struct thread_info
consistently everywhere (simplest).
2) Rename all usage of the struct thread_info to something else e.g. struct
gdb_thread_info (not future-proof though).
3) Create a gdb namespace for all your code to avoid conflicts.
4) Segregate the use of system header files as you write above. Dunno how to do
that though, but some of you should.

Thanks!
  

On 12/17/18 12:51 PM, Svante Signell wrote:
> On Sun, 2018-12-16 at 16:10 -0700, Tom Tromey wrote:
>>>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:
>> Svante> Another issue is to compile C-code (and C++-code) in *.c files.
>> Svante> Please rename these to *.cpp (and eventually the header files to
>> *.hpp)! As it is now it is very confusing.
>>
>> I think this was discussed When we made this switch, and the conclusion
>> was that a mass rename would be worse.
> 
> Do you really need to compile all files with a C++ compiler? Pure C-code could
> still be compiled with a C compiler.

GDB isn't written in C anymore.  target_ops is a class with virtual methods,
the use of gdb::unique_ptr<> (basically std::unique_ptr<>) is rather
ubiquitous now, etc.  In fact, I think we even require C++11 and not just
C++03.

>> Svante> Finally, I've found the problem (but no workaround yet): thread_info
>> is an RPC on GNU/Hurd, and including mach.h in gdb/config/i386/nm-
>> i386gnu.h:#include <mach.h> further includes <mach/mach_interface.h> which has
>> the conflicting name Svante> of that RPC: kern_return_t thread_info
>>
>> Typical answers for this kind of thing are either to segregate the use
>> of the system header somehow, or maybe namespacing or some other kind of
>> renaming.  I haven't looked into the details much in this case I'm
>> afraid.
> 
> As I see it you need to:
> 
> 1) Apply the patches submitted earlier in this thread using struct thread_info
> consistently everywhere (simplest).
> 2) Rename all usage of the struct thread_info to something else e.g. struct
> gdb_thread_info (not future-proof though).
> 3) Create a gdb namespace for all your code to avoid conflicts.
> 4) Segregate the use of system header files as you write above. Dunno how to do
> that though, but some of you should.

Normally code for native targets do 4).  I've had to be careful about which
includes I use in native FreeBSD targets to avoid namespace collisions, etc.
3) might not be a terrible idea eventually, but you'd have to make it explict
and avoid using 'using namespace gdb' for it to actually be useful.
  

On Mon, 2018-12-17 at 21:51 +0100, Svante Signell wrote:
> On Sun, 2018-12-16 at 16:10 -0700, Tom Tromey wrote:

> > Svante> Finally, I've found the problem (but no workaround yet):
> > thread_info is an RPC on GNU/Hurd, and including mach.h in
> > gdb/config/i386/nm-i386gnu.h:#include <mach.h> further includes
> > <mach/mach_interface.h> which has the conflicting name of that RPC:
> > kern_return_t thread_info
> > 
> > Typical answers for this kind of thing are either to segregate the
> > use of the system header somehow, or maybe namespacing or some
> > other kind of renaming. I haven't looked into the details much in
> > this case I'm afraid.
> 
> As I see it you need to:
> 
> 1) Apply the patches submitted earlier in this thread using struct
> thread_info
> consistently everywhere (simplest).
> 2) Rename all usage of the struct thread_info to something else e.g.
> struct gdb_thread_info (not future-proof though).
> 3) Create a gdb namespace for all your code to avoid conflicts.
> 4) Segregate the use of system header files as you write above. Dunno
> how to do that though, but some of you should.

Hi again. Seem like I did not get the reply from John Baldwin on this
thread, some gmail issue? Can somebody forward it to me, I've only seen
it on the mailing list archives: https://sourceware.org/ml/gdb-patches/
2018-12/msg00202.html

Seems like nothing is happening so far. Did you expect me to take
action on this issue? Or is this a low-prio problem for you, Hurd being
a second class citizen?
  

>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:

Svante> Seems like nothing is happening so far. Did you expect me to
Svante> take action on this issue? Or is this a low-prio problem for
Svante> you, Hurd being a second class citizen?

It's not so much second-class as that the usual way gdb development
works is that ports need maintainers, and the current contributors are
pretty busy and/or have other interests.

I took a quick look at it and one question I have is whether
nm-i386gnu.h really needs to include any of the headers it does.  defs.h
already includes unistd.h, and regcache.h should probably not be needed;
and removing the mach includes would at least reduce the number of spots
needing the "struct".  Could you try removing these lines?

Also I happened to notice that file declares gnu_target_pid_to_str --
but I didn't see a definition.  So perhaps that could be removed as
well.

thanks,
Tom
  

On 2018-12-20 17:34, Tom Tromey wrote:
>>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:
> 
> Svante> Seems like nothing is happening so far. Did you expect me to
> Svante> take action on this issue? Or is this a low-prio problem for
> Svante> you, Hurd being a second class citizen?
> 
> It's not so much second-class as that the usual way gdb development
> works is that ports need maintainers, and the current contributors are
> pretty busy and/or have other interests.

That's an important point.  For example, the Freebsd port is in pretty 
good shape, because somebody (thanks John) uses it daily and is able to 
report quickly any breakage to us non-Freebsd users.

I have tried earlier to give it a shot and build GDB using the image 
available here:

   https://www.gnu.org/software/hurd/hurd/running/qemu.html

but couldn't get very far (apt-get install wouldn't work).  Or at least, 
is there an easy way to get a cross-compiling environment for 
Debian/Hurd, let's say on Debian/Linux stable?

> I took a quick look at it and one question I have is whether
> nm-i386gnu.h really needs to include any of the headers it does.  
> defs.h
> already includes unistd.h, and regcache.h should probably not be 
> needed;
> and removing the mach includes would at least reduce the number of 
> spots
> needing the "struct".  Could you try removing these lines?
> 
> Also I happened to notice that file declares gnu_target_pid_to_str --
> but I didn't see a definition.  So perhaps that could be removed as
> well.

And to answer Svante's question about "what to do next", I think the 
intention long term was to put all of gdb in its own namespace, 
initially for fixing some gnulib clashes.  That's what triggered Pedro 
to do some C++ improvements (including wild matching).  For more 
details, see this, around the 15:00 mark:

https://slideslive.com/38902352/gdb-c-conversion-dogfooding-c

Looks like the prototype he did is on this branch:

https://github.com/palves/gdb/commits/palves/cxx-gdb-namespace

If this would fix the Hurd case, then maybe we can go with this.  Pedro, 
do you think we would be ready for such a move?  I would prefer to do 
that than to re-add a ton of "struct" keywords.

Simon
  

On 12/20/18 2:34 PM, Tom Tromey wrote:
>>>>>> "Svante" == Svante Signell <svante.signell@gmail.com> writes:
> 
> Svante> Seems like nothing is happening so far. Did you expect me to
> Svante> take action on this issue? Or is this a low-prio problem for
> Svante> you, Hurd being a second class citizen?
> 
> It's not so much second-class as that the usual way gdb development
> works is that ports need maintainers, and the current contributors are
> pretty busy and/or have other interests.
> 
> I took a quick look at it and one question I have is whether
> nm-i386gnu.h really needs to include any of the headers it does.  defs.h
> already includes unistd.h, and regcache.h should probably not be needed;
> and removing the mach includes would at least reduce the number of spots
> needing the "struct".  Could you try removing these lines?
> 
> Also I happened to notice that file declares gnu_target_pid_to_str --
> but I didn't see a definition.  So perhaps that could be removed as
> well.

I think this entire file can probably be removed.  The only useful content
it has that I can see are the THREAD_STATE_* macros, and those are only
used in one file: gnu-nat.c.  You could just move those there.  If in your
pending out-of-tree patches you need them in multiple files, you could
move the macros to gnu-nat.h.  If you have different versions (64-bit
vs 32-bit), then I think it would be fine to use #ifdef's to define the
appropriate version in whatever file you move the macros to.  That is
probably the simplest and quickest solution as moving everything into a
gdb namespace is probably a fairly large (and tedious) patch.