[PATCH] Fix building gdb with gcc-4.x

[PATCH] Fix building gdb with gcc-4.x

[Bernd Edlinger]

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

Hi,

with Luis' commit of today the trunk is no longer able to
be compiled with gcc-4.x.

The problem is std::is_trivially_default_constructible is
not defined before gcc-5 although the compiler supports C++11 

I am not sure about what's the best approach for conditionally
enabling the code, especially for compilers other than g++.


This fixes the build for me.
Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-building-gdb-with-gcc-4.x.patch --]
[-- Type: text/x-patch; name="0001-Fix-building-gdb-with-gcc-4.x.patch", Size: 1632 bytes --]

From 0879e2cff0ea732769aa80b89fa6fbba70c84260 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Mon, 4 Jan 2021 21:40:41 +0100
Subject: [PATCH] Fix building gdb with gcc-4.x
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since is_trivially_default_constructible was not implemented before gcc-5
it cannot be used with gcc-4.x.

../../binutils-gdb/gdb/trad-frame.c: In function ‘trad_frame_saved_reg* trad_frame_alloc_saved_regs(gdbarch*)’:
../../binutils-gdb/gdb/trad-frame.c:64:22: error: ‘is_trivially_default_constructible’ is not a member of ‘std’
   gdb_static_assert (std::is_trivially_default_constructible<trad_frame_saved_reg>::value);

Fix build by using conditional compilation around that line.

Fixes: 098caef485a ("Refactor struct trad_frame_saved_regs")

gdb:
2021-01-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

        * trad-frame.c (trad_frame_alloc_saved_regs): Avoid compile-error
	because is_trivially_default_constructible was first implemented with
	gcc-5.
---
 gdb/trad-frame.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c
index 17375e8..de1acc1 100644
--- a/gdb/trad-frame.c
+++ b/gdb/trad-frame.c
@@ -60,7 +60,9 @@ struct trad_frame_cache *
 trad_frame_saved_reg *
 trad_frame_alloc_saved_regs (struct gdbarch *gdbarch)
 {
+#if defined(__GNUC__) && __GNUC__ >= 5
   gdb_static_assert (std::is_trivially_default_constructible<trad_frame_saved_reg>::value);
+#endif
 
   int numregs = gdbarch_num_cooked_regs (gdbarch);
   trad_frame_saved_reg *this_saved_regs
-- 
1.9.1

Re: [PATCH] Fix building gdb with gcc-4.x

[Luis Machado]

Hi Bernd,

On 1/4/21 5:57 PM, Bernd Edlinger wrote:
> Hi,
> 
> with Luis' commit of today the trunk is no longer able to
> be compiled with gcc-4.x.
> 
> The problem is std::is_trivially_default_constructible is
> not defined before gcc-5 although the compiler supports C++11
> 

Sorry for the breakage. It looks like GCC 4.x does not fully support 
C++11, although it used to be able to build GDB.

> I am not sure about what's the best approach for conditionally
> enabling the code, especially for compilers other than g++.

Right now it looks like GDB's requirement is a compiler that (fully?) 
supports C++ 11. It doesn't seem to name specific minimum versions of 
compilers.

With that said, I wouldn't mind a conditional in the code to support 
builds with GCC 4.x, if that is deemed important. But as we start using 
more and more C++ 11 constructs, breakages may happen again in the future.

> 
> 
> This fixes the build for me.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

Re: [PATCH] Fix building gdb with gcc-4.x

[Simon Marchi]

On 2021-01-04 3:57 p.m., Bernd Edlinger wrote:
> Hi,
> 
> with Luis' commit of today the trunk is no longer able to
> be compiled with gcc-4.x.
> 
> The problem is std::is_trivially_default_constructible is
> not defined before gcc-5 although the compiler supports C++11 
> 
> I am not sure about what's the best approach for conditionally
> enabling the code, especially for compilers other than g++.
> 
> 
> This fixes the build for me.
> Is it OK for trunk?
> 
> 
> Thanks
> Bernd.
> 

We have other instances of this, I'd suggest inspiring yourself from:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/unittests/enum-flags-selftests.c;h=e3b6cf81d07f1eaefec72bce131d1c75ce00ef82;hb=HEAD#l66

These defines are defined here:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/traits.h;h=f545edbb0d93e95f65e954fbf54cbc8843e5239a;hb=HEAD#l28

As you see, we already have a HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, we could
use it.  The code in trad-frame.c could be changed to use
std::is_trivially_constructible instead of
std::is_trivially_default_constructible, I believe it's the same when
passing no Args... to is_trivially_constructible.  See "Possible
implementation" in:

https://en.cppreference.com/w/cpp/types/is_default_constructible

Simon

Re: [PATCH] Fix building gdb with gcc-4.x

[Luis Machado]

On 1/4/21 6:16 PM, Luis Machado wrote:
> Hi Bernd,
> 
> On 1/4/21 5:57 PM, Bernd Edlinger wrote:
>> Hi,
>>
>> with Luis' commit of today the trunk is no longer able to
>> be compiled with gcc-4.x.
>>
>> The problem is std::is_trivially_default_constructible is
>> not defined before gcc-5 although the compiler supports C++11
>>
> 
> Sorry for the breakage. It looks like GCC 4.x does not fully support 
> C++11, although it used to be able to build GDB.
> 
>> I am not sure about what's the best approach for conditionally
>> enabling the code, especially for compilers other than g++.
> 
> Right now it looks like GDB's requirement is a compiler that (fully?) 
> supports C++ 11. It doesn't seem to name specific minimum versions of 
> compilers.
> 
> With that said, I wouldn't mind a conditional in the code to support 
> builds with GCC 4.x, if that is deemed important. But as we start using 
> more and more C++ 11 constructs, breakages may happen again in the future.
> 
>>
>>
>> This fixes the build for me.
>> Is it OK for trunk?

It's worth mentioning that I'll be changing this code again in the 
future, and that static assertion will likely go away when I do.

Re: [PATCH] Fix building gdb with gcc-4.x

[Bernd Edlinger]

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

On 1/4/21 10:30 PM, Simon Marchi wrote:
> On 2021-01-04 3:57 p.m., Bernd Edlinger wrote:
>> Hi,
>>
>> with Luis' commit of today the trunk is no longer able to
>> be compiled with gcc-4.x.
>>
>> The problem is std::is_trivially_default_constructible is
>> not defined before gcc-5 although the compiler supports C++11 
>>
>> I am not sure about what's the best approach for conditionally
>> enabling the code, especially for compilers other than g++.
>>
>>
>> This fixes the build for me.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
> 
> We have other instances of this, I'd suggest inspiring yourself from:
> 
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/unittests/enum-flags-selftests.c;h=e3b6cf81d07f1eaefec72bce131d1c75ce00ef82;hb=HEAD#l66
> 
> These defines are defined here:
> 
> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/traits.h;h=f545edbb0d93e95f65e954fbf54cbc8843e5239a;hb=HEAD#l28
> 
> As you see, we already have a HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, we could
> use it.  The code in trad-frame.c could be changed to use
> std::is_trivially_constructible instead of
> std::is_trivially_default_constructible, I believe it's the same when
> passing no Args... to is_trivially_constructible.  See "Possible
> implementation" in:
> 
> https://en.cppreference.com/w/cpp/types/is_default_constructible
> 
> Simon
> 

Okay, good point.

I updated the patch following your suggestion.

Is the updated patch OK?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-building-gdb-with-gcc-4.x.patch --]
[-- Type: text/x-patch; name="0001-Fix-building-gdb-with-gcc-4.x.patch", Size: 2005 bytes --]

From 588e7a57056e8aabdd6906993dd3d27354cc70fc Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Mon, 4 Jan 2021 21:40:41 +0100
Subject: [PATCH] Fix building gdb with gcc-4.x
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Since is_trivially_default_constructible was not implemented before gcc-5
it cannot be used with gcc-4.x.

../../binutils-gdb/gdb/trad-frame.c: In function ‘trad_frame_saved_reg* trad_frame_alloc_saved_regs(gdbarch*)’:
../../binutils-gdb/gdb/trad-frame.c:64:22: error: ‘is_trivially_default_constructible’ is not a member of ‘std’
   gdb_static_assert (std::is_trivially_default_constructible<trad_frame_saved_reg>::value);

Fix the build by using conditional compilation around that line.
Use the equivalent is_trivially_constructible<T> instead, since
we already have HAVE_IS_TRIVIALLY_CONSTRUCTIBLE for that purpose.

Fixes: 098caef485a ("Refactor struct trad_frame_saved_regs")

gdb:
2021-01-14  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* trad-frame.c (trad_frame_alloc_saved_regs): Avoid compile-error
	because is_trivially_default_constructible was first implemented with
	gcc-5.
---
 gdb/trad-frame.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/trad-frame.c b/gdb/trad-frame.c
index 17375e8..3284c45 100644
--- a/gdb/trad-frame.c
+++ b/gdb/trad-frame.c
@@ -25,6 +25,7 @@
 #include "target.h"
 #include "value.h"
 #include "gdbarch.h"
+#include "gdbsupport/traits.h"
 
 struct trad_frame_cache
 {
@@ -60,7 +61,9 @@ struct trad_frame_cache *
 trad_frame_saved_reg *
 trad_frame_alloc_saved_regs (struct gdbarch *gdbarch)
 {
-  gdb_static_assert (std::is_trivially_default_constructible<trad_frame_saved_reg>::value);
+#ifdef HAVE_IS_TRIVIALLY_CONSTRUCTIBLE
+  gdb_static_assert (std::is_trivially_constructible<trad_frame_saved_reg>::value);
+#endif
 
   int numregs = gdbarch_num_cooked_regs (gdbarch);
   trad_frame_saved_reg *this_saved_regs
-- 
1.9.1

Re: [PATCH] Fix building gdb with gcc-4.x

[Simon Marchi]

On 2021-01-14 1:36 a.m., Bernd Edlinger wrote:
> On 1/4/21 10:30 PM, Simon Marchi wrote:
>> On 2021-01-04 3:57 p.m., Bernd Edlinger wrote:
>>> Hi,
>>>
>>> with Luis' commit of today the trunk is no longer able to
>>> be compiled with gcc-4.x.
>>>
>>> The problem is std::is_trivially_default_constructible is
>>> not defined before gcc-5 although the compiler supports C++11 
>>>
>>> I am not sure about what's the best approach for conditionally
>>> enabling the code, especially for compilers other than g++.
>>>
>>>
>>> This fixes the build for me.
>>> Is it OK for trunk?
>>>
>>>
>>> Thanks
>>> Bernd.
>>>
>>
>> We have other instances of this, I'd suggest inspiring yourself from:
>>
>> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdb/unittests/enum-flags-selftests.c;h=e3b6cf81d07f1eaefec72bce131d1c75ce00ef82;hb=HEAD#l66
>>
>> These defines are defined here:
>>
>> https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gdbsupport/traits.h;h=f545edbb0d93e95f65e954fbf54cbc8843e5239a;hb=HEAD#l28
>>
>> As you see, we already have a HAVE_IS_TRIVIALLY_CONSTRUCTIBLE, we could
>> use it.  The code in trad-frame.c could be changed to use
>> std::is_trivially_constructible instead of
>> std::is_trivially_default_constructible, I believe it's the same when
>> passing no Args... to is_trivially_constructible.  See "Possible
>> implementation" in:
>>
>> https://en.cppreference.com/w/cpp/types/is_default_constructible
>>
>> Simon
>>
> 
> Okay, good point.
> 
> I updated the patch following your suggestion.
> 
> Is the updated patch OK?
> 
> 
> Thanks
> Bernd.
> 

Yep, LGTM, thanks.

Simon