New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build: enable -Wvla #4038
base: master
Are you sure you want to change the base?
build: enable -Wvla #4038
Conversation
Enable -Wvla by default, if supported by the compiler, to warn about Variable Length Arrays (VLA). Misuse of VLAs can result in a stack overflow. VLA can sometimes simplify things, so allow its use only if explicitly marked through a new v_vla_(type, name, count) macro. Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>
Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>
Signed-off-by: Asad Sajjad Ahmed <asadsa@varnish-software.com>
f30fe6d
to
4e1aa44
Compare
I am 👎🏽 because the suggested macro significantly decreases readability and I do not see any value in introducing a boilerplate which will just be applied wholesale. We need to think about VLAs and know what we are doing, and I do not see how this proposal makes this any easier or safer. |
Not sure if this is a good idea, but we could maybe have something like #define ASSERT_VLA(x) assert(sizeof x < 1<<14)) reasoning: outside pcre, we should "always" have 32k stack free, so we could maybe trigger early to safeguard against stack overflow attacks. |
To be fair, this will trigger on future variable-length arrays and force us to think twice when we introduce some. Also, this very pull request would actually be the occasion for us to review the legitimacy of existing instances, if anything. |
Just for completeness, the macro ended up looking like this piece of code: https://github.com/jemalloc/jemalloc/blob/dev/include/jemalloc/internal/jemalloc_internal_types.h#L126-L142 I did not bother to add support for pre C99, maybe this should be done as well? |
|
Enable -Wvla by default, if supported by the compiler, to warn about Variable Length Arrays (VLA). Misuse of VLAs can result in a stack overflow.
VLA can sometimes simplify things, so allow its use only if explicitly marked through a new v_vla_(type, name, count) macro.