[PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Simon Marchi]

We see some additional failures when running the testsuite against a GDB
compiled with ASan, compared to a GDB compiled without ASan.  Some of
them are caused by the memory leak report shown by the GDB process when
it exits, and the fact that it makes it exit with a non-zero exit code.

I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
environment when running the tests, but I don't always do it.  I think
it would be nice if the testsuite did it.  I don't see any use to have
leak detection when running the tests.  That is, unless we ever have a
test that ensures GDB doesn't leak memory, which isn't going to happen
any time soon.

Here are some tests I found that were affected by this:

    gdb.base/batch-exit-status.exp
    gdb.base/many-headers.exp: read core file
    gdb.base/quit.exp
    gdb.base/with-mf.exp
    gdb.dwarf2/gdb-add-index.exp
    gdb.dwarf2/gdb-add-index-symlink.exp
    gdb.dwarf2/imported-unit-runto-main.exp

Change-Id: I784c7df8a13979eb96587f735c1d33ba2cc6e0ca
---
 gdb/testsuite/lib/gdb.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 7f02504262d..97bedd5cb58 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -25,6 +25,13 @@ if {$tool == ""} {
     exit 2
 }
 
+# If GDB is built with ASAN (and because there are leaks), it will output a
+# leak report when exiting as well as exit with a non-zero (failure) status.
+# This can affect tests that are sensitive to what GDB prints on stderr or its
+# exit status.  Add `detect_leaks=0` to the ASAN_OPTIONS environment variable
+# (which will affect any spawned sub-process) to avoid this.
+append ::env(ASAN_OPTIONS) ",detect_leaks=0"
+
 # List of procs to run in gdb_finish.
 set gdb_finish_hooks [list]
 
-- 
2.33.0

Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Tom de Vries]

On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
> We see some additional failures when running the testsuite against a GDB
> compiled with ASan, compared to a GDB compiled without ASan.  Some of
> them are caused by the memory leak report shown by the GDB process when
> it exits, and the fact that it makes it exit with a non-zero exit code.
> 
> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
> environment when running the tests, but I don't always do it.  I think
> it would be nice if the testsuite did it.  I don't see any use to have
> leak detection when running the tests.  That is, unless we ever have a
> test that ensures GDB doesn't leak memory, which isn't going to happen
> any time soon.
> 

I like the idea.  I also use the setting in my test scripts.

FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
triggered at some point.  I've just done a run without this setting, and
it didn't trigger anything.

I tried to understand why, and stumbled onto
https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
which seems to have been approved, but never committed.  The problem
described there, using LD_PRELOAD like so:
...
$ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
...
does reproduce for me, and applying the patch fixes it.  I've done a
build and test run, and will commit shortly.

Also there a few test-cases which fail when using asan, we could
annotate those perhaps with abort_on_error=1 or some such.

Thanks,
- Tom

> Here are some tests I found that were affected by this:
> 
>     gdb.base/batch-exit-status.exp
>     gdb.base/many-headers.exp: read core file
>     gdb.base/quit.exp
>     gdb.base/with-mf.exp
>     gdb.dwarf2/gdb-add-index.exp
>     gdb.dwarf2/gdb-add-index-symlink.exp
>     gdb.dwarf2/imported-unit-runto-main.exp
> 
> Change-Id: I784c7df8a13979eb96587f735c1d33ba2cc6e0ca
> ---
>  gdb/testsuite/lib/gdb.exp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index 7f02504262d..97bedd5cb58 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -25,6 +25,13 @@ if {$tool == ""} {
>      exit 2
>  }
>  
> +# If GDB is built with ASAN (and because there are leaks), it will output a
> +# leak report when exiting as well as exit with a non-zero (failure) status.
> +# This can affect tests that are sensitive to what GDB prints on stderr or its
> +# exit status.  Add `detect_leaks=0` to the ASAN_OPTIONS environment variable
> +# (which will affect any spawned sub-process) to avoid this.
> +append ::env(ASAN_OPTIONS) ",detect_leaks=0"
> +
>  # List of procs to run in gdb_finish.
>  set gdb_finish_hooks [list]
>  
> 

Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Tom de Vries]

On 11/3/21 11:24 AM, Tom de Vries wrote:
> On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
>> We see some additional failures when running the testsuite against a GDB
>> compiled with ASan, compared to a GDB compiled without ASan.  Some of
>> them are caused by the memory leak report shown by the GDB process when
>> it exits, and the fact that it makes it exit with a non-zero exit code.
>>
>> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
>> environment when running the tests, but I don't always do it.  I think
>> it would be nice if the testsuite did it.  I don't see any use to have
>> leak detection when running the tests.  That is, unless we ever have a
>> test that ensures GDB doesn't leak memory, which isn't going to happen
>> any time soon.
>>
> 
> I like the idea.  I also use the setting in my test scripts.
> 
> FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
> triggered at some point.  I've just done a run without this setting, and
> it didn't trigger anything.
> 
> I tried to understand why, and stumbled onto
> https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
> which seems to have been approved, but never committed.  The problem
> described there, using LD_PRELOAD like so:
> ...
> $ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
> ...
> does reproduce for me, and applying the patch fixes it.  I've done a
> build and test run, and will commit shortly.
> 
> Also there a few test-cases which fail when using asan, we could
> annotate those perhaps with abort_on_error=1 or some such.

FTR, I meant f.i.:
...
AddressSanitizer can not provide additional info.^M
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xf15fa) in __libc_poll^M
==23338==ABORTING^M
FAIL: gdb.base/bt-on-fatal-signal.exp: SEGV: $saw_fatal_msg
...
but I didn't find an ASAN_OPTIONS settting to disable this.

BTW, this is with gcc 7.5.0 (using libasan.so.4 according to ldd), I
don't see this when doing an address sanitizer build with gcc 11.2.1,
but then I see fails in gdb.base/early-init-file.exp.

Thanks,
- Tom

Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Simon Marchi]

On 2021-11-03 8:28 a.m., Tom de Vries via Gdb-patches wrote:
> On 11/3/21 11:24 AM, Tom de Vries wrote:
>> On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
>>> We see some additional failures when running the testsuite against a GDB
>>> compiled with ASan, compared to a GDB compiled without ASan.  Some of
>>> them are caused by the memory leak report shown by the GDB process when
>>> it exits, and the fact that it makes it exit with a non-zero exit code.
>>>
>>> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
>>> environment when running the tests, but I don't always do it.  I think
>>> it would be nice if the testsuite did it.  I don't see any use to have
>>> leak detection when running the tests.  That is, unless we ever have a
>>> test that ensures GDB doesn't leak memory, which isn't going to happen
>>> any time soon.
>>>
>>
>> I like the idea.  I also use the setting in my test scripts.

Ok, I'll push that patch.

>> FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
>> triggered at some point.  I've just done a run without this setting, and
>> it didn't trigger anything.
>>
>> I tried to understand why, and stumbled onto
>> https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
>> which seems to have been approved, but never committed.  The problem
>> described there, using LD_PRELOAD like so:
>> ...
>> $ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
>> ...
>> does reproduce for me, and applying the patch fixes it.  I've done a
>> build and test run, and will commit shortly.

Ok, thanks.  Since the issue is fixed, do you think we still need to add
alloc_dealloc_mismatch=0?  I would think not.  If an
alloc_dealloc_mismatch problem is ever introduced, then that would be a
GDB bug needing to be fixed.

>> Also there a few test-cases which fail when using asan, we could
>> annotate those perhaps with abort_on_error=1 or some such.
>
> FTR, I meant f.i.:
> ...
> AddressSanitizer can not provide additional info.^M
> SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xf15fa) in __libc_poll^M
> ==23338==ABORTING^M
> FAIL: gdb.base/bt-on-fatal-signal.exp: SEGV: $saw_fatal_msg
> ...
> but I didn't find an ASAN_OPTIONS settting to disable this.
>
> BTW, this is with gcc 7.5.0 (using libasan.so.4 according to ldd), I
> don't see this when doing an address sanitizer build with gcc 11.2.1,
> but then I see fails in gdb.base/early-init-file.exp.

Indeed.  I haven't compared a no-asan build with an asan build yet.

I am not sure how abort_on_error would here in
gdb.base/bt-on-fatal-signal.exp.  The output that the test is looking
for seems to be printed regardless of whether ASan calls abort later or
not.  But I am trying with gcc 11, so perhaps that's the difference with
gcc 7 you are talking about.  In any case, I wouldn't mind enabling
abort_on_error all the time, that may just help making crashes more
apparent.

Also, looking at:

  https://clang.llvm.org/docs/AddressSanitizer.html

... it seems like there are some disabled-by-default checks that we
could enable, like "check_initialization_order=1" and
"detect_stack_use_after_return=1".

Simon

Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Tom de Vries]

On 11/3/21 3:45 PM, Simon Marchi wrote:
> On 2021-11-03 8:28 a.m., Tom de Vries via Gdb-patches wrote:
>> On 11/3/21 11:24 AM, Tom de Vries wrote:
>>> On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
>>>> We see some additional failures when running the testsuite against a GDB
>>>> compiled with ASan, compared to a GDB compiled without ASan.  Some of
>>>> them are caused by the memory leak report shown by the GDB process when
>>>> it exits, and the fact that it makes it exit with a non-zero exit code.
>>>>
>>>> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
>>>> environment when running the tests, but I don't always do it.  I think
>>>> it would be nice if the testsuite did it.  I don't see any use to have
>>>> leak detection when running the tests.  That is, unless we ever have a
>>>> test that ensures GDB doesn't leak memory, which isn't going to happen
>>>> any time soon.
>>>>
>>>
>>> I like the idea.  I also use the setting in my test scripts.
> 
> Ok, I'll push that patch.
> 
>>> FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
>>> triggered at some point.  I've just done a run without this setting, and
>>> it didn't trigger anything.
>>>
>>> I tried to understand why, and stumbled onto
>>> https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
>>> which seems to have been approved, but never committed.  The problem
>>> described there, using LD_PRELOAD like so:
>>> ...
>>> $ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
>>> ...
>>> does reproduce for me, and applying the patch fixes it.  I've done a
>>> build and test run, and will commit shortly.
> 
> Ok, thanks.  Since the issue is fixed, do you think we still need to add
> alloc_dealloc_mismatch=0?  I would think not.

Agreed.

> If an
> alloc_dealloc_mismatch problem is ever introduced, then that would be a
> GDB bug needing to be fixed.
> 

Ack.

>>> Also there a few test-cases which fail when using asan, we could
>>> annotate those perhaps with abort_on_error=1 or some such.
>>
>> FTR, I meant f.i.:
>> ...
>> AddressSanitizer can not provide additional info.^M
>> SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0xf15fa) in __libc_poll^M
>> ==23338==ABORTING^M
>> FAIL: gdb.base/bt-on-fatal-signal.exp: SEGV: $saw_fatal_msg
>> ...
>> but I didn't find an ASAN_OPTIONS settting to disable this.
>>
>> BTW, this is with gcc 7.5.0 (using libasan.so.4 according to ldd), I
>> don't see this when doing an address sanitizer build with gcc 11.2.1,
>> but then I see fails in gdb.base/early-init-file.exp.
> 
> Indeed.  I haven't compared a no-asan build with an asan build yet.
> 
> I am not sure how abort_on_error would here in
> gdb.base/bt-on-fatal-signal.exp.  The output that the test is looking
> for seems to be printed regardless of whether ASan calls abort later or
> not.  But I am trying with gcc 11, so perhaps that's the difference with
> gcc 7 you are talking about.  In any case, I wouldn't mind enabling
> abort_on_error all the time, that may just help making crashes more
> apparent.
> 

Sorry, copy-pasto, I meant abort_on_error=0 in that particular
test-case, but as mentioned before, that didn't help.

> Also, looking at:
> 
>   https://clang.llvm.org/docs/AddressSanitizer.html
> 
> ... it seems like there are some disabled-by-default checks that we
> could enable, like "check_initialization_order=1" and
> "detect_stack_use_after_return=1".

Agreed.  I have no experience with those, if the extra runtime tradeoff
is not too bad, then why not.

Thanks,
- Tom

Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while running tests

[Simon Marchi]

On 2021-11-03 06:24, Tom de Vries via Gdb-patches wrote:
> On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
>> We see some additional failures when running the testsuite against a GDB
>> compiled with ASan, compared to a GDB compiled without ASan.  Some of
>> them are caused by the memory leak report shown by the GDB process when
>> it exits, and the fact that it makes it exit with a non-zero exit code.
>>
>> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
>> environment when running the tests, but I don't always do it.  I think
>> it would be nice if the testsuite did it.  I don't see any use to have
>> leak detection when running the tests.  That is, unless we ever have a
>> test that ensures GDB doesn't leak memory, which isn't going to happen
>> any time soon.
>>
> 
> I like the idea.  I also use the setting in my test scripts.
> 
> FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
> triggered at some point.  I've just done a run without this setting, and
> it didn't trigger anything.
> 
> I tried to understand why, and stumbled onto
> https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
> which seems to have been approved, but never committed.  The problem
> described there, using LD_PRELOAD like so:
> ...
> $ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
> ...
> does reproduce for me, and applying the patch fixes it.  I've done a
> build and test run, and will commit shortly.
> 
> Also there a few test-cases which fail when using asan, we could
> annotate those perhaps with abort_on_error=1 or some such.

Starting with this commit, I get these 4 errors with clang 12, in a
non-ASan build.  The 2 -Wimplicit-exception-spec-mismatch can be fixed
by adding "noexcept", but I don't know about the -Wmissing-prototypes
ones.  It might indicate that the prototype of the functions isn't
right, but I can't find the problem.

Note that with Clang, I build with -std=gnu++17 in CXXFLAGS to avoid
the problem with gnulib headers in the string_view selftests.


    $ clang++  -DHAVE_CONFIG_H -I. -I/home/simark/src/binutils-gdb/gdbsupport  -I/home/simark/src/binutils-gdb/gdbsupport/../include -I/home/simark/src/binutils-gdb/gdbsupport/../gdb -I../gnulib/import -I/home/simark/src/binutils-gdb/gdbsupport/../gnulib/import -I.. -I/home/simark/src/binutils-gdb/gdbsupport/..  -I../bfd -I/home/simark/src/binutils-gdb/gdbsupport/../bfd   -Wall -Wpointer-arith -Wno-unused -Wunused-value -Wunused-function -Wno-switch -Wno-char-subscripts -Wempty-body -Wno-sign-compare -Wno-mismatched-tags -Wno-error=deprecated-register -Wsuggest-override -Wdeprecated-copy -Wdeprecated-copy-dtor -Wredundant-move -Wmissing-declarations -Wmissing-prototypes -Wformat -Wformat-nonliteral -Werror -std=gnu++17 -g3 -O0 -D_GLIBCXX_DEBUG=1   -MT new-op.o -MD -MP -MF .deps/new-op.Tpo -c -o new-op.o /home/simark/src/binutils-gdb/gdbsupport/new-op.cc
    
    /home/simark/src/binutils-gdb/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/bin/../lib64/gcc/x86_64-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/new:130:6: note: previous declaration is here
    void operator delete(void*) _GLIBCXX_USE_NOEXCEPT
         ^
    /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:114:1: error: no previous prototype for function 'operator delete' [-Werror,-Wmissing-prototypes]
    operator delete (void *p, std::size_t) noexcept
    ^
    /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:113:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    void
    ^
    static 
    /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:120: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/bin/../lib64/gcc/x86_64-pc-linux-gnu/11.1.0/../../../../include/c++/11.1.0/new:132:6: note: previous declaration is here
    void operator delete[](void*) _GLIBCXX_USE_NOEXCEPT
         ^
    /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:132:1: error: no previous prototype for function 'operator delete[]' [-Werror,-Wmissing-prototypes]
    operator delete[] (void *p, std::size_t) noexcept
    ^
    /home/simark/src/binutils-gdb/gdbsupport/new-op.cc:131:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
    void
    ^
    static 
    4 errors generated.

Simon

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

[Tom de Vries]

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

[ was: Re: [PATCH] gdb/testsuite: set ASAN_OPTIONS=detect_leaks=0 while
running tests ]

On 11/4/21 9:04 PM, Simon Marchi wrote:
> On 2021-11-03 06:24, Tom de Vries via Gdb-patches wrote:
>> On 11/2/21 9:34 PM, Simon Marchi via Gdb-patches wrote:
>>> We see some additional failures when running the testsuite against a GDB
>>> compiled with ASan, compared to a GDB compiled without ASan.  Some of
>>> them are caused by the memory leak report shown by the GDB process when
>>> it exits, and the fact that it makes it exit with a non-zero exit code.
>>>
>>> I generally try to remember to set ASAN_OPTIONS=detect_leaks=0 in my
>>> environment when running the tests, but I don't always do it.  I think
>>> it would be nice if the testsuite did it.  I don't see any use to have
>>> leak detection when running the tests.  That is, unless we ever have a
>>> test that ensures GDB doesn't leak memory, which isn't going to happen
>>> any time soon.
>>>
>>
>> I like the idea.  I also use the setting in my test scripts.
>>
>> FWIW, in addition, I also use "alloc_dealloc_mismatch=0", that error was
>> triggered at some point.  I've just done a run without this setting, and
>> it didn't trigger anything.
>>
>> I tried to understand why, and stumbled onto
>> https://sourceware.org/pipermail/gdb-patches/2021-May/178413.html ,
>> which seems to have been approved, but never committed.  The problem
>> described there, using LD_PRELOAD like so:
>> ...
>> $ LD_PRELOAD=/usr/lib64/libasan.so.6 ./gdb
>> ...
>> does reproduce for me, and applying the patch fixes it.  I've done a
>> build and test run, and will commit shortly.
>>
>> Also there a few test-cases which fail when using asan, we could
>> annotate those perhaps with abort_on_error=1 or some such.
> 
> Starting with this commit, I get these 4 errors with clang 12, in a
> non-ASan build.  The 2 -Wimplicit-exception-spec-mismatch can be fixed
> by adding "noexcept", but I don't know about the -Wmissing-prototypes
> ones.  It might indicate that the prototype of the functions isn't
> right, but I can't find the problem.
> 
> Note that with Clang, I build with -std=gnu++17 in CXXFLAGS to avoid
> the problem with gnulib headers in the string_view selftests.
> 

Thanks for mentioning this, that saved me some time :)

Reproduced, with clang 13.

Fixed by patch below, WDYT?

Thanks,
- Tom


[-- Attachment #2: 0001-gdb-build-Fix-Wimplicit-exception-spec-mismatch-in-clang-build.patch --]
[-- Type: text/x-patch, Size: 2038 bytes --]

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

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.

Also fix a few Wmissing-prototypes warnings by adding the missing prototype.

Build on x86_64-linux, using gcc 7.5.0 and clang 13.0.0.

---
 gdbsupport/new-op.cc | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdbsupport/new-op.cc b/gdbsupport/new-op.cc
index 2f4c71457b1..ac7bc146bf3 100644
--- a/gdbsupport/new-op.cc
+++ b/gdbsupport/new-op.cc
@@ -99,7 +99,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);
 }
@@ -110,14 +110,19 @@ operator delete (void *p, const std::nothrow_t&) noexcept
   return ::operator delete (p);
 }
 
+extern void
+operator delete (void *p, std::size_t) noexcept;
+
 void
 operator delete (void *p, std::size_t) noexcept
 {
   return ::operator delete (p, std::nothrow);
 }
 
+extern void operator delete[] (void *p) noexcept;
+
 void
-operator delete[] (void *p)
+operator delete[] (void *p) noexcept
 {
   return ::operator delete (p);
 }
@@ -128,6 +133,8 @@ operator delete[] (void *p, const std::nothrow_t&) noexcept
   return ::operator delete (p, std::nothrow);
 }
 
+extern void operator delete[] (void *p, std::size_t) noexcept;
+
 void
 operator delete[] (void *p, std::size_t) noexcept
 {

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

[Simon Marchi]

> Thanks for mentioning this, that saved me some time :)
>
> Reproduced, with clang 13.
>
> Fixed by patch below, WDYT?
>
> Thanks,
> - Tom

It does fix the build, but I would like if somebody who knows well about
this could tell why these two delete operators don't have a built-in
declaration, like the others.  Because it looks a bit fishy like this.

Jan, would you know, or know someone who knows?

Simon

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

[Pedro Alves]

On 2021-11-05 14:24, Simon Marchi via Gdb-patches wrote:
>> Thanks for mentioning this, that saved me some time :)
>>
>> Reproduced, with clang 13.
>>
>> Fixed by patch below, WDYT?
>>
>> Thanks,
>> - Tom
> 
> It does fix the build, but I would like if somebody who knows well about
> this could tell why these two delete operators don't have a built-in
> declaration, like the others.  Because it looks a bit fishy like this.
> 
> Jan, would you know, or know someone who knows?

The declarations for the ones that do have one aren't built-in, as in declared
by the compiler itself, they're declared in <new>.

The ones that the compiler is saying are missing a declaration are:

  operator delete (void *p, std::size_t) noexcept
  operator delete[] (void *p, std::size_t) noexcept

If you look at <new>, you'll see they are there, but guarded by __cpp_sized_deallocation:

#if __cpp_sized_deallocation
void operator delete(void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
  __attribute__((__externally_visible__));
void operator delete[](void*, std::size_t) _GLIBCXX_USE_NOEXCEPT
  __attribute__((__externally_visible__));
#endif

__cpp_sized_deallocation is only defined if you're compiling with C++14 (or later),
since Sized deallocation is a C++14 feature.

Clang defaults to C++11.

https://en.cppreference.com/w/cpp/feature_test
https://en.cppreference.com/w/cpp/memory/new/operator_delete

Pedro Alves

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

[Pedro Alves]

On 2021-11-05 10:03, Tom de Vries via Gdb-patches wrote:
>  
> +extern void
> +operator delete (void *p, std::size_t) noexcept;

The convention is to not format declarations with function name at column 0.  So here:

extern void operator delete (void *p, std::size_t) noexcept;

> +
>  void
>  operator delete (void *p, std::size_t) noexcept
>  {
>    return ::operator delete (p, std::nothrow);
>  }
>  
> +extern void operator delete[] (void *p) noexcept;
> +

... like you did here.

I suggest a comment mentioning the need to declare these if compiling in C++11.

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

[Tom de Vries]

On 11/5/21 6:07 PM, Pedro Alves wrote:
> On 2021-11-05 10:03, Tom de Vries via Gdb-patches wrote:
>>  
>> +extern void
>> +operator delete (void *p, std::size_t) noexcept;
> 
> The convention is to not format declarations with function name at column 0.  So here:
> 
> extern void operator delete (void *p, std::size_t) noexcept;
> 

Done.

>> +
>>  void
>>  operator delete (void *p, std::size_t) noexcept
>>  {
>>    return ::operator delete (p, std::nothrow);
>>  }
>>  
>> +extern void operator delete[] (void *p) noexcept;
>> +
> 
> ... like you did here.
> 
> I suggest a comment mentioning the need to declare these if compiling in C++11.
> 

Done, and split off into separate patch, refiled at
https://sourceware.org/pipermail/gdb-patches/2021-November/183311.html .

I'll commit in a few days unless there are further comments.

Thanks,
- Tom