C++ conversion status update

C++ conversion status update

[Pedro Alves]

Hi all,

[Project page here: https://sourceware.org/gdb/wiki/cxx-conversion]

Good news, we've reached a significant milestone on the C++ conversion -- on
a few important hosts, GDB now builds cleanly as a C++ program with no
hacks.  These are at least:

 - Aarch64 GNU/Linux
 - ARM GNU/Linux
 - x86 GNU/Linux
 - x86_64 GNU/Linux
 - x86_64 Mingw-w64

In addition, the testsuite shows no regressions in C++ mode, compared to C mode.

Up until recently, we were still using g++'s -fpermissive switch as a shortcut.
That allowed deferring fixing a huge number boring cast and enum conversion issues.
Most of those issues are now fixed, and since the hosts above cover most of
the common, non-host-specific code, we changed the build system to no longer
use g++'s -fpermissive switch, and at the same time, made C++ mode default to
build with -Werror too, just like C mode.  Big thanks goes to Simon Marchi,
Yao Qi and others for all the help.

There's still a lot of work ahead though.  Until all supported hosts can build
with a C++ compiler, we'll have to keep supporting building GDB as a C program
as well.  It's not sufficient to say "builds cleanly on GNU/Linux", because
each architecture+OS combination (ARM GNU/Linux, x86 Windows, etc.) has it's
own set of native debugger support files (the *-nat.c files) that are not built
on any other host.

The plan I propose next is to default to building in C++ mode on hosts
that are known to build cleanly in C++ mode.  Or rather, change the default
to be C++ mode, _except_ for hosts/ports that haven't been converted yet.
Then, as ports get converted, one by one they'll be removed from
the "can't do C++ yet" list.  That gives us a defined stopping point -- when
the list becomes empty, and after a reasonable period, we can decide to remove
support for building in C mode and start making use of C++ features.

The question I have is which hosts (Architecture + OS combination) people
care about that still need C++ conversion work and thus should be on
that list?

This is where anyone can help!

I've created the "users/palves/cxx-conversion" branch (on sourceware.org)
where I committed a patch that implements the list idea.  I filled in
a set of hosts that I _thought_ wouldn't build, but I don't really know
for sure.

So I'd like to invite people to try to build master on their favorite
host with --enable-build-with-cxx, and report back the result.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> Good news, we've reached a significant milestone on the C++ conversion -- on
> a few important hosts, GDB now builds cleanly as a C++ program with no
> hacks.  These are at least:
>
>  - Aarch64 GNU/Linux
>  - ARM GNU/Linux
>  - x86 GNU/Linux
>  - x86_64 GNU/Linux
>  - x86_64 Mingw-w64
>
> In addition, the testsuite shows no regressions in C++ mode, compared to C mode.
>

We need a wiki page to track these hosts on which we enabled C++ build.
Like this gcc wiki page, https://gcc.gnu.org/wiki/CppBuildStatus or we
can track them in existing https://sourceware.org/gdb/wiki/cxx-conversion
wiki page.

> The plan I propose next is to default to building in C++ mode on hosts
> that are known to build cleanly in C++ mode.  Or rather, change the default
> to be C++ mode, _except_ for hosts/ports that haven't been converted
> yet.

I prefer the latter.

> Then, as ports get converted, one by one they'll be removed from
> the "can't do C++ yet" list.  That gives us a defined stopping point -- when
> the list becomes empty, and after a reasonable period, we can decide to remove
> support for building in C mode and start making use of C++ features.

That sounds good to me.

>
> The question I have is which hosts (Architecture + OS combination) people
> care about that still need C++ conversion work and thus should be on
> that list?

How about all linux and mingw hosts? assuming that xtensa can be
converted.

>
> This is where anyone can help!
>
> I've created the "users/palves/cxx-conversion" branch (on sourceware.org)
> where I committed a patch that implements the list idea.  I filled in
> a set of hosts that I _thought_ wouldn't build, but I don't really know
> for sure.
>
> So I'd like to invite people to try to build master on their favorite
> host with --enable-build-with-cxx, and report back the result.

We didn't hear anything on this yet, but we can't wait for it forever.
I'd like to turn on C++ build for linux and mingw hosts in default after
the 7.11 release branch is created.

-- 
Yao (齐尧)

Re: C++ conversion status update

[Pedro Alves]

On 12/14/2015 02:40 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:


> We need a wiki page to track these hosts on which we enabled C++ build.
> Like this gcc wiki page, https://gcc.gnu.org/wiki/CppBuildStatus or we
> can track them in existing https://sourceware.org/gdb/wiki/cxx-conversion
> wiki page.

Ok.

> 
>> The plan I propose next is to default to building in C++ mode on hosts
>> that are known to build cleanly in C++ mode.  Or rather, change the default
>> to be C++ mode, _except_ for hosts/ports that haven't been converted
>> yet.
> 
> I prefer the latter.
> 
>> Then, as ports get converted, one by one they'll be removed from
>> the "can't do C++ yet" list.  That gives us a defined stopping point -- when
>> the list becomes empty, and after a reasonable period, we can decide to remove
>> support for building in C mode and start making use of C++ features.
> 
> That sounds good to me.

Ok, that's my preference as well.

> 
>>
>> The question I have is which hosts (Architecture + OS combination) people
>> care about that still need C++ conversion work and thus should be on
>> that list?
> 
> How about all linux and mingw hosts? assuming that xtensa can be
> converted.

x86-64 and x86 mingw built cleanly last I tried.

I don't know about _all_ Linux hosts, hence the RFH.

At least x86, x86_64, Aarch64, ARM, and PPC64 build cleanly.  Or did, a
few weeks back.

>>
>> This is where anyone can help!
>>
>> I've created the "users/palves/cxx-conversion" branch (on sourceware.org)
>> where I committed a patch that implements the list idea.  I filled in
>> a set of hosts that I _thought_ wouldn't build, but I don't really know
>> for sure.
>>
>> So I'd like to invite people to try to build master on their favorite
>> host with --enable-build-with-cxx, and report back the result.
> 
> We didn't hear anything on this yet, but we can't wait for it forever.

Yeah.  I got enthusiastic support offlist for C++ in general, but people
don't seem to really be that interested or paying attention to random
host architectures.

Meanwhile, I tried a few BSD hosts on the compile farm and
wrote a few patches:

  https://github.com/palves/gdb/commits/palves/cxx-conversion-netbsd

but haven't had a moment yet to drive them to completion.  I probably should sync
up with Simon though, as I think I may be duplicating work he's done already.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Jose E. Marchesi]

    >> The question I have is which hosts (Architecture + OS combination) people
    >> care about that still need C++ conversion work and thus should be on
    >> that list?
    > 
    > How about all linux and mingw hosts? assuming that xtensa can be
    > converted.
    
    x86-64 and x86 mingw built cleanly last I tried.
    
    I don't know about _all_ Linux hosts, hence the RFH.
    
    At least x86, x86_64, Aarch64, ARM, and PPC64 build cleanly.  Or did, a
    few weeks back.

In sparc-*-linux-gnu:
- Builds fine with GCC 6 (svn).
- Builds fine with GCC 4.9.1.
- Build fails with GCC 4.4.7:

g++ -g -O2   -I. -I../../gdb -I../../gdb/common -I../../gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../opcodes/.. -I../../gdb/../readline/.. -I../../gdb/../zlib -I../bfd -I../../gdb/../bfd -I../../gdb/../include -I../libdecnumber -I../../gdb/../libdecnumber  -I../../gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.6 -I/usr/include/python2.6 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wno-sign-compare -Wno-write-strings -Wno-narrowing -Wformat-nonliteral -Werror -c -o sparc64-tdep.o -MT sparc64-tdep.o -MMD -MP -MF .deps/sparc64-tdep.Tpo ../../gdb/sparc64-tdep.c
cc1plus: warnings being treated as errors
In file included from ../../gdb/target.h:74,
                 from ../../gdb/exec.h:23,
                 from ../../gdb/gdbcore.h:29,
                 from ../../gdb/sparc64-tdep.c:27:
../../gdb/btrace.h: In function ‘size_t VEC_btrace_insn_s_embedded_size(int)’:
../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_alloc(int)’:
../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_copy(VEC_btrace_insn_s*)’:
../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_merge(VEC_btrace_insn_s*, VEC_btrace_insn_s*)’:
../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
../../gdb/btrace.h: In function ‘int VEC_btrace_insn_s_reserve(VEC_btrace_insn_s**, int, const char*, unsigned int)’:
../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
At global scope:
cc1plus: error: unrecognized command line option "-Wno-narrowing"
make[2]: *** [sparc64-tdep.o] Error 1
make[2]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build/gdb'
make[1]: *** [all-gdb] Error 2
make[1]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build'
make: *** [all] Error 2

Re: C++ conversion status update

[Simon Marchi]

On 15 December 2015 at 06:46, Jose E. Marchesi <jose.marchesi@oracle.com> wrote:
> g++ -g -O2   -I. -I../../gdb -I../../gdb/common -I../../gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../opcodes/.. -I../../gdb/../readline/.. -I../../gdb/../zlib -I../bfd -I../../gdb/../bfd -I../../gdb/../include -I../libdecnumber -I../../gdb/../libdecnumber  -I../../gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.6 -I/usr/include/python2.6 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wno-sign-compare -Wno-write-strings -Wno-narrowing -Wformat-nonliteral -Werror -c -o sparc64-tdep.o -MT sparc64-tdep.o -MMD -MP -MF .deps/sparc64-tdep.Tpo ../../gdb/sparc64-tdep.c
> cc1plus: warnings being treated as errors
> In file included from ../../gdb/target.h:74,
>                  from ../../gdb/exec.h:23,
>                  from ../../gdb/gdbcore.h:29,
>                  from ../../gdb/sparc64-tdep.c:27:
> ../../gdb/btrace.h: In function ‘size_t VEC_btrace_insn_s_embedded_size(int)’:
> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_alloc(int)’:
> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_copy(VEC_btrace_insn_s*)’:
> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_merge(VEC_btrace_insn_s*, VEC_btrace_insn_s*)’:
> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
> ../../gdb/btrace.h: In function ‘int VEC_btrace_insn_s_reserve(VEC_btrace_insn_s**, int, const char*, unsigned int)’:
> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
> At global scope:
> cc1plus: error: unrecognized command line option "-Wno-narrowing"
> make[2]: *** [sparc64-tdep.o] Error 1
> make[2]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build/gdb'
> make[1]: *** [all-gdb] Error 2
> make[1]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build'
> make: *** [all] Error 2

This can be reproduced with g++ 4.4 on any architecture, i think.

This seems to be caused by the interaction of enum_flags and the
vector implementation.  After preprocessing we have this:

  __builtin_offsetof (VEC_btrace_insn_s, vec)

However, VEC_btrace_insn_s is a non-POD data type, because it
ultimately contains an enum_flags object, which is non-POD.

  VEC_btrace_insn_s
    btrace_insn_s vec[1];
      btrace_insn_flags flags; (enum_flags <enum btrace_insn_flag>)

From what I read, using offsetof on a non-POD structure is an
undefined behavior, at least in C++98.  I don't know if it has changed
in later standard versions, but later g++ do not complain.

For g++ 4.4, it is possible to silence the warning/error with
-Wno-invalid-offsetof, if we know that it still produces good results.

Re: C++ conversion status update

[Pedro Alves]

On 12/15/2015 08:02 PM, Simon Marchi wrote:
> On 15 December 2015 at 06:46, Jose E. Marchesi <jose.marchesi@oracle.com> wrote:
>> g++ -g -O2   -I. -I../../gdb -I../../gdb/common -I../../gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../gdb/../include/opcode -I../../gdb/../opcodes/.. -I../../gdb/../readline/.. -I../../gdb/../zlib -I../bfd -I../../gdb/../bfd -I../../gdb/../include -I../libdecnumber -I../../gdb/../libdecnumber  -I../../gdb/gnulib/import -Ibuild-gnulib/import   -DTUI=1  -I/usr/include/python2.6 -I/usr/include/python2.6 -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wno-sign-compare -Wno-write-strings -Wno-narrowing -Wformat-nonliteral -Werror -c -o sparc64-tdep.o -MT sparc64-tdep.o -MMD -MP -MF .deps/sparc64-tdep.Tpo ../../gdb/sparc64-tdep.c
>> cc1plus: warnings being treated as errors
>> In file included from ../../gdb/target.h:74,
>>                  from ../../gdb/exec.h:23,
>>                  from ../../gdb/gdbcore.h:29,
>>                  from ../../gdb/sparc64-tdep.c:27:
>> ../../gdb/btrace.h: In function ‘size_t VEC_btrace_insn_s_embedded_size(int)’:
>> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
>> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
>> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_alloc(int)’:
>> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
>> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
>> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_copy(VEC_btrace_insn_s*)’:
>> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
>> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
>> ../../gdb/btrace.h: In function ‘VEC_btrace_insn_s* VEC_btrace_insn_s_merge(VEC_btrace_insn_s*, VEC_btrace_insn_s*)’:
>> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
>> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
>> ../../gdb/btrace.h: In function ‘int VEC_btrace_insn_s_reserve(VEC_btrace_insn_s**, int, const char*, unsigned int)’:
>> ../../gdb/btrace.h:84: error: invalid access to non-static data member ‘VEC_btrace_insn_s::vec’  of NULL object
>> ../../gdb/btrace.h:84: error: (perhaps the ‘offsetof’ macro was used incorrectly)
>> At global scope:
>> cc1plus: error: unrecognized command line option "-Wno-narrowing"
>> make[2]: *** [sparc64-tdep.o] Error 1
>> make[2]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build/gdb'
>> make[1]: *** [all-gdb] Error 2
>> make[1]: Leaving directory `/home/jemarch/couts2/binutils-gdb/build'
>> make: *** [all] Error 2
> 
> This can be reproduced with g++ 4.4 on any architecture, i think.
> 
> This seems to be caused by the interaction of enum_flags and the
> vector implementation.  After preprocessing we have this:
> 
>   __builtin_offsetof (VEC_btrace_insn_s, vec)
> 
> However, VEC_btrace_insn_s is a non-POD data type, because it
> ultimately contains an enum_flags object, which is non-POD.
> 
>   VEC_btrace_insn_s
>     btrace_insn_s vec[1];
>       btrace_insn_flags flags; (enum_flags <enum btrace_insn_flag>)
> 
> From what I read, using offsetof on a non-POD structure is an
> undefined behavior, at least in C++98.  I don't know if it has changed
> in later standard versions, but later g++ do not complain.

Yeah, C++11 relaxed the rules a lot.  enum_flags is designed to be
POD-like, though it has user-defined constructors.  It's quite safe
to memcpy it.

> 
> For g++ 4.4, it is possible to silence the warning/error with
> -Wno-invalid-offsetof, if we know that it still produces good results.
> 

Yeah, it should work.  The real fix of course would be to
reimplement vec.h using real C++ templates (until we can get rid of
C mode, that is).

An alternative is to avoid offsetof in the code itself, like in the
patch below.  It's really offsetof-but-avoid-NULL.

An advantage over disabling the warning is that it's contained in
the VEC code, which lets G++ catch issues elsewhere.  OTOH, disabling
warning is simpler.  WDYT?

From c0803a1aca62cde14c47a103aa996f1e1f4a2875 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 15 Dec 2015 22:43:06 +0000
Subject: [PATCH] no offsetof

---
 gdb/common/vec.h | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/gdb/common/vec.h b/gdb/common/vec.h
index 6189283..a188b02 100644
--- a/gdb/common/vec.h
+++ b/gdb/common/vec.h
@@ -437,13 +437,23 @@ DEF_VEC_FUNC_O(T)							  \
 DEF_VEC_ALLOC_FUNC_O(T)							  \
 struct vec_swallow_trailing_semi
 
+/* Avoid offsetof (or its usual C implementation) as it triggers
+   -Winvalid-offsetof warnings with enum_flags types with G++ <= 4.4,
+   even though those types are memcpyable.  This requires allocating a
+   dummy local VEC in all routines that use this, but that has the
+   advantage that it only works if T is default constructible, which
+   is exactly a check we want, to keep C compatibility.  */
+#define vec_offset(T, VPTR) ((size_t) ((char *) &(VPTR)->vec - (char *) VPTR))
+
 #define DEF_VEC_ALLOC_FUNC_I(T)						  \
 static inline VEC(T) *VEC_OP (T,alloc)					  \
      (int alloc_)							  \
 {									  \
+  VEC(T) dummy;							  	  \
+									  \
   /* We must request exact size allocation, hence the negation.  */	  \
   return (VEC(T) *) vec_o_reserve (NULL, -alloc_,			  \
-                                   offsetof (VEC(T),vec), sizeof (T));	  \
+                                   vec_offset (T, &dummy), sizeof (T));	  \
 }									  \
 									  \
 static inline VEC(T) *VEC_OP (T,copy) (VEC(T) *vec_)			  \
@@ -453,9 +463,11 @@ static inline VEC(T) *VEC_OP (T,copy) (VEC(T) *vec_)			  \
 									  \
   if (len_)								  \
     {									  \
+      VEC(T) dummy;							  \
+									  \
       /* We must request exact size allocation, hence the negation.  */	  \
       new_vec_ = (VEC (T) *)						  \
-	vec_o_reserve (NULL, -len_, offsetof (VEC(T),vec), sizeof (T));	  \
+	vec_o_reserve (NULL, -len_, vec_offset (T, &dummy), sizeof (T));	\
 									  \
       new_vec_->num = len_;						  \
       memcpy (new_vec_->vec, vec_->vec, sizeof (T) * len_);		  \
@@ -467,12 +479,13 @@ static inline VEC(T) *VEC_OP (T,merge) (VEC(T) *vec1_, VEC(T) *vec2_)	  \
 {									  \
   if (vec1_ && vec2_)							  \
     {									  \
+      VEC(T) dummy;							  \
       size_t len_ = vec1_->num + vec2_->num;				  \
       VEC (T) *new_vec_ = NULL;						  \
 									  \
       /* We must request exact size allocation, hence the negation.  */	  \
       new_vec_ = (VEC (T) *)						  \
-	vec_o_reserve (NULL, -len_, offsetof (VEC(T),vec), sizeof (T));	  \
+	vec_o_reserve (NULL, -len_, vec_offset (T, &dummy), sizeof (T));	  \
 									  \
       new_vec_->num = len_;						  \
       memcpy (new_vec_->vec, vec1_->vec, sizeof (T) * vec1_->num);	  \
@@ -505,12 +518,13 @@ static inline void VEC_OP (T,cleanup)					  \
 static inline int VEC_OP (T,reserve)					  \
      (VEC(T) **vec_, int alloc_ VEC_ASSERT_DECL)			  \
 {									  \
+  VEC(T) dummy;								  \
   int extend = !VEC_OP (T,space)					  \
 	(*vec_, alloc_ < 0 ? -alloc_ : alloc_ VEC_ASSERT_PASS);		  \
 									  \
   if (extend)								  \
     *vec_ = (VEC(T) *) vec_o_reserve (*vec_, alloc_,			  \
-				      offsetof (VEC(T),vec), sizeof (T)); \
+				      vec_offset (T, &dummy), sizeof (T)); \
 									  \
   return extend;							  \
 }									  \
@@ -581,7 +595,9 @@ static inline int VEC_OP (T,iterate)					  \
 static inline size_t VEC_OP (T,embedded_size)				  \
      (int alloc_)							  \
 {									  \
-  return offsetof (VEC(T),vec) + alloc_ * sizeof(T);			  \
+  VEC(T) dummy;								  \
+									  \
+  return vec_offset (T, &dummy) + alloc_ * sizeof(T);			  \
 }									  \
 									  \
 static inline void VEC_OP (T,embedded_init)				  \
@@ -863,7 +879,9 @@ static inline int VEC_OP (T,iterate)					  \
 static inline size_t VEC_OP (T,embedded_size)				  \
      (int alloc_)							  \
 {									  \
-  return offsetof (VEC(T),vec) + alloc_ * sizeof(T);			  \
+  VEC(T) dummy;								  \
+									  \
+  return vec_offset (T, &dummy) + alloc_ * sizeof(T);			  \
 }									  \
 									  \
 static inline void VEC_OP (T,embedded_init)				  \
@@ -998,9 +1016,11 @@ static inline unsigned VEC_OP (T,lower_bound)				  \
 static inline VEC(T) *VEC_OP (T,alloc)					  \
      (int alloc_)							  \
 {									  \
+  VEC(T) dummy;								  \
+									  \
   /* We must request exact size allocation, hence the negation.  */	  \
   return (VEC(T) *) vec_o_reserve (NULL, -alloc_,			  \
-                                   offsetof (VEC(T),vec), sizeof (T));	  \
+                                   vec_offset (T, &dummy), sizeof (T));	  \
 }									  \
 									  \
 static inline VEC(T) *VEC_OP (T,copy) (VEC(T) *vec_)			  \
@@ -1010,9 +1030,11 @@ static inline VEC(T) *VEC_OP (T,copy) (VEC(T) *vec_)			  \
 									  \
   if (len_)								  \
     {									  \
+      VEC(T) dummy;							  \
+									  \
       /* We must request exact size allocation, hence the negation.  */	  \
       new_vec_ = (VEC (T) *)						  \
-	vec_o_reserve  (NULL, -len_, offsetof (VEC(T),vec), sizeof (T));  \
+	vec_o_reserve  (NULL, -len_, vec_offset (T, &dummy), sizeof (T)); \
 									  \
       new_vec_->num = len_;						  \
       memcpy (new_vec_->vec, vec_->vec, sizeof (T) * len_);		  \
@@ -1024,12 +1046,13 @@ static inline VEC(T) *VEC_OP (T,merge) (VEC(T) *vec1_, VEC(T) *vec2_)	  \
 {									  \
   if (vec1_ && vec2_)							  \
     {									  \
+      VEC(T) dummy;							  \
       size_t len_ = vec1_->num + vec2_->num;				  \
       VEC (T) *new_vec_ = NULL;						  \
 									  \
       /* We must request exact size allocation, hence the negation.  */	  \
       new_vec_ = (VEC (T) *)						  \
-	vec_o_reserve (NULL, -len_, offsetof (VEC(T),vec), sizeof (T));	  \
+	vec_o_reserve (NULL, -len_, vec_offset (T, &dummy), sizeof (T));  \
 									  \
       new_vec_->num = len_;						  \
       memcpy (new_vec_->vec, vec1_->vec, sizeof (T) * vec1_->num);	  \
@@ -1062,12 +1085,13 @@ static inline void VEC_OP (T,cleanup)					  \
 static inline int VEC_OP (T,reserve)					  \
      (VEC(T) **vec_, int alloc_ VEC_ASSERT_DECL)			  \
 {									  \
+  VEC(T) dummy;								  \
   int extend = !VEC_OP (T,space) (*vec_, alloc_ < 0 ? -alloc_ : alloc_	  \
 				  VEC_ASSERT_PASS);			  \
 									  \
   if (extend)								  \
     *vec_ = (VEC(T) *)							  \
-	vec_o_reserve (*vec_, alloc_, offsetof (VEC(T),vec), sizeof (T)); \
+      vec_o_reserve (*vec_, alloc_, vec_offset (T, &dummy), sizeof (T));  \
 									  \
   return extend;							  \
 }									  \
-- 
1.9.3

Re: C++ conversion status update

[Pedro Alves]

Forgot to mention that this compiles down to the same as offsetof at -O2.

On 12/16/2015 12:19 AM, Pedro Alves wrote:

> diff --git a/gdb/common/vec.h b/gdb/common/vec.h
> index 6189283..a188b02 100644
> --- a/gdb/common/vec.h
> +++ b/gdb/common/vec.h
> @@ -437,13 +437,23 @@ DEF_VEC_FUNC_O(T)							  \
>  DEF_VEC_ALLOC_FUNC_O(T)							  \
>  struct vec_swallow_trailing_semi
>  
> +/* Avoid offsetof (or its usual C implementation) as it triggers
> +   -Winvalid-offsetof warnings with enum_flags types with G++ <= 4.4,
> +   even though those types are memcpyable.  This requires allocating a
> +   dummy local VEC in all routines that use this, but that has the
> +   advantage that it only works if T is default constructible, which
> +   is exactly a check we want, to keep C compatibility.  */
> +#define vec_offset(T, VPTR) ((size_t) ((char *) &(VPTR)->vec - (char *) VPTR))

Thanks,
Pedro Alves

Re: C++ conversion status update

[Simon Marchi]

On 15 December 2015 at 19:21, Pedro Alves <palves@redhat.com> wrote:
> Forgot to mention that this compiles down to the same as offsetof at -O2.

Then your solution sounds better on every aspect.

Re: C++ conversion status update

[Pedro Alves]

On 12/16/2015 01:18 AM, Simon Marchi wrote:
> On 15 December 2015 at 19:21, Pedro Alves <palves@redhat.com> wrote:
>> Forgot to mention that this compiles down to the same as offsetof at -O2.
> 
> Then your solution sounds better on every aspect.

Alright, now pushed:

 https://sourceware.org/ml/gdb-patches/2015-12/msg00308.html

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 12/15/2015 11:46 AM, Jose E. Marchesi wrote:

> At global scope:
> cc1plus: error: unrecognized command line option "-Wno-narrowing"

Simon, do you see this one?

Thanks,
Pedro Alves

Re: C++ conversion status update

[Simon Marchi]

On 16 December 2015 at 15:15, Pedro Alves <palves@redhat.com> wrote:
> Simon, do you see this one?

Yes, but it doesn't seem to make the compilation fail on its own
(despite stating it's an error).  I only see it when the build fails
because of another error.  I figured that that flag was introduced gcc
version, and that it doesn't hurt if it's not recognized.

Re: C++ conversion status update

[Pedro Alves]

On 12/16/2015 08:29 PM, Simon Marchi wrote:
> On 16 December 2015 at 15:15, Pedro Alves <palves@redhat.com> wrote:
>> Simon, do you see this one?
> 
> Yes, but it doesn't seem to make the compilation fail on its own
> (despite stating it's an error).  I only see it when the build fails
> because of another error.  I figured that that flag was introduced gcc
> version, and that it doesn't hurt if it's not recognized.

Ah, yes, that's it.  We have code in configure.ac that tries to
detect whether the compiler supports each warning and suppress it
it no, but it's getting fooled by:

"When an unrecognized warning option is requested (e.g., -Wunknown-warning), GCC emits a
diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the
behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other
diagnostics are being produced. This allows the use of new -Wno- options with old
compilers, but if something goes wrong, the compiler warns that an unrecognized
option is present."

That's from https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html.

I think we can handle this by making that warning-support test code
check whether -Wfoo works when we actually want -Wno-foo.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 12/16/2015 10:10 PM, Pedro Alves wrote:

> "When an unrecognized warning option is requested (e.g., -Wunknown-warning), GCC emits a
> diagnostic stating that the option is not recognized. However, if the -Wno- form is used, the
> behavior is slightly different: no diagnostic is produced for -Wno-unknown-warning unless other
> diagnostics are being produced. This allows the use of new -Wno- options with old
> compilers, but if something goes wrong, the compiler warns that an unrecognized
> option is present."
> 
> I think we can handle this by making that warning-support test code
> check whether -Wfoo works when we actually want -Wno-foo.

Proposed fix posted at:

 https://sourceware.org/ml/gdb-patches/2015-12/msg00312.html

Thanks,
Pedro Alves

Re: C++ conversion status update

[John Baldwin]

On Monday, December 14, 2015 07:09:08 PM Pedro Alves wrote:
> On 12/14/2015 02:40 PM, Yao Qi wrote:
> Meanwhile, I tried a few BSD hosts on the compile farm and
> wrote a few patches:
> 
>   https://github.com/palves/gdb/commits/palves/cxx-conversion-netbsd
> 
> but haven't had a moment yet to drive them to completion.  I probably should sync
> up with Simon though, as I think I may be duplicating work he's done already.

I started doing some test builds on FreeBSD with C++ today and ran into one
unexpected thing that I'm not sure how to fix.

I build gdb from git on my own for testing, but the gdb package for FreeBSD
users is built from FreeBSD's ports framework.  The framework sets default
CFLAGS / CXXFLAGS values for the ports it builds (and each port can
override or adjust those as needed).  However, gdb uses CFLAGS instead of
CXXFLAGS when building with C++.  The default flags in FreeBSD's ports
infrastructure include "-std=gnu89" in the default CFLAGS (but not in
CXXFLAGS since it is a C-only flag).  The CXXFLAGS set in the environment
when ./configure was run do not appear to be honored in the generated
gdb/Makefile and also don't appear to be used during the build of gdb
itself.

-- 
John Baldwin

Re: C++ conversion status update

[Pedro Alves]

On 01/19/2016 07:00 PM, John Baldwin wrote:
> On Monday, December 14, 2015 07:09:08 PM Pedro Alves wrote:
>> On 12/14/2015 02:40 PM, Yao Qi wrote:
>> Meanwhile, I tried a few BSD hosts on the compile farm and
>> wrote a few patches:
>>
>>   https://github.com/palves/gdb/commits/palves/cxx-conversion-netbsd
>>
>> but haven't had a moment yet to drive them to completion.  I probably should sync
>> up with Simon though, as I think I may be duplicating work he's done already.
> 
> I started doing some test builds on FreeBSD with C++ today and ran into one
> unexpected thing that I'm not sure how to fix.

Thanks!

> 
> I build gdb from git on my own for testing, but the gdb package for FreeBSD
> users is built from FreeBSD's ports framework.  The framework sets default
> CFLAGS / CXXFLAGS values for the ports it builds (and each port can
> override or adjust those as needed).  However, gdb uses CFLAGS instead of
> CXXFLAGS when building with C++.  The default flags in FreeBSD's ports
> infrastructure include "-std=gnu89" in the default CFLAGS (but not in
> CXXFLAGS since it is a C-only flag).  The CXXFLAGS set in the environment
> when ./configure was run do not appear to be honored in the generated
> gdb/Makefile and also don't appear to be used during the build of gdb
> itself.

Indeed, thanks for noticing this.  I've sent a patch now:
  https://sourceware.org/ml/gdb-patches/2016-01/msg00469.html

I've also pushed it to:
  git@github.com:palves/gdb.git palves/cxx-conversion-attempt-part-2-no-fpermissive

(note there are a few more wip/pending C++ patches on that branch)

Could you give it a try?

Thanks,
Pedro Alves

Re: C++ conversion status update

[John Baldwin]

On Wednesday, January 20, 2016 11:10:21 AM Pedro Alves wrote:
> Indeed, thanks for noticing this.  I've sent a patch now:
>   https://sourceware.org/ml/gdb-patches/2016-01/msg00469.html
> 
> I've also pushed it to:
>   git@github.com:palves/gdb.git palves/cxx-conversion-attempt-part-2-no-fpermissive
> 
> (note there are a few more wip/pending C++ patches on that branch)
> 
> Could you give it a try?

Yes, that works well.

I see that you also have a patch (41bed896a) in that branch to fix ptrace()
args detection (which also broke for me, though I just hacked config.h locally
so I could work on the actual problems).

Unfortunately, your patch doesn't fix FreeBSD/amd64 with GCC 4.8 (at least).
The detection of the ptrace return type ('int' on FreeBSD) fails and falls
back to long and I think that prevents it from subsequently matching on the
correct types for the argument list.

The error for the failing test for the return type (which should work instead
I think) is:

configure:12453: /usr/local/bin/g++48 -c -pipe -DRL_NO_COMPAT -Wno-unused-function -Wno-unused-variable  -g -DLIBICONV_PLUG -g -fno-strict-aliasing  -DLIBICONV_PLUG  conftest.cpp >&5
conftest.cpp:166:22: error: declaration of C function 'int ptrace()' conflicts with
 EXTERN_C int ptrace ();
                      ^
In file included from conftest.cpp:154:0:
/usr/include/sys/ptrace.h:185:5: error: previous declaration 'int ptrace(int, pid_t, caddr_t, int)' here
 int ptrace(int _request, pid_t _pid, caddr_t _addr, int _data);
     ^
configure:12453: $? = 1
configure: failed program was:
....
| EXTERN_C int ptrace ();
| 
| int
| main ()
| {
| 
|   ;
|   return 0;
| }
configure:12462: result: long
configure:12470: checking types of arguments for ptrace

I think the problem is that whereas in C "int foo()" means the args to foo()
are undefined, "int foo()" in C++ is equivalent to "int foo(void)".

I know that a comment in ptrace.ac4 mentions that C++ needs to be used to
determine if the first argument to ptrace is an enum that needs a cast,
but I wonder if we couldn't fall back to plain C for the return type
check?  (I'm not sure which of the tests in ptrace.ac4 needs C++ to make
the enum determination such that we could possibly shuffle things around
to do the one test needed while in C++ mode and the rest in C?)

Another possibility would be to move the return type check into the big
loop that currently checks the arguments as an outermost loop, something
like:

for gdb_ret in 'int' 'long'; do
  for gdb_arg1 in 'int' 'long'; do
    ...

This would work in C++ mode I think.

-- 
John Baldwin

Re: C++ conversion status update

[Pedro Alves]

On 01/20/2016 11:33 PM, John Baldwin wrote:
> On Wednesday, January 20, 2016 11:10:21 AM Pedro Alves wrote:
>> Indeed, thanks for noticing this.  I've sent a patch now:
>>   https://sourceware.org/ml/gdb-patches/2016-01/msg00469.html
>>

>> Could you give it a try?
> 
> Yes, that works well.

Great, this bit is now in.

(I'll get back to the ptrace issue separately.)

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 12/14/2015 07:09 PM, Pedro Alves wrote:
> On 12/14/2015 02:40 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
> 
> 
>> We need a wiki page to track these hosts on which we enabled C++ build.
>> Like this gcc wiki page, https://gcc.gnu.org/wiki/CppBuildStatus or we
>> can track them in existing https://sourceware.org/gdb/wiki/cxx-conversion
>> wiki page.
> 
> Ok.
> 
>>
>>> The plan I propose next is to default to building in C++ mode on hosts
>>> that are known to build cleanly in C++ mode.  Or rather, change the default
>>> to be C++ mode, _except_ for hosts/ports that haven't been converted
>>> yet.
>>
>> I prefer the latter.
>>
>>> Then, as ports get converted, one by one they'll be removed from
>>> the "can't do C++ yet" list.  That gives us a defined stopping point -- when
>>> the list becomes empty, and after a reasonable period, we can decide to remove
>>> support for building in C mode and start making use of C++ features.
>>
>> That sounds good to me.
> 
> Ok, that's my preference as well.
> 
>>
>>>
>>> The question I have is which hosts (Architecture + OS combination) people
>>> care about that still need C++ conversion work and thus should be on
>>> that list?
>>
>> How about all linux and mingw hosts? assuming that xtensa can be
>> converted.
> 
> x86-64 and x86 mingw built cleanly last I tried.
> 
> I don't know about _all_ Linux hosts, hence the RFH.
> 
> At least x86, x86_64, Aarch64, ARM, and PPC64 build cleanly.  Or did, a
> few weeks back.
> 
>>>
>>> This is where anyone can help!
>>>
>>> I've created the "users/palves/cxx-conversion" branch (on sourceware.org)
>>> where I committed a patch that implements the list idea.  I filled in
>>> a set of hosts that I _thought_ wouldn't build, but I don't really know
>>> for sure.
>>>
>>> So I'd like to invite people to try to build master on their favorite
>>> host with --enable-build-with-cxx, and report back the result.
>>
>> We didn't hear anything on this yet, but we can't wait for it forever.
> 
> Yeah.  I got enthusiastic support offlist for C++ in general, but people
> don't seem to really be that interested or paying attention to random
> host architectures.
> 

Now that the "exceptions signals handlers" issue is resolved, I think we've
now done all we could re. C++ conversion while keeping C supported as well.

I think the time has come to starting to default to building in C++ mode
on all hosts, except those known to not having been fully converted yet,
as with the patch at the top of the users/palves/cxx-conversion branch.

Thoughts?

Thanks,
Pedro Alves

Re: C++ conversion status update

[Yao Qi]

Pedro Alves <palves@redhat.com> writes:

> I think the time has come to starting to default to building in C++ mode
> on all hosts, except those known to not having been fully converted yet,
> as with the patch at the top of the users/palves/cxx-conversion branch.
>
> Thoughts?

I read that patch on your tree, and that is quite clean and small.  I am
fine on that.

We need to update buildbot builder Fedora-x86_64-cxx-build-m64 to build
gdb in C mode.

-- 
Yao (齐尧)

Re: C++ conversion status update

[Joel Brobecker]

> Now that the "exceptions signals handlers" issue is resolved, I think we've
> now done all we could re. C++ conversion while keeping C supported as well.
> 
> I think the time has come to starting to default to building in C++ mode
> on all hosts, except those known to not having been fully converted yet,
> as with the patch at the top of the users/palves/cxx-conversion branch.
> 
> Thoughts?

I wish I could say I'm ready for the switch, but I'm not. Mostly,
it's cross-compiling gdbserver, because I need to start building
a C++ cross-compiler to be able to achieve that. But I don't think
I can use that excuse to hold you guys back forever. If others
are in the same situation, then we might have to reconsider our
timeline, but so far as I know, I'm the only one. So, I'm OK with
changing the default as well.

If we do, perhaps it's time to change the major version number
to 8.0?

-- 
Joel

Re: C++ conversion status update

[Pedro Alves]

On 04/13/2016 01:41 PM, Joel Brobecker wrote:
>> Now that the "exceptions signals handlers" issue is resolved, I think we've
>> now done all we could re. C++ conversion while keeping C supported as well.
>>
>> I think the time has come to starting to default to building in C++ mode
>> on all hosts, except those known to not having been fully converted yet,
>> as with the patch at the top of the users/palves/cxx-conversion branch.
>>
>> Thoughts?
> 
> I wish I could say I'm ready for the switch, but I'm not. Mostly,
> it's cross-compiling gdbserver, because I need to start building
> a C++ cross-compiler to be able to achieve that. But I don't think
> I can use that excuse to hold you guys back forever. If others
> are in the same situation, then we might have to reconsider our
> timeline, but so far as I know, I'm the only one. So, I'm OK with
> changing the default as well.

OK.  We can just add an entry to lynxos to GDB_AC_BUILD_WITH_CXX
to make it default to C, still.  Like this, against
the version in the users/palves/cxx-conversion branch:

diff --git i/gdb/build-with-cxx.m4 w/gdb/build-with-cxx.m4
index aa3661f..e42719e 100644
--- i/gdb/build-with-cxx.m4
+++ w/gdb/build-with-cxx.m4
@@ -30,6 +30,7 @@ AC_DEFUN([GDB_AC_BUILD_WITH_CXX],
     *-*nto* | \
     *-*bsd* | \
     xtensa*-*-linux* | \
+    *-*-lynxos* | \
     null)
       enable_build_with_cxx=no ;;
     *)


That'll give you until all the other hosts are confirmed-converted,
at least.

> 
> If we do, perhaps it's time to change the major version number
> to 8.0?
> 

Or maybe defer that until we actually declare C mode unsupported.
I myself hope to do that before the next release, but ...  :-)

(Another thought that crossed my mind before is to switch the
numbering scheme to follow something like gcc's.  That is, make
the minor number be the major number going forward, as
the minor number actually counts new-feature releases, while
the major number is arbitrary.)

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 04/13/2016 12:07 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> I think the time has come to starting to default to building in C++ mode
>> on all hosts, except those known to not having been fully converted yet,
>> as with the patch at the top of the users/palves/cxx-conversion branch.
>>
>> Thoughts?
> 
> I read that patch on your tree, and that is quite clean and small.  I am
> fine on that.
> 
> We need to update buildbot builder Fedora-x86_64-cxx-build-m64 to build
> gdb in C mode.

Hi Sergio,

Could we have a build-only Fedora-x86_64-c-build-m64 builder, based on 
Fedora-x86_64-cxx-build-m64, but with explicit --enable-build-with-cxx=no ?

Thanks,
Pedro Alves

Re: C++ conversion status update

[Joel Brobecker]

> OK.  We can just add an entry to lynxos to GDB_AC_BUILD_WITH_CXX
> to make it default to C, still.  Like this, against
> the version in the users/palves/cxx-conversion branch:
> 
> diff --git i/gdb/build-with-cxx.m4 w/gdb/build-with-cxx.m4
> index aa3661f..e42719e 100644
> --- i/gdb/build-with-cxx.m4
> +++ w/gdb/build-with-cxx.m4
> @@ -30,6 +30,7 @@ AC_DEFUN([GDB_AC_BUILD_WITH_CXX],
>      *-*nto* | \
>      *-*bsd* | \
>      xtensa*-*-linux* | \
> +    *-*-lynxos* | \
>      null)
>        enable_build_with_cxx=no ;;
>      *)
> 
> That'll give you until all the other hosts are confirmed-converted,
> at least.

We could do that indeed; or, IIRC, there is a configure option
to force the use of C instead of C++. I don't mind requiring
the user to force it; it's a good reminder (and ties in nicely
with another comment below).

> > If we do, perhaps it's time to change the major version number
> > to 8.0?
> > 
> 
> Or maybe defer that until we actually declare C mode unsupported.
> I myself hope to do that before the next release, but ...  :-)

:-). I know it sounds self-serving, but I think we should be giving
users a one-release grace period. Make C++ the default, but with
C support for those who can't build with C++. That way, they can
still build, but they are clearly on notice about the impending
change. Then, as soon as the release branch is cut, we can flip
the switch. That way, I don't think you'll have to keep the compat
mode for much longer than you were planning to.

> (Another thought that crossed my mind before is to switch the
> numbering scheme to follow something like gcc's.  That is, make
> the minor number be the major number going forward, as
> the minor number actually counts new-feature releases, while
> the major number is arbitrary.)

Definitely a possibility. Let's discuss that separately at some point!

-- 
Joel

Re: C++ conversion status update

[Luis Machado]

On 04/13/2016 09:16 AM, Joel Brobecker wrote:
>> OK.  We can just add an entry to lynxos to GDB_AC_BUILD_WITH_CXX
>> to make it default to C, still.  Like this, against
>> the version in the users/palves/cxx-conversion branch:
>>
>> diff --git i/gdb/build-with-cxx.m4 w/gdb/build-with-cxx.m4
>> index aa3661f..e42719e 100644
>> --- i/gdb/build-with-cxx.m4
>> +++ w/gdb/build-with-cxx.m4
>> @@ -30,6 +30,7 @@ AC_DEFUN([GDB_AC_BUILD_WITH_CXX],
>>       *-*nto* | \
>>       *-*bsd* | \
>>       xtensa*-*-linux* | \
>> +    *-*-lynxos* | \
>>       null)
>>         enable_build_with_cxx=no ;;
>>       *)
>>
>> That'll give you until all the other hosts are confirmed-converted,
>> at least.
>
> We could do that indeed; or, IIRC, there is a configure option
> to force the use of C instead of C++. I don't mind requiring
> the user to force it; it's a good reminder (and ties in nicely
> with another comment below).
>
>>> If we do, perhaps it's time to change the major version number
>>> to 8.0?
>>>
>>
>> Or maybe defer that until we actually declare C mode unsupported.
>> I myself hope to do that before the next release, but ...  :-)
>
> :-). I know it sounds self-serving, but I think we should be giving
> users a one-release grace period. Make C++ the default, but with
> C support for those who can't build with C++. That way, they can
> still build, but they are clearly on notice about the impending
> change. Then, as soon as the release branch is cut, we can flip
> the switch. That way, I don't think you'll have to keep the compat
> mode for much longer than you were planning to.

I like the idea of the one-release grace period. We've been discussing 
this for years anyway, waiting 6 additional months does not sound so 
bad. :-)

That gives toolchains enough time to adapt. So we're probably talking 
GDB 7.13 built with a C++ compiler?

Re: C++ conversion status update

[Sergio Durigan Junior]

On Wednesday, April 13 2016, Pedro Alves wrote:

> On 04/13/2016 12:07 PM, Yao Qi wrote:
>> Pedro Alves <palves@redhat.com> writes:
>> 
>>> I think the time has come to starting to default to building in C++ mode
>>> on all hosts, except those known to not having been fully converted yet,
>>> as with the patch at the top of the users/palves/cxx-conversion branch.
>>>
>>> Thoughts?
>> 
>> I read that patch on your tree, and that is quite clean and small.  I am
>> fine on that.
>> 
>> We need to update buildbot builder Fedora-x86_64-cxx-build-m64 to build
>> gdb in C mode.
>
> Hi Sergio,
>
> Could we have a build-only Fedora-x86_64-c-build-m64 builder, based on 
> Fedora-x86_64-cxx-build-m64, but with explicit --enable-build-with-cxx=no ?

Hey,

Yes, for sure.  I've added it now.  Let me know if you need something
else.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/

RE: C++ conversion status update

[Marc Khouzam]

> > (Another thought that crossed my mind before is to switch the
> > numbering scheme to follow something like gcc's.  That is, make
> > the minor number be the major number going forward, as
> > the minor number actually counts new-feature releases, while
> > the major number is arbitrary.)
> 
> Definitely a possibility. Let's discuss that separately at some point!

I'm not sure if this was ever brought up before, but version numbers
don't have much meaning to users.  What is GDB 7.5?  Is that an old
version?  Is that the latest?  
Using a date scheme would be better, a la Ubuntu. When a user
see 16.04 they know they have the latest-and-greatest.

http://blog.codinghorror.com/whats-in-a-version-number-anyway/

Just a thought.

Marc

Re: C++ conversion status update

[Pedro Alves]

On 04/13/2016 03:16 PM, Joel Brobecker wrote:
>> OK.  We can just add an entry to lynxos to GDB_AC_BUILD_WITH_CXX
>> to make it default to C, still.  Like this, against
>> the version in the users/palves/cxx-conversion branch:
>>
>> diff --git i/gdb/build-with-cxx.m4 w/gdb/build-with-cxx.m4
>> index aa3661f..e42719e 100644
>> --- i/gdb/build-with-cxx.m4
>> +++ w/gdb/build-with-cxx.m4
>> @@ -30,6 +30,7 @@ AC_DEFUN([GDB_AC_BUILD_WITH_CXX],
>>      *-*nto* | \
>>      *-*bsd* | \
>>      xtensa*-*-linux* | \
>> +    *-*-lynxos* | \
>>      null)
>>        enable_build_with_cxx=no ;;
>>      *)
>>
>> That'll give you until all the other hosts are confirmed-converted,
>> at least.
> 
> We could do that indeed; or, IIRC, there is a configure option
> to force the use of C instead of C++.

It's --enable-build-with-cxx=yes|no.  That hunk above decides
the default for when the --enable-build-with-cxx option is not
specified.

 I don't mind requiring
> the user to force it; it's a good reminder (and ties in nicely
> with another comment below).

Hmm.  That's an excellent point.  If others are OK with it, I'm in
favor too.

>>> If we do, perhaps it's time to change the major version number
>>> to 8.0?
>>>
>>
>> Or maybe defer that until we actually declare C mode unsupported.
>> I myself hope to do that before the next release, but ...  :-)
> 
> :-). I know it sounds self-serving, but I think we should be giving
> users a one-release grace period. Make C++ the default, but with
> C support for those who can't build with C++. That way, they can
> still build, but they are clearly on notice about the impending
> change. Then, as soon as the release branch is cut, we can flip
> the switch. That way, I don't think you'll have to keep the compat
> mode for much longer than you were planning to.

That may well be.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Joel Brobecker]

> I'm not sure if this was ever brought up before, but version numbers
> don't have much meaning to users.  What is GDB 7.5?  Is that an old
> version?  Is that the latest?  
> Using a date scheme would be better, a la Ubuntu. When a user
> see 16.04 they know they have the latest-and-greatest.
> 
> http://blog.codinghorror.com/whats-in-a-version-number-anyway/

I had the same thought. In fact, AdaCore is moving in that direction
as well.

-- 
Joel

Re: C++ conversion status update

[Pedro Alves]

Hi John,

Finally managed to get back to the ptrace prototype detection issue.

On 01/20/2016 11:33 PM, John Baldwin wrote:

> I see that you also have a patch (41bed896a) in that branch to fix ptrace()
> args detection (which also broke for me, though I just hacked config.h locally
> so I could work on the actual problems).
> 
> Unfortunately, your patch doesn't fix FreeBSD/amd64 with GCC 4.8 (at least).
> The detection of the ptrace return type ('int' on FreeBSD) fails and falls
> back to long and I think that prevents it from subsequently matching on the
> correct types for the argument list.
> 
> The error for the failing test for the return type (which should work instead
> I think) is:
> 
> configure:12453: /usr/local/bin/g++48 -c -pipe -DRL_NO_COMPAT -Wno-unused-function -Wno-unused-variable  -g -DLIBICONV_PLUG -g -fno-strict-aliasing  -DLIBICONV_PLUG  conftest.cpp >&5
> conftest.cpp:166:22: error: declaration of C function 'int ptrace()' conflicts with
>  EXTERN_C int ptrace ();
>                       ^
> In file included from conftest.cpp:154:0:
> /usr/include/sys/ptrace.h:185:5: error: previous declaration 'int ptrace(int, pid_t, caddr_t, int)' here
>  int ptrace(int _request, pid_t _pid, caddr_t _addr, int _data);
>      ^
> configure:12453: $? = 1
> configure: failed program was:
> ....
> | EXTERN_C int ptrace ();
> | 
> | int
> | main ()
> | {
> | 
> |   ;
> |   return 0;
> | }
> configure:12462: result: long
> configure:12470: checking types of arguments for ptrace
> 
> I think the problem is that whereas in C "int foo()" means the args to foo()
> are undefined, "int foo()" in C++ is equivalent to "int foo(void)".
> 
> I know that a comment in ptrace.ac4 mentions that C++ needs to be used to
> determine if the first argument to ptrace is an enum that needs a cast,
> but I wonder if we couldn't fall back to plain C for the return type
> check?  (I'm not sure which of the tests in ptrace.ac4 needs C++ to make
> the enum determination such that we could possibly shuffle things around
> to do the one test needed while in C++ mode and the rest in C?)
> 
> Another possibility would be to move the return type check into the big
> loop that currently checks the arguments as an outermost loop, something
> like:
> 
> for gdb_ret in 'int' 'long'; do
>   for gdb_arg1 in 'int' 'long'; do
>     ...
> 
> This would work in C++ mode I think.

Yes, agreed.  I've sent a patch that does that to the
list now:

 https://sourceware.org/ml/gdb-patches/2016-04/msg00374.html

If you could give it a try it'd be much appreciated.  I've pushed
it to the users/palves/ptrace-detection branch (at sourceware.org)
as well, for convenience.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 04/16/2016 01:21 AM, Pedro Alves wrote:
> Yes, agreed.  I've sent a patch that does that to the
> list now:
> 
>  https://sourceware.org/ml/gdb-patches/2016-04/msg00374.html
> 
> If you could give it a try it'd be much appreciated.  I've pushed
> it to the users/palves/ptrace-detection branch (at sourceware.org)
> as well, for convenience.

FYI, a fix is in master now.

Thanks,
Pedro Alves

Re: C++ conversion status update

[Pedro Alves]

On 04/13/2016 03:40 PM, Pedro Alves wrote:
> On 04/13/2016 03:16 PM, Joel Brobecker wrote:

>> We could do that indeed; or, IIRC, there is a configure option
>> to force the use of C instead of C++.
> 
> It's --enable-build-with-cxx=yes|no.  That hunk above decides
> the default for when the --enable-build-with-cxx option is not
> specified.
> 
>  I don't mind requiring
>> the user to force it; it's a good reminder (and ties in nicely
>> with another comment below).
> 
> Hmm.  That's an excellent point.  If others are OK with it, I'm in
> favor too.

I've posted a patch that does this now.

 https://sourceware.org/ml/gdb-patches/2016-04/msg00417.html
Thanks,
Pedro Alves

Re: C++ conversion status update

[John Baldwin]

On Monday, April 18, 2016 05:51:12 PM Pedro Alves wrote:
> On 04/16/2016 01:21 AM, Pedro Alves wrote:
> > Yes, agreed.  I've sent a patch that does that to the
> > list now:
> > 
> >  https://sourceware.org/ml/gdb-patches/2016-04/msg00374.html
> > 
> > If you could give it a try it'd be much appreciated.  I've pushed
> > it to the users/palves/ptrace-detection branch (at sourceware.org)
> > as well, for convenience.
> 
> FYI, a fix is in master now.

Thanks, I was able to test it and it works great on FreeBSD/amd64.

I have a set of simple C++ build fixes for fbsd-nat.c I will post to patches@
in a bit.

The only remaining issue is that FreeBSD's stack_t defines ss_sp as char *
instead of void *.  Apparently 4.4BSD had this and the other BSD's fixed this
long ago.  When I first ran into this in January I fixed FreeBSD's trunk, so
11.0 will ship with a proper ss_sp of void *, but older releases will not.

The affected code is in setup_alternate_signal_stack() in gdb/main.c where
ss_sp is assigned to the void * returned from xmalloc().

I was torn between just supporting C++ builds on FreeBSD 11 and later, or
adding autoconf glue for just this part.  However, given that it seems like
the recent discussion is to deprecate C mode in the near future, it seems
like I should  do the latter.  Do you have any better suggestions?

-- 
John Baldwin

Re: C++ conversion status update

[Pedro Alves]

On 04/19/2016 07:22 PM, John Baldwin wrote:
> Thanks, I was able to test it and it works great on FreeBSD/amd64.

Great, thanks for confirming.

> The only remaining issue is that FreeBSD's stack_t defines ss_sp as char *
> instead of void *.  Apparently 4.4BSD had this and the other BSD's fixed this
> long ago.  When I first ran into this in January I fixed FreeBSD's trunk, so
> 11.0 will ship with a proper ss_sp of void *, but older releases will not.
> 

> The affected code is in setup_alternate_signal_stack() in gdb/main.c where
> ss_sp is assigned to the void * returned from xmalloc().
> 
> I was torn between just supporting C++ builds on FreeBSD 11 and later, or
> adding autoconf glue for just this part.  However, given that it seems like
> the recent discussion is to deprecate C mode in the near future, it seems
> like I should  do the latter.  Do you have any better suggestions?
> 

C++ allows conversion to 'void *' without a cast.  So I think
we just need to do:

 - ss.ss_sp = xmalloc (SIGSTKSZ);
 + ss.ss_sp = (char *) xmalloc (SIGSTKSZ);

and it'll work on both char * and void * hosts.  I tried it
here on GNU/Linux, where ss_sp is void *, and it works.

Would you like to send that as a patch (with a comment)?

We can leave adding an autoconf check if some other host uses
a type different than void * and char *.

Thanks,
Pedro Alves

Re: C++ conversion status update

[John Baldwin]

On Tuesday, April 19, 2016 09:36:33 PM Pedro Alves wrote:
> On 04/19/2016 07:22 PM, John Baldwin wrote:
> > Thanks, I was able to test it and it works great on FreeBSD/amd64.
> 
> Great, thanks for confirming.
> 
> > The only remaining issue is that FreeBSD's stack_t defines ss_sp as char *
> > instead of void *.  Apparently 4.4BSD had this and the other BSD's fixed this
> > long ago.  When I first ran into this in January I fixed FreeBSD's trunk, so
> > 11.0 will ship with a proper ss_sp of void *, but older releases will not.
> > 
> 
> > The affected code is in setup_alternate_signal_stack() in gdb/main.c where
> > ss_sp is assigned to the void * returned from xmalloc().
> > 
> > I was torn between just supporting C++ builds on FreeBSD 11 and later, or
> > adding autoconf glue for just this part.  However, given that it seems like
> > the recent discussion is to deprecate C mode in the near future, it seems
> > like I should  do the latter.  Do you have any better suggestions?
> > 
> 
> C++ allows conversion to 'void *' without a cast.  So I think
> we just need to do:
> 
>  - ss.ss_sp = xmalloc (SIGSTKSZ);
>  + ss.ss_sp = (char *) xmalloc (SIGSTKSZ);
> 
> and it'll work on both char * and void * hosts.  I tried it
> here on GNU/Linux, where ss_sp is void *, and it works.
> 
> Would you like to send that as a patch (with a comment)?
> 
> We can leave adding an autoconf check if some other host uses
> a type different than void * and char *.

Ok, I will include that in my current patchset.

-- 
John Baldwin

Re: C++ conversion status update

[Pedro Alves]

On 04/19/2016 10:40 PM, John Baldwin wrote:
> On Tuesday, April 19, 2016 09:36:33 PM Pedro Alves wrote:

>> C++ allows conversion to 'void *' without a cast.  So I think
>> we just need to do:
>>
>>  - ss.ss_sp = xmalloc (SIGSTKSZ);
>>  + ss.ss_sp = (char *) xmalloc (SIGSTKSZ);
>>
>> and it'll work on both char * and void * hosts.  I tried it
>> here on GNU/Linux, where ss_sp is void *, and it works.
>>
>> Would you like to send that as a patch (with a comment)?
>>
>> We can leave adding an autoconf check if some other host uses
>> a type different than void * and char *.
> 
> Ok, I will include that in my current patchset.

Great, thanks much.

-- 
Pedro Alves