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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to