[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #25 from Martin Uecker --- I agree that they are not idiomatic C++ and that there exist good reasons why a programmer may want to avoid them (including standards compliance). But code not being idiomatic is not a terrible good reason for having a warning. As a matter of principle, we should not warn about our own extensions without a very good reason with -std=gnu modes and neither should clang IMHO. But the idea that VLAs are inherently very dangerous is incorrect, so let's not perpetuate that myth. There are many useful things a compiler could do to improve security for VLAs and also for std::vector or elsewhere by having better static analysis and more efficient options for bounds checking. Neither clang nor GCC will currently give any compile-time warning about a problem here with -Wall -Wextra nor will there be a run-error with UBSan: https://godbolt.org/z/7vhGMn3E5 And yes, -D_GLIBXX_DEBUG which will detect the out-of-bounds access but not the memset. Maybe -D_FORTIFY_SOURCE=3 will do this (as it does for VLAs), but it does not seem to work on godbolt for both cases, so I can't check. Asan will catch both. For comparison, with VLAs we have this: https://godbolt.org/z/hGxGrc569
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #24 from Jonathan Wakely --- (In reply to Martin Uecker from comment #22) > There may be many good reasons to prefer std::vector over VLAs in C++ but > security and safety is not one of them. There are plenty of CVEs caused by > std::vector out-of-bounds accesses. There are plenty of CVEs caused by those for arrays too, static and variable length ones. The point is that vector carries its length with it properly, in a way that actually works with the type system (e.g. works with std::span and std::end etc.) A VLA has a length that the compiler knows in a limited scope, but you can't pass that to a function without passing the length explicitly as a separate argument. The length information is easily lost. > The question is whether in GNU mode one > should warn about a GNU extension. People who want to avoid VLAs for reasons > of standard compliance would also not use a GNU mode. Yes, I know, and the lack of integration with the type system should show they are simply inappropriate for general purpose use in idiomatic C++ code.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848
--- Comment #23 from Jonathan Wakely ---
(In reply to Martin Uecker from comment #20)
> And what alternative do you think is fundamentally safer than VLAs?
>
> VLAs know their bound. Thus, they integrate with _FORTIFY_SOURCE, and UBSan
> bounds checking. Also UBSan address checking at run-time. At compile-time
> there is -Ws
They don't integrate with idiomatic C++ such as ranges algorithms, and
std::end.
More generally, they simply don't integrate with the C++ type system, so are
unusable with most generic code using templates. Not only does std::is_array
not recognise them as arrays, but even attempting to ask the question with
std::is_array is ill-formed. Variably modified types are not part of the C++
type system, so can't be template arguments.
int n = 2;
int a[n]{};
static_assert(not std::is_array_v); // error
Clang doesn't even allow the {} initializer in the code above, so they're not
portable either, even among compilers that support -std=gnu++17 modes.
> std::vector has some protection, e.g. ASAN finds the out of bounds
> access (at a high run-time cost) and one could activate the GLIBC assertions
> someho:
>
> https://godbolt.org/z/8zanMG5x4
This will abort with the recommended hardening flags, specifically
-D_GLIBCXX_ASSERTIONS (which is nothing to do with Glibc, and is suitable for
production use, unlike ASan). Those assertions will be enabled by the proposed
-fhardening flag.
>
> But I would not call it safer and overhead is much worse.
>
> Fundamentally, VLAs are the dynamic buffer which can be protected most
> easily and should be *preferred*.
Maybe for C, but not for C++.
I know you are a big fan of VLAs, but please don't try to push them into a
language where they do not fit, and are not needed.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #22 from Martin Uecker --- There may be many good reasons to prefer std::vector over VLAs in C++ but security and safety is not one of them. There are plenty of CVEs caused by std::vector out-of-bounds accesses. The question is whether in GNU mode one should warn about a GNU extension. People who want to avoid VLAs for reasons of standard compliance would also not use a GNU mode.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #21 from Jonathan Wakely --- std::vector should be preferred in C++. This warning will not drive responsible C++ developers to use alloca, it will drive them to use std::vector. As Aaron has said, there are cases where people use a VLA in C++ by mistake without even realising it, just because the code compiles and they don't notice they're using a non-constant bound.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #20 from Martin Uecker --- And what alternative do you think is fundamentally safer than VLAs? VLAs know their bound. Thus, they integrate with _FORTIFY_SOURCE, and UBSan bounds checking. Also UBSan address checking at run-time. At compile-time there is -Wstringop-overflow -fanalyzer and -Walloc-larger-than. Then also stack clash protection against stack overflow and stack protection against overflow on the stack (which is a second-line defense after bounds checking, which failed here in a very specific case to protect alloca and VLAs. This is - of course - by itself not a vulnerability.) https://godbolt.org/z/PfcWP55or alloca is clearly worse. Fixed size arrays on the stack which a worst-case bounds are also worse in most cases. std::vector has some protection, e.g. ASAN finds the out of bounds access (at a high run-time cost) and one could activate the GLIBC assertions someho: https://godbolt.org/z/8zanMG5x4 But I would not call it safer and overhead is much worse. Fundamentally, VLAs are the dynamic buffer which can be protected most easily and should be *preferred*.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #19 from Aaron Ballman --- (In reply to Andrew Pinski from comment #18) > (In reply to Aaron Ballman from comment #17) > > In the time I opened this request, a new CVE related to VLAs came out: > > https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-4039 > > Everything is a security risk. Seriously it is. Everything can and will be > abused; does not mean it is always right to warn about it. Also > -fstack-protector should never be a CVE. CVEs will get to the point where > they will be ignored because how they are now pointing out non-security > issues. My point is that this was a case where the developer used the language feature and tried to do what they could to protect against security issues and still ran into the security issue which resulted in a CVE. That's pretty different from "everything can be abused". (I wasn't suggesting there's an issue with using -fstack-protector or that it's a security issue itself)
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #18 from Andrew Pinski --- (In reply to Aaron Ballman from comment #17) > In the time I opened this request, a new CVE related to VLAs came out: > https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-4039 Everything is a security risk. Seriously it is. Everything can and will be abused; does not mean it is always right to warn about it. Also -fstack-protector should never be a CVE. CVEs will get to the point where they will be ignored because how they are now pointing out non-security issues.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #17 from Aaron Ballman --- (In reply to Martin Uecker from comment #16) > I do not think -Wall should warn about GNU extensions when used with > -std=gnu++XX in C++ and I think it is annoying that clang does it now. It > only drives people to use alloca or other alternatives with worse safety > properties. > > And I think the security concerns for VLAs are largely based on a logical > fallacy: Because they appear in CVE is no reason to believe they caused it: > It is likely saying that people ICDs have more often cardiac arrests if > because of the ICDs. Any kind of dynamically sized buffer will appear in > CVEs because buffers are used to process data from the network. If you > discourage the one with the best potential for bounds checking people will > turn to worse options. This will not improve safety. > > But stack clash protection should become the default. In the time I opened this request, a new CVE related to VLAs came out: https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2023-4039 Stack protection should become the default and it should certainly help mitigate issues, but VLAs are still a valid security concern IMO. So yes, this is intended to drive people to use alternatives (not necessarily `alloca`, which would be a strange choice of replacement for VLAs in C++ in 2023).
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #16 from Martin Uecker --- I do not think -Wall should warn about GNU extensions when used with -std=gnu++XX in C++ and I think it is annoying that clang does it now. It only drives people to use alloca or other alternatives with worse safety properties. And I think the security concerns for VLAs are largely based on a logical fallacy: Because they appear in CVE is no reason to believe they caused it: It is likely saying that people ICDs have more often cardiac arrests if because of the ICDs. Any kind of dynamically sized buffer will appear in CVEs because buffers are used to process data from the network. If you discourage the one with the best potential for bounds checking people will turn to worse options. This will not improve safety. But stack clash protection should become the default.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #15 from Eric Gallager --- (In reply to Eric Gallager from comment #14) > Oh right, if we're considering changing things for plain-C here, too, then > maybe have it enabled by -Wc++-compat there? Or rather, for plain-C modes, where "XY" is 99 or newer: -std=cXY: enabled by -Wc++-compat -std=gnuXY: still completely opt-in; -Wvla must be enabled explicitly (and then for the pre-c99 modes, well, I guess they'd just keep their current behavior)
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #14 from Eric Gallager --- Oh right, if we're considering changing things for plain-C here, too, then maybe have it enabled by -Wc++-compat there?
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #13 from Aaron Ballman --- Just to circle back around on this topic, these are changes recently landed in Clang: https://github.com/llvm/llvm-project/commit/84a3aadf0f2483dde0acfc4e79f2a075a5f35bd1 It enables the -Wvla-extension diagnostic group in C++ language modes by default, adds the warning group to -Wall in GNU++ language modes, and the warning is still opt-in in C language modes.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #12 from Aaron Ballman --- (In reply to Eric Gallager from comment #11) > How about: > > -std=c++XY: enabled by default (as per the proposal) > -std=gnu++XY: enabled by -Wall and/or -Wextra (in addition to being enabled > by -pedantic like it already is) That's a good suggestion -- I'd be quite happy with adding it to -Wall (or barring that, -Wextra) in GNU++ modes.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 Eric Gallager changed: What|Removed |Added CC||egallager at gcc dot gnu.org --- Comment #11 from Eric Gallager --- How about: -std=c++XY: enabled by default (as per the proposal) -std=gnu++XY: enabled by -Wall and/or -Wextra (in addition to being enabled by -pedantic like it already is)
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #10 from Aaron Ballman --- (In reply to Jonathan Wakely from comment #9) > Personally I'd like it diagnosed in GNU modes, but this is pretty much the > poster child for "conforming extension diagnosed with -pedantic" so I can > see the argument for not diagnosing it by default. Agreed -- I also see the argument for not diagnosing it by default. Normally I wouldn't suggest such a thing and I would not consider this to be a precedent to be applied to other extensions if it does get warned on by default. (I think the only other extension that might m be in the same category would be nested functions in C, but I've not spent nearly enough time researching that one to be sure.)
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #9 from Jonathan Wakely --- Personally I'd like it diagnosed in GNU modes, but this is pretty much the poster child for "conforming extension diagnosed with -pedantic" so I can see the argument for not diagnosing it by default.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 --- Comment #8 from Aaron Ballman --- (In reply to Richard Biener from comment #7) > I think -std=c++XY should diagnose (at least with a warning) the use of GNU > extensions. Let me alter the summary and confirm. Thanks! I still think this should be diagnosed in all language modes due to the ease of accidental usage along with the feature's security concerns, but at least getting it diagnosed by default in C++ language modes is a step in the right direction. Some more evidence of the security concerns (VLAs in general, not specific to C++): https://nvd.nist.gov/vuln/detail/CVE-2015-5147 https://nvd.nist.gov/vuln/detail/CVE-2020-11203 https://nvd.nist.gov/vuln/detail/CVE-2021-3527 That said, it sounds like GCC maintainers feel (at least somewhat) strongly that this extension should not be diagnosed by default in GNU mode. I think Clang can follow suit so that there's less problems for folks porting between the two compilers. But we've recently started being more aggressive about diagnosing things that have security implications in C and C++ because of warnings to not use these languages due to poor security practices and lack of coverage with tooling: https://advocacy.consumerreports.org/wp-content/uploads/2023/01/Memory-Safety-Convening-Report-1-1.pdf https://media.defense.gov/2022/Nov/10/2003112742/-1/-1/0/CSI_SOFTWARE_MEMORY_SAFETY.PDF I think VLA usage in C++ meets the bar as something to be more aggressive with warning users about. It's not that the extension is broken, it's that it's very often a surprise you're using the extension in the first place. It's unfortunate to have to opt out of diagnostics about an extension you're intentionally using; IMO, it's more unfortunate to have a CVE for your product due to accidentally using an extension you weren't aware of.
[Bug c++/110848] Consider enabling -Wvla by default in non-GNU C++ modes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110848 Richard Biener changed: What|Removed |Added CC||jason at gcc dot gnu.org Ever confirmed|0 |1 Status|UNCONFIRMED |NEW Last reconfirmed||2023-07-31 Summary|Consider enabling -Wvla by |Consider enabling -Wvla by |default in C++ modes|default in non-GNU C++ ||modes --- Comment #7 from Richard Biener --- I think -std=c++XY should diagnose (at least with a warning) the use of GNU extensions. Let me alter the summary and confirm.
