martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1071-1072
bool ShouldFreeOnFail) const {
+ // HACK: CallDescription currently recognizes non-standard realloc functions
+ // as standard because it doesn't check the type, or wether its a non-method
+ // function. This should be solved by making CallDescription smarter.
----------------
Indeed. CallDescrtiption could be improved to do precise type checking. Also it
could be matching FunctionDecls instead of names of functions (strings), we see
how error prone this can be.
I think, the mechanisms in StdCLibraryFunctionChecker could be integrated into
CallDescription, so all other checkers could benefit.
See also the discussion we had on this with @xazax.hun :
>>! In D80016#2063978, @martong wrote:
>>>! In D80016#2063234, @xazax.hun wrote:
>> A high level comment.
>>
>> Trying to match function signatures is not a unique problem! In fact, almost
>> all of the checks the analyzer have is trying to solve the very some problem.
>> One of the methods we have at this point is called CallDescription, see
>> here:
>> https://github.com/llvm/llvm-project/blob/master/clang/lib/StaticAnalyzer/Core/CallEvent.cpp#L358
>>
>> Moreover, I would assume something similar needs to be done for APINotes.
>>
>> Do you think it would be possible to extend the CallDescription interface to
>> match your needs? In that case all of the checks could profit from this work.
>> What do you think?
>
> Yes, viewing from this angle, I am trying to solve a problem here that
> perhaps could be handled by CallDescription and CallDescriptionMap with some
> extensions.
>
> Here are the differences between the two approaches so far:
> - CallDescription is evaluated for every call event (e.g. checkPreCall), the
> names - strings - are compared; containing declaration contexts are compared
> by names (see CallEvent::isCalled). Contrary to this, summaries are
> associated directly to FunctionDecls, so during a call event we compare two
> FunctionDecl pointers.
> - A CallDescription does not support overloading on different types if the
> number of parameters are the same. With summaries this works.
> - A CallDescriptionMap is a static map with fixed number of entries. Contrary
> to this, the FunctionSummaryMap contains an entry (a summary) for a function
> only if we can lookup a matching FunctionDecl in the TU. The initialization
> of the FunctionSummaryMap happens lazily during the first call event.
>
> Except the lack of proper overloading support, these differences are in the
> implementation. So, yes, giving it more thought, maybe we could refactor
> CallDescriptionMap to behave this way, but that would be some very heavy
> refactoring :) Still, would be possible, I guess.
>
>> Moreover, I would assume something similar needs to be done for APINotes.
> Yes, I agree, I could not find out (yet) how the do it, but somehow they must
> match a given FunctionDecl to the Name in the YAML.
>
> -----------------------------
> I was thinking about an alternative method for a long time now.
> Making one step backwards: what we need is to be able to associate some data
> to a given function. And we can perfectly identify the function with it's
> prototype written in C/C++.
> So, what if we'd be able to write a CallDescriptionMap (or a
> FunctionSummaryMap) like this:
> ```
> CallDescriptionMap<FnCheck> Callbacks = {
> {{"void *memcpy(void *dest, const void *src, size_t n)"},
> &CStringChecker::evalMemcpy},
> ...
> ```
> Actually, we could reuse the Parser and the Sema itself to solve this issue.
> In fact, we could achieve this by reusing the ExternalASTSource together with
> the ASTImporter to find precisely the existing redecl chain in the TU for
> memcpy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81745/new/
https://reviews.llvm.org/D81745
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
