gdbsupport, gdbserver, gdb: use -Wno-vla-cxx-extension
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
Commit Message
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(+)
base-commit: ec48903170926f3827144525b50ddd3c6ae3fbf0
Comments
>>>>> "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
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.
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
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
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
@@ -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
@@ -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
@@ -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
@@ -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