[clang] Respect the [[clang::unsafe_buffer_usage]] attribute for constructors (PR #91777)
Dana Jansens via cfe-commits
cfe-commits at lists.llvm.org
Tue May 14 13:51:25 PDT 2024
danakj wrote:
> > https://github.com/llvm/llvm-project/blob/2ff43ce87e66d9324370e35ea6743ef57400c76e/clang/lib/Analysis/UnsafeBufferUsage.cpp#L1373-L1374
> >
> > These assert that exactly one gadget matched. I think it's kinda worthwhile keeping the warnings independent too, so I don't see this as a bad thing. If we were to mark the span ctor as `[[clang::unsafe_buffer_usage]]` for instance, it the span-specific warning gives a better output still, and generating two warnings for the same piece of code is noisy.
>
> Hmm this isn't really what these assertions are about. They don't protect against duplicate gadgets, they protect against duplicate `.bind()` tags across gadgets. Because gadgets are connected by the `anyOf()` matcher (as opposed to `eachOf()`), a single match result will always have exactly one gadget, the first one in the list, even if they match the same statement.
>
> So I think you're right that there's a problem: if you have two gadgets match the same statement, only one will be reported. So your solution is probably necessary. We should also maybe do something about that, so that warning gadgets weren't conflicting this way. But I don't think _this_ assertion guards against that.
Ah yeah, okay, that makes sense. I did bump into the asserts while working on this, but not in the way that I thought. When I remove the `unless(HasTwoParamSpanCtorDecl)` I don't hit the asserts.
> If this still doesn't sound right to you, can you include a test where the assertion fails for you? No matter how ridiculous the test is. The machine is complex enough to appreciate ridiculous tests. I think we want to get to the bottom of this.
It sounds right to me yeah, especially after testing it.
https://github.com/llvm/llvm-project/pull/91777
More information about the cfe-commits
mailing list