Bug 31141 - gdbsupport fails to compile with clang due to missing C++ prototype for operator delete
Summary: gdbsupport fails to compile with clang due to missing C++ prototype for opera...
Status: RESOLVED FIXED
Alias: None
Product: gdb
Classification: Unclassified
Component: build (show other bugs)
Version: HEAD
: P2 normal
Target Milestone: 15.1
Assignee: Tom Tromey
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-11 16:12 UTC by Jens Remus
Modified: 2023-12-22 16:42 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2023-12-11 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jens Remus 2023-12-11 16:12:02 UTC
gdbsupport fails to compile with clang on s390x and x86 since commit 553b78748fd ("Rely on C++17 <new> in new-op.cc") as follows:

$ make
make  all-recursive
make[1]: Entering directory '/home/jremus/my-dev-stuff/binutils-build/gdbsupport'
make[2]: Entering directory '/home/jremus/my-dev-stuff/binutils-build/gdbsupport'
  CXX      new-op.o
../../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
^
../../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
../../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
^
../../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
2 errors generated.
make[2]: *** [Makefile:554: new-op.o] Error 1
make[2]: Leaving directory '/home/jremus/my-dev-stuff/binutils-build/gdbsupport'
make[1]: *** [Makefile:574: all-recursive] Error 1
make[1]: Leaving directory '/home/jremus/my-dev-stuff/binutils-build/gdbsupport'
make: *** [Makefile:446: all] Error 2


It works again when reverting commit 553b78748fd ("Rely on C++17 <new> in new-op.cc").


My approach to build with clang are as follows:

cd $HOME
git clone git://sourceware.org/git/binutils-gdb.git
mkdir binutils-build
cd binutils-build
CC=clang CXX=clang++ ../binutils-gdb/configure
make


My above test build on x86 was on Fedora 38 with clang 16.0.6 and gcc C++ system header files from package libstdc++-devel 13.2.1.
Comment 1 Tom Tromey 2023-12-11 17:00:18 UTC
Seems like a clang bug but we can revert the patch.
Comment 3 Jens Remus 2023-12-11 17:33:30 UTC
Thanks, Tom! Peeking at /usr/include/c++/13/new it looks like both subject operator delete instances are 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

Maybe that helps identify what would need to be changed in the build to make it work with clang?
Comment 4 Jens Remus 2023-12-12 16:02:35 UTC
Hello Tom, I can confirm that your referenced patch resolves the gdb build issue with clang on both s390 and x86. Thanks!

Googling for "clang __cpp_sized_deallocation" I found that clang requires the command-line option -fsized-deallocation to support C++ sized deallocation:
https://bcain-llvm.readthedocs.io/projects/clang/en/release_37/ReleaseNotes/#new-compiler-flags
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fsized-deallocation

Further details:
https://reviews.llvm.org/D8467

Ongoing effort to enable C++ sized deallocation by default:
https://reviews.llvm.org/D112921

Besides reverting your changes there could possibly be two alternative approaches:

1. Add -fsized-deallocation to the C++ flags (see https://github.com/tomhughes/valgrind/blob/75a6b5764a264f2f2897adef40ecb4827b0bfa97/configure.ac#L2697)

2. Guard the two subject functions with __cpp_sized_deallocation in gdbsupport/new-op.cc, provided they are currently unused.
Comment 5 Tom Tromey 2023-12-12 20:32:23 UTC
Reopening until the patch lands.

-fsized-deallocation sounds like more of a pain than just
reverting the patch, IMO.
Comment 6 Sourceware Commits 2023-12-22 16:40:56 UTC
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9b9e5c09b1879766af959d276e99780982f4350b

commit 9b9e5c09b1879766af959d276e99780982f4350b
Author: Tom Tromey <tromey@adacore.com>
Date:   Mon Dec 11 10:04:23 2023 -0700

    Fix build with clang 16
    
    clang 16 reports a missing declaration in new-op.cc.  We believed
    these operators to be declared starting with C++14, but apparently
    that is not the case.
    
    This patch reverts the earlier change and then updates the comment to
    reflect the current state.
    
    Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=31141
Comment 7 Tom Tromey 2023-12-22 16:42:04 UTC
Fixed now.