[PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Simon Marchi]

When building with clang 18, I see:

      CXX    aarch64-linux-tdep.o
    /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
     1299 |       gdb_byte za_zeroed[za_bytes];
          |                          ^~~~~~~~
    /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
    /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
     1282 |   size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
          |          ^

Since we are using VLAs right now, that warning doesn't make sense for
us.  add `-Wno-vla-cxx-extension` to the list of warning flags we try to
enable.  If we ever choose to disallow VLAs, we can remove that flag.

Change-Id: Ie41feafc50c343f6e75333d4f836ce32fbeb6d8c
---
 gdb/configure         | 1 +
 gdbserver/configure   | 1 +
 gdbsupport/configure  | 1 +
 gdbsupport/warning.m4 | 1 +
 4 files changed, 4 insertions(+)

diff --git a/gdb/configure b/gdb/configure
index a77e3e273328..c3d5cf7ed63d 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -31179,6 +31179,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wredundant-move \
 -Wmissing-declarations \
 -Wstrict-null-sentinel \
+-Wno-vla-cxx-extension \
 "
 
 # The -Wmissing-prototypes flag will be accepted by GCC, but results
diff --git a/gdbserver/configure b/gdbserver/configure
index b85db9cd49df..026d250cc732 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -13706,6 +13706,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wredundant-move \
 -Wmissing-declarations \
 -Wstrict-null-sentinel \
+-Wno-vla-cxx-extension \
 "
 
 # The -Wmissing-prototypes flag will be accepted by GCC, but results
diff --git a/gdbsupport/configure b/gdbsupport/configure
index b45f12de45be..ae991250ce4c 100755
--- a/gdbsupport/configure
+++ b/gdbsupport/configure
@@ -14179,6 +14179,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wredundant-move \
 -Wmissing-declarations \
 -Wstrict-null-sentinel \
+-Wno-vla-cxx-extension \
 "
 
 # The -Wmissing-prototypes flag will be accepted by GCC, but results
diff --git a/gdbsupport/warning.m4 b/gdbsupport/warning.m4
index bdac8b3e7d22..d12bccbd3fb9 100644
--- a/gdbsupport/warning.m4
+++ b/gdbsupport/warning.m4
@@ -52,6 +52,7 @@ build_warnings="-Wall -Wpointer-arith \
 -Wredundant-move \
 -Wmissing-declarations \
 -Wstrict-null-sentinel \
+-Wno-vla-cxx-extension \
 "
 
 # The -Wmissing-prototypes flag will be accepted by GCC, but results

base-commit: ec48903170926f3827144525b50ddd3c6ae3fbf0
-- 
2.44.0

Re: [PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> When building with clang 18, I see:
Simon>       CXX    aarch64-linux-tdep.o
Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
Simon>      1299 |       gdb_byte za_zeroed[za_bytes];
Simon>           |                          ^~~~~~~~
Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
Simon>      1282 |   size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
Simon>           |          ^

Simon> Since we are using VLAs right now, that warning doesn't make sense for
Simon> us.  add `-Wno-vla-cxx-extension` to the list of warning flags we try to
Simon> enable.  If we ever choose to disallow VLAs, we can remove that flag.

I wonder if we should instead remove the use of VLAs.
Is it just the one spot?

Tom

Re: [PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Luis Machado]

On 4/15/24 17:22, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> When building with clang 18, I see:
> Simon>       CXX    aarch64-linux-tdep.o
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
> Simon>      1299 |       gdb_byte za_zeroed[za_bytes];
> Simon>           |                          ^~~~~~~~
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
> Simon>      1282 |   size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
> Simon>           |          ^
> 
> Simon> Since we are using VLAs right now, that warning doesn't make sense for
> Simon> us.  add `-Wno-vla-cxx-extension` to the list of warning flags we try to
> Simon> enable.  If we ever choose to disallow VLAs, we can remove that flag.
> 
> I wonder if we should instead remove the use of VLAs.
> Is it just the one spot?

If we want to go that route, we can modify that code to not use it. I think it was mostly convenience rather than a need for it.

Re: [PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Simon Marchi]

On 4/15/24 12:22 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> When building with clang 18, I see:
> Simon>       CXX    aarch64-linux-tdep.o
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
> Simon>      1299 |       gdb_byte za_zeroed[za_bytes];
> Simon>           |                          ^~~~~~~~
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1299:26: note: read of non-const variable 'za_bytes' is not allowed in a constant expression
> Simon>     /home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1282:10: note: declared here
> Simon>      1282 |   size_t za_bytes = std::pow (sve_vl_from_vg (svg), 2);
> Simon>           |          ^
> 
> Simon> Since we are using VLAs right now, that warning doesn't make sense for
> Simon> us.  add `-Wno-vla-cxx-extension` to the list of warning flags we try to
> Simon> enable.  If we ever choose to disallow VLAs, we can remove that flag.
> 
> I wonder if we should instead remove the use of VLAs.
> Is it just the one spot?

Here's the list of those I see in my build (x86-64 Linux all targets):

/home/smarchi/src/binutils-gdb/gdb/aarch64-linux-tdep.c:1298:26: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/aarch64-tdep.c:1729:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/aarch64-tdep.c:2545:17: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/aarch64-tdep.c:2659:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/aarch64-tdep.c:3306:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/aarch64-tdep.c:5718:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/amd64-linux-nat.c:238:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/amd64-linux-nat.c:303:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/arc-linux-tdep.c:203:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/fbsd-tdep.c:2035:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/loongarch-tdep.c:38:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/loongarch-tdep.c:1308:19: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/mips-linux-tdep.c:101:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/riscv-tdep.c:1017:24: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/solib-darwin.c:241:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/solib-darwin.c:335:20: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]
/home/smarchi/src/binutils-gdb/gdb/trad-frame.c:156:16: warning: variable length arrays in C++ are a Clang extension [-Wvla-cxx-extension]

I think we should have a vector type with "small vector optimization",
it would be great to replace these uses of VLAs.  I might give it a shot
at some point (or maybe someone else wants to try?).  But until then, I
think it makes sense to silence the warning.

Simon

Re: [PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Tom Tromey]

Simon> Here's the list of those I see in my build (x86-64 Linux all targets):

Simon> I think we should have a vector type with "small vector optimization",
Simon> it would be great to replace these uses of VLAs.  I might give it a shot
Simon> at some point (or maybe someone else wants to try?).  But until then, I
Simon> think it makes sense to silence the warning.

I suppose I'm ok with that.

On the whole I'd prefer we remove VLAs (since they aren't standard) and
alloca (since it is bad).  I don't really care if we have a small vector
unless it's proven we need it.

Tom

Re: [PATCH] gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension

[Simon Marchi]

On 2024-04-16 18:56, Tom Tromey wrote:
> Simon> Here's the list of those I see in my build (x86-64 Linux all targets):
> 
> Simon> I think we should have a vector type with "small vector optimization",
> Simon> it would be great to replace these uses of VLAs.  I might give it a shot
> Simon> at some point (or maybe someone else wants to try?).  But until then, I
> Simon> think it makes sense to silence the warning.
> 
> I suppose I'm ok with that.

Ok, pushed.  Whoever takes on the project of removing VLAs can remove
the flag.

> On the whole I'd prefer we remove VLAs (since they aren't standard) and
> alloca (since it is bad).  I don't really care if we have a small vector
> unless it's proven we need it.

Agreed.

Simon