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

scanf format attribute type confusion with char/bool #64987

Closed
AaronBallman opened this issue Aug 25, 2023 · 4 comments
Closed

scanf format attribute type confusion with char/bool #64987

AaronBallman opened this issue Aug 25, 2023 · 4 comments
Assignees
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer quality-of-implementation

Comments

@AaronBallman
Copy link
Collaborator

The scanf format attribute currently issues no diagnostics when confusing a bool value and a char specifier, though GCC does diagnose. Consider:

void foo(void) {
    bool b;
    scanf("%hhi", &b);
}

https://godbolt.org/z/jrcKzPb9r

Clang emits no diagnostic in this case, while GCC emits format '%hhi' expects argument of type 'signed char *', but argument 2 has type '_Bool *'

Perhaps Clang should diagnose similar to GCC?

@AaronBallman AaronBallman added quality-of-implementation clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Aug 25, 2023
@AaronBallman
Copy link
Collaborator Author

Many thanks to @apple-fcloutier for spotting this issue during the review of https://reviews.llvm.org/D158318#4615285.

@apple-fcloutier
Copy link
Contributor

Also, I thought about it yesterday evening and I've convinced myself that it really shouldn't be allowed. This can easily store a value other than 0 or 1 in the bool's storage unit, which we've observed to lead to security issues through memory corruption not so long ago. (I don't think that it's a terribly high-priority thing to fix, but I wanted to communicate that I now think there's a clear answer to whether it should be done.)

@AaronBallman
Copy link
Collaborator Author

I agree, I think we should diagnose this by default.

@Fznamznon Fznamznon self-assigned this Aug 31, 2023
@Fznamznon
Copy link
Contributor

avillega pushed a commit to avillega/llvm-project that referenced this issue Sep 11, 2023
qihangkong pushed a commit to rvgpu/llvm that referenced this issue Apr 18, 2024
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 quality-of-implementation
Projects
None yet
Development

No branches or pull requests

3 participants