[PATCH] D79423: [analyzer][NFC] StdLibraryFunctionsChecker: Add empty Signatures

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D79423#2022920 <https://reviews.llvm.org/D79423#2022920>, @xazax.hun wrote:

> In D79423#2022812 <https://reviews.llvm.org/D79423#2022812>, @martong wrote:
>
> > I don't think that could be a concern.
> >  Actually, redefinition of a reserved name either in the C or in the C++ standard library is undefined behavior:
>
>
> I disagree. As you mentioned in another revision, we plan to model functions beyond the C and C++ standard library. We cannot prevent name collisions for those other libraries (and sometimes we cannot even prevent unintended name collisions with the standard libraries). 
>  I think to reduce the risk of applying the wrong summary to a function worth the effort of spelling the signature out (since it only needs to be done once).


I see your point Gabor. I agree that it would be the best if we could give a precise signatures to every summary. But we can't. 
There are two difficulties to provide proper signatures:

1. Some function types contain non-builtin types. E.g. `FILE*`. We cannot get this type as easily as we do with builtin types (we can get builtins simply from the ASTContext). In case of such a compound type, we should be digging up the type from the AST, and that can be done by doing a lookup. But instead of looking up the type of one parameter, it is better to do the lookup for the function itself. So, in these cases we rather use the `Irrelevant` type in the `Signature`. Signatures with irrelevant types are not precise, they may match accidentally other functions.
2. Cppcheck config files does not provide any signature, so it would require overly too much manual work to synthesize the signatures (that may not be precise anyway, see the above point).

However, not having a signature is not a problem in the case of LibC and POSIX. This is because LibC implementations do provide POSIX compatiblity, so they define the functions from POSIX (e.g. e.g. glibc <https://www.gnu.org/software/libc/manual/html_node/POSIX.html>). Besides they claim

> All other library names are reserved if your program explicitly includes the header file that defines or declares them.

In my understanding this includes the names defined by POSIX.
Consequently, user code should not redefine - or add any overload in case of C++-  to any functions defined in the standard C, C++ or POSIX library. This is also related to C++ <https://en.cppreference.com/w/cpp/language/extending_std#Addressable_functions>.

So, here is my suggestion.

- LibC, STL and POSIX: We should allow empty/irrelevant signatures for standardized functions. If the user code provides any colliding function then we emit an error.
- Other libraries: We should be more rigorous with these functions and we could require a signature for each function. If the user code provides any colliding function then we just emit a warning and the corresponding summary is not added and will not be used. One example is when the user enables summaries from OpenSSL where we have a summary for let's say `void encrypt(FILE*)`, and the Signature is `void(Irrelevant)`. If the user code defines `encrypt(double)` that seems to be okay as it overloads with `encrypt(FILE*)` but our signature matching mechanism cannot cope with this, so we rather back out and keep the conservative evaluation for the function.

TLDR; having empty signatures makes it possible to easily add summaries for standard functions that we know they must have only one definition and they cannot be overloaded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79423