https://github.com/haoNoQ commented:

> 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.

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.

https://github.com/llvm/llvm-project/pull/91777
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to