Skip to content
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

-Wstringop-overread not implemented in Clang #72455

Open
ldionne opened this issue Nov 15, 2023 · 2 comments
Open

-Wstringop-overread not implemented in Clang #72455

ldionne opened this issue Nov 15, 2023 · 2 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature

Comments

@ldionne
Copy link
Member

ldionne commented Nov 15, 2023

GCC warns on the following code:

#include <cstring>

void f(char* dst){
    std::memcpy(dst, "abc", 5);
}
<source>: In function 'void f(char*)':
<source>:4:16: warning: 'void* memcpy(void*, const void*, size_t)' reading 5 bytes from a region of size 4 [-Wstringop-overread]
    4 |     std::memcpy(dst, "abc", 5);
      |     ~~~~~~~~~~~^~~~~~~~~~~~~~~
Compiler returned: 0

Clang doesn't warn. I tried with -Wall -Wextra. It seems like it should be fairly easy to warn on this with Clang, perhaps we should implement that diagnostic?

https://godbolt.org/z/qP736q5je

@ldionne ldionne added clang Clang issues not falling into any other category clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer labels Nov 15, 2023
@EugeneZelenko EugeneZelenko removed the clang Clang issues not falling into any other category label Nov 15, 2023
@shafik
Copy link
Collaborator

shafik commented Nov 16, 2023

Confirmed.

CC @AaronBallman

@shafik shafik added the confirmed Verified by a second party label Nov 16, 2023
@AaronBallman AaronBallman added enhancement Improving things as opposed to bug fixing, e.g. new or missing feature and removed confirmed Verified by a second party labels Nov 16, 2023
@AaronBallman
Copy link
Collaborator

GCC implements a whole lot of those diagnostics in their middle/back-end and they're based on optimizer behavior, so we're not likely to pick up all of GCC's diagnostic behavior if we did implement this one. That said, so long as we're going through the builtin function, it seems reasonable to catch static cases like this when we can. Anything more complicated than static cases should be handled by the static analyzer though because they'll require dataflow analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer enhancement Improving things as opposed to bug fixing, e.g. new or missing feature
Projects
None yet
Development

No branches or pull requests

4 participants