[PATCH 1/2] [gdb/build] Fix build with -std=c++11

[PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Tom de Vries]

When building with -std=c++11, we run into two Werror=missing-declarations:
...
new-op.cc: In function 'void operator delete(void*, std::size_t)':
new-op.cc:114:1: error: no previous declaration for \
  'void operator delete(void*, std::size_t)' [-Werror=missing-declarations]
 operator delete (void *p, std::size_t) noexcept
 ^~~~~~~~
new-op.cc: In function 'void operator delete [](void*, std::size_t)':
new-op.cc:132:1: error: no previous declaration for \
  'void operator delete [](void*, std::size_t)' [-Werror=missing-declarations]
 operator delete[] (void *p, std::size_t) noexcept
 ^~~~~~~~
...

These are due to recent commit 5fff6115fea "Fix
LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".

The declarations are provided by <new> (which is included) for c++14 onwards,
but they are missing for c++11.

Fix this by adding the missing declarations.

Tested on x86_64-linux, with gcc 7.5.0, both without (implying -std=gnu++14) and
with -std=c++11.
---
 gdbsupport/new-op.cc | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 2f4c71457b1..716fa1a66de 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -27,6 +27,11 @@
 #include "host-defs.h"
 #include <new>
 
+/* These are declared in <new> starting C++14.  Add these here to enable
+   compilation using C++11. */
+extern void operator delete (void *p, std::size_t) noexcept;
+extern void operator delete[] (void *p, std::size_t) noexcept;
+
 /* Override operator new / operator new[], in order to internal_error
    on allocation failure and thus query the user for abort/core
    dump/continue, just like xmalloc does.  We don't do this from a

base-commit: b08625af20ade108868edd6f31d1cee68889a0df
-- 
2.26.2

[PATCH 2/2] [gdb/build] Fix Wimplicit-exception-spec-mismatch in clang build

[Tom de Vries]

When building with clang 13 (and -std=gnu++17 to work around an issue in
string_view-selftests.c), we run into a few Wimplicit-exception-spec-mismatch
warnings:
...
src/gdbsupport/new-op.cc:102:1: error: function previously declared with an \
  explicit exception specification redeclared with an implicit exception \
  specification [-Werror,-Wimplicit-exception-spec-mismatch]
operator delete (void *p)
^
/usr/include/c++/11/new:130:6: note: previous declaration is here
void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
     ^
...

These are due to recent commit 5fff6115fea "Fix
LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".

Fix this by adding the missing noexcept.

Build on x86_64-linux, using gcc 7.5.0 and clang 13.0.0.
---
 gdbsupport/new-op.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 716fa1a66de..2fe5f648abb 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -104,7 +104,7 @@ operator new[] (std::size_t sz, const std::nothrow_t&) noexcept
    errors from AddressSanitizers.  */
 
 void
-operator delete (void *p)
+operator delete (void *p) noexcept
 {
   free (p);
 }
@@ -122,7 +122,7 @@ operator delete (void *p, std::size_t) noexcept
 }
 
 void
-operator delete[] (void *p)
+operator delete[] (void *p) noexcept
 {
   return ::operator delete (p);
 }
-- 
2.26.2

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Pedro Alves]

On 2021-11-10 13:29, Tom de Vries via Gdb-patches wrote:
> When building with -std=c++11, we run into two Werror=missing-declarations:
> ...
> new-op.cc: In function 'void operator delete(void*, std::size_t)':
> new-op.cc:114:1: error: no previous declaration for \
>   'void operator delete(void*, std::size_t)' [-Werror=missing-declarations]
>  operator delete (void *p, std::size_t) noexcept
>  ^~~~~~~~
> new-op.cc: In function 'void operator delete [](void*, std::size_t)':
> new-op.cc:132:1: error: no previous declaration for \
>   'void operator delete [](void*, std::size_t)' [-Werror=missing-declarations]
>  operator delete[] (void *p, std::size_t) noexcept
>  ^~~~~~~~
> ...
> 
> These are due to recent commit 5fff6115fea "Fix
> LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".
> 
> The declarations are provided by <new> (which is included) for c++14 onwards,
> but they are missing for c++11.
> 
> Fix this by adding the missing declarations.
> 
> Tested on x86_64-linux, with gcc 7.5.0, both without (implying -std=gnu++14) and
> with -std=c++11.

OK.

Re: [PATCH 2/2] [gdb/build] Fix Wimplicit-exception-spec-mismatch in clang build

[Pedro Alves]

On 2021-11-10 13:29, Tom de Vries via Gdb-patches wrote:
> When building with clang 13 (and -std=gnu++17 to work around an issue in
> string_view-selftests.c), we run into a few Wimplicit-exception-spec-mismatch
> warnings:
> ...
> src/gdbsupport/new-op.cc:102:1: error: function previously declared with an \
>   explicit exception specification redeclared with an implicit exception \
>   specification [-Werror,-Wimplicit-exception-spec-mismatch]
> operator delete (void *p)
> ^
> /usr/include/c++/11/new:130:6: note: previous declaration is here
> void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
>      ^
> ...
> 
> These are due to recent commit 5fff6115fea "Fix
> LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".
> 
> Fix this by adding the missing noexcept.
> 
> Build on x86_64-linux, using gcc 7.5.0 and clang 13.0.0.

OK.

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Simon Marchi]

On 2021-11-11 05:16, Pedro Alves wrote:
> On 2021-11-10 13:29, Tom de Vries via Gdb-patches wrote:
>> When building with -std=c++11, we run into two Werror=missing-declarations:
>> ...
>> new-op.cc: In function 'void operator delete(void*, std::size_t)':
>> new-op.cc:114:1: error: no previous declaration for \
>>   'void operator delete(void*, std::size_t)' [-Werror=missing-declarations]
>>  operator delete (void *p, std::size_t) noexcept
>>  ^~~~~~~~
>> new-op.cc: In function 'void operator delete [](void*, std::size_t)':
>> new-op.cc:132:1: error: no previous declaration for \
>>   'void operator delete [](void*, std::size_t)' [-Werror=missing-declarations]
>>  operator delete[] (void *p, std::size_t) noexcept
>>  ^~~~~~~~
>> ...
>>
>> These are due to recent commit 5fff6115fea "Fix
>> LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".
>>
>> The declarations are provided by <new> (which is included) for c++14 onwards,
>> but they are missing for c++11.
>>
>> Fix this by adding the missing declarations.
>>
>> Tested on x86_64-linux, with gcc 7.5.0, both without (implying -std=gnu++14) and
>> with -std=c++11.
> 
> OK.
> 

It's not terribly important, but I was wondering whether this could be
fixed by adding the appropriate #ifdefs, to include or exclude the
problematic definitions.  Providing our own declarations works too, but
with this there's always the risk that the signature is not exactly
right right, and we won't define exactly the right function.

Simon

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Tom de Vries]

On 11/11/21 12:41 PM, Simon Marchi wrote:
> On 2021-11-11 05:16, Pedro Alves wrote:
>> On 2021-11-10 13:29, Tom de Vries via Gdb-patches wrote:
>>> When building with -std=c++11, we run into two Werror=missing-declarations:
>>> ...
>>> new-op.cc: In function 'void operator delete(void*, std::size_t)':
>>> new-op.cc:114:1: error: no previous declaration for \
>>>   'void operator delete(void*, std::size_t)' [-Werror=missing-declarations]
>>>  operator delete (void *p, std::size_t) noexcept
>>>  ^~~~~~~~
>>> new-op.cc: In function 'void operator delete [](void*, std::size_t)':
>>> new-op.cc:132:1: error: no previous declaration for \
>>>   'void operator delete [](void*, std::size_t)' [-Werror=missing-declarations]
>>>  operator delete[] (void *p, std::size_t) noexcept
>>>  ^~~~~~~~
>>> ...
>>>
>>> These are due to recent commit 5fff6115fea "Fix
>>> LD_PRELOAD=/usr/lib64/libasan.so.6 gdb".
>>>
>>> The declarations are provided by <new> (which is included) for c++14 onwards,
>>> but they are missing for c++11.
>>>
>>> Fix this by adding the missing declarations.
>>>
>>> Tested on x86_64-linux, with gcc 7.5.0, both without (implying -std=gnu++14) and
>>> with -std=c++11.
>>
>> OK.
>>
> 
> It's not terribly important, but I was wondering whether this could be
> fixed by adding the appropriate #ifdefs, to include or exclude the
> problematic definitions.  Providing our own declarations works too, but
> with this there's always the risk that the signature is not exactly
> right right, and we won't define exactly the right function.

I also wondered about this, but AFAIU we cannot rely on a c++11
implementation to provide some functionality if the user defines the
feature macro.  It's supposed to work the other way round: using the
feature macro we detect whether something is supported.

It would be nice if we could do something like:
...
/* Tentatively define feature macro.  */
#define __cpp_sized_deallocation
#include <new>

#if still_not_defined
extern void operator delete (void *p, std::size_t) noexcept;
extern void operator delete[] (void *p, std::size_t) noexcept;
#endif
...
but I'm not sure how to formulate the still_not_defined test.  Perhaps
we could figure that out in a configure test, but I'm not sure that's
worth the effort.

Alternatively, we could do things unconditionally, like so:
...
/* Tentatively define feature macro.  */
#define __cpp_sized_deallocation
#include <new>

extern void operator delete (void *p, std::size_t) noexcept;
extern void operator delete[] (void *p, std::size_t) noexcept;
...
to detect declaration mismatches.

Thanks,
- Tom

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Pedro Alves]

On 2021-11-15 13:55, Tom de Vries wrote:
> On 11/11/21 12:41 PM, Simon Marchi wrote:

>> It's not terribly important, but I was wondering whether this could be
>> fixed by adding the appropriate #ifdefs, to include or exclude the
>> problematic definitions.  Providing our own declarations works too, but
>> with this there's always the risk that the signature is not exactly
>> right right, and we won't define exactly the right function.
> 
> I also wondered about this, but AFAIU we cannot rely on a c++11
> implementation to provide some functionality if the user defines the
> feature macro.  It's supposed to work the other way round: using the
> feature macro we detect whether something is supported.
> 
> It would be nice if we could do something like:
> ...
> /* Tentatively define feature macro.  */
> #define __cpp_sized_deallocation
> #include <new>
> 
> #if still_not_defined
> extern void operator delete (void *p, std::size_t) noexcept;
> extern void operator delete[] (void *p, std::size_t) noexcept;
> #endif
> ...
> but I'm not sure how to formulate the still_not_defined test.  Perhaps
> we could figure that out in a configure test, but I'm not sure that's
> worth the effort.
> 
> Alternatively, we could do things unconditionally, like so:
> ...
> /* Tentatively define feature macro.  */
> #define __cpp_sized_deallocation
> #include <new>
> 
> extern void operator delete (void *p, std::size_t) noexcept;
> extern void operator delete[] (void *p, std::size_t) noexcept;
> ...
> to detect declaration mismatches.

I suspect the risk of defining a compiler/library feature macro (I'd guess this
is undefined behavior land) like __cpp_sized_deallocation behind the compiler's back and
that going wrong is higher than ending up with misdetected declaration mismatches.

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Simon Marchi]

On 2021-11-15 9:05 a.m., Pedro Alves wrote:
> On 2021-11-15 13:55, Tom de Vries wrote:
>> On 11/11/21 12:41 PM, Simon Marchi wrote:
> 
>>> It's not terribly important, but I was wondering whether this could be
>>> fixed by adding the appropriate #ifdefs, to include or exclude the
>>> problematic definitions.  Providing our own declarations works too, but
>>> with this there's always the risk that the signature is not exactly
>>> right right, and we won't define exactly the right function.
>>
>> I also wondered about this, but AFAIU we cannot rely on a c++11
>> implementation to provide some functionality if the user defines the
>> feature macro.  It's supposed to work the other way round: using the
>> feature macro we detect whether something is supported.
>>
>> It would be nice if we could do something like:
>> ...
>> /* Tentatively define feature macro.  */
>> #define __cpp_sized_deallocation
>> #include <new>
>>
>> #if still_not_defined
>> extern void operator delete (void *p, std::size_t) noexcept;
>> extern void operator delete[] (void *p, std::size_t) noexcept;
>> #endif
>> ...
>> but I'm not sure how to formulate the still_not_defined test.  Perhaps
>> we could figure that out in a configure test, but I'm not sure that's
>> worth the effort.
>>
>> Alternatively, we could do things unconditionally, like so:
>> ...
>> /* Tentatively define feature macro.  */
>> #define __cpp_sized_deallocation
>> #include <new>
>>
>> extern void operator delete (void *p, std::size_t) noexcept;
>> extern void operator delete[] (void *p, std::size_t) noexcept;
>> ...
>> to detect declaration mismatches.
> 
> I suspect the risk of defining a compiler/library feature macro (I'd guess this
> is undefined behavior land) like __cpp_sized_deallocation behind the compiler's back and
> that going wrong is higher than ending up with misdetected declaration mismatches.
> 

I'm probably missing something, but couldn't we do just:

#if defined(__cpp_sized_deallocation)
extern void operator delete (void *p, std::size_t) noexcept;
extern void operator delete[] (void *p, std::size_t) noexcept;
#endif

?

Simon

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Pedro Alves]

On 2021-11-15 16:32, Simon Marchi wrote:
> On 2021-11-15 9:05 a.m., Pedro Alves wrote:

>> I suspect the risk of defining a compiler/library feature macro (I'd guess this
>> is undefined behavior land) like __cpp_sized_deallocation behind the compiler's back and
>> that going wrong is higher than ending up with misdetected declaration mismatches.
>>
> 

> I'm probably missing something, but couldn't we do just:
> 
> #if defined(__cpp_sized_deallocation)
> extern void operator delete (void *p, std::size_t) noexcept;
> extern void operator delete[] (void *p, std::size_t) noexcept;
> #endif
> 
> ?

That should work, yes.  Not sure it's a benefit, though?  As is, if we declare
the function incorrectly, we'll notice it via a redeclaration error on a modern
compiler.  With that change, we'd never notice such a mismatch.

Pedro Alves

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Simon Marchi]

On 2021-11-15 11:43 a.m., Pedro Alves wrote:
> On 2021-11-15 16:32, Simon Marchi wrote:
>> On 2021-11-15 9:05 a.m., Pedro Alves wrote:
>
>>> I suspect the risk of defining a compiler/library feature macro (I'd guess this
>>> is undefined behavior land) like __cpp_sized_deallocation behind the compiler's back and
>>> that going wrong is higher than ending up with misdetected declaration mismatches.
>>>
>>
>
>> I'm probably missing something, but couldn't we do just:
>>
>> #if defined(__cpp_sized_deallocation)
>> extern void operator delete (void *p, std::size_t) noexcept;
>> extern void operator delete[] (void *p, std::size_t) noexcept;
>> #endif
>>
>> ?
>
> That should work, yes.  Not sure it's a benefit, though?  As is, if we declare
> the function incorrectly, we'll notice it via a redeclaration error on a modern
> compiler.  With that change, we'd never notice such a mismatch.
>
> Pedro Alves
>

Arg, I got confused sorry (I thought about this last week and only
typed it out today).  I actually meant to make the definitions
conditional, this patch basically:


diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 2fe5f648abb..f881799522d 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -27,11 +27,6 @@
 #include "host-defs.h"
 #include <new>

-/* These are declared in <new> starting C++14.  Add these here to enable
-   compilation using C++11. */
-extern void operator delete (void *p, std::size_t) noexcept;
-extern void operator delete[] (void *p, std::size_t) noexcept;
-
 /* Override operator new / operator new[], in order to internal_error
    on allocation failure and thus query the user for abort/core
    dump/continue, just like xmalloc does.  We don't do this from a
@@ -115,12 +110,16 @@ operator delete (void *p, const std::nothrow_t&) noexcept
   return ::operator delete (p);
 }

+#if defined(__cpp_sized_deallocation)
+
 void
 operator delete (void *p, std::size_t) noexcept
 {
   return ::operator delete (p, std::nothrow);
 }

+#endif
+
 void
 operator delete[] (void *p) noexcept
 {
@@ -133,6 +132,8 @@ operator delete[] (void *p, const std::nothrow_t&) noexcept
   return ::operator delete (p, std::nothrow);
 }

+#if defined(__cpp_sized_deallocation)
+
 void
 operator delete[] (void *p, std::size_t) noexcept
 {
@@ -140,3 +141,5 @@ operator delete[] (void *p, std::size_t) noexcept
 }

 #endif
+
+#endif

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Pedro Alves]

On 2021-11-15 18:23, Simon Marchi wrote:
> On 2021-11-15 11:43 a.m., Pedro Alves wrote:
>> On 2021-11-15 16:32, Simon Marchi wrote:
>>> On 2021-11-15 9:05 a.m., Pedro Alves wrote:
>>
>>>> I suspect the risk of defining a compiler/library feature macro (I'd guess this
>>>> is undefined behavior land) like __cpp_sized_deallocation behind the compiler's back and
>>>> that going wrong is higher than ending up with misdetected declaration mismatches.
>>>>
>>>
>>
>>> I'm probably missing something, but couldn't we do just:
>>>
>>> #if defined(__cpp_sized_deallocation)
>>> extern void operator delete (void *p, std::size_t) noexcept;
>>> extern void operator delete[] (void *p, std::size_t) noexcept;
>>> #endif
>>>
>>> ?
>>
>> That should work, yes.  Not sure it's a benefit, though?  As is, if we declare
>> the function incorrectly, we'll notice it via a redeclaration error on a modern
>> compiler.  With that change, we'd never notice such a mismatch.
>>
>> Pedro Alves
>>
> 
> Arg, I got confused sorry (I thought about this last week and only
> typed it out today).  I actually meant to make the definitions
> conditional, this patch basically:

If GDB links with something that is itself built with C++ >= 14 (e.g., say, libsource-highlight.so),
it can end up calling these operators, even if GDB itself is built with C++ 11.  Can't the absence of
such operators defined in GDB still result in the ASAN mismatch errors described in the original
commit from Jan (5fff6115feae)?

Pedro Alves

Re: [PATCH 1/2] [gdb/build] Fix build with -std=c++11

[Simon Marchi]

On 2021-11-15 1:42 p.m., Pedro Alves wrote:
> If GDB links with something that is itself built with C++ >= 14 (e.g., say, libsource-highlight.so),
> it can end up calling these operators, even if GDB itself is built with C++ 11.  Can't the absence of
> such operators defined in GDB still result in the ASAN mismatch errors described in the original
> commit from Jan (5fff6115feae)?


Hmmm maybe, I haven't tested :)

Simon