[PATCH] D139604: [PATCH] Github Issue: Create a check that warns about using %p printf specifier #43453

[Balázs Benics via Phabricator via cfe-commits]

steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

I appreciate that you are interested in tackling this. I do think it's a low-hanging fruit, so it's a good choice!

However, there are a couple of things we need to improve before we could proceed:

1. Add tests demonstrating all possible aspects of your change.
2. Generally, one should check preconditions in the `Pre` checker callbacks, and apply postconditions (effects) in `Post` checker callbacks.
3. We should have a specific `BugType` pointer for the specific bug you hunt - in other words you should not reuse `BT_mallocZero` for your checker.
4. One should be able to disable your rule in case something goes wild. So, your checker should be registered at the end of the file by applying the  `REGISTER_CHECKER()` macro.
5. One should update the documentation and also mention the new check in the release docs to give visibility.
6. For matching if a function is interesting or not, you should use `CallDescription`s instead of looking at the `IdentifierInfo` directly - unless there are other cases in the scope where we don't use this facility. However, for those case we should consider updating those to use `CallDescriptionSet|Map`.
7. I can see that you directly use the `isZeroConstant()` on the value of the pointer. This might not work for all cases, e.g. when the pointer is symbolic. In that case, we might be able to prove that the pointer should be null, because the path-constraints we collected so far imply that.

Thanks for improving the detection!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139604/new/

https://reviews.llvm.org/D139604