The checks for the 'z' and 't' format specifiers added in the original
PR #143653 had some issues and were overly strict, causing some build
failures and were consequently reverted at
4c85bf2fe8.
In the latest commit
27c58629ec,
I relaxed the checks for the 'z' and 't' format specifiers, so warnings
are now only issued when they are used with mismatched types.
The original intent of these checks was to diagnose code that assumes
the underlying type of `size_t` is `unsigned` or `unsigned long`, for
example:
```c
printf("%zu", 1ul); // Not portable, but not an error when size_t is unsigned long
```
However, it produced a significant number of false positives. This was
partly because Clang does not treat the `typedef` `size_t` and
`__size_t` as having a common "sugar" type, and partly because a large
amount of existing code either assumes `unsigned` (or `unsigned long`)
is `size_t`, or they define the equivalent of size_t in their own way
(such as
sanitizer_internal_defs.h).2e67dcfdcd/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h (L203)
Including the results of `sizeof`, `sizeof...`, `__datasizeof`,
`__alignof`, `_Alignof`, `alignof`, `_Countof`, `size_t` literals, and
signed `size_t` literals, the results of pointer-pointer subtraction and
checks for standard library functions (and their calls).
The goal is to enable clang and downstream tools such as clangd and
clang-tidy to provide more portable hints and diagnostics.
The previous discussion can be found at #136542.
This PR implements this feature by introducing a new subtype of `Type`
called `PredefinedSugarType`, which was considered appropriate in
discussions. I tried to keep `PredefinedSugarType` simple enough yet not
limited to `size_t` and `ptrdiff_t` so that it can be used for other
purposes. `PredefinedSugarType` wraps a canonical `Type` and provides a
name, conceptually similar to a compiler internal `TypedefType` but
without depending on a `TypedefDecl` or a source file.
Additionally, checks for the `z` and `t` format specifiers in format
strings for `scanf` and `printf` were added. It will precisely match
expressions using `typedef`s or built-in expressions.
The affected tests indicates that it works very well.
Several code require that `SizeType` is canonical, so I kept `SizeType`
to its canonical form.
The failed tests in CI are allowed to fail. See the
[comment](https://github.com/llvm/llvm-project/pull/135386#issuecomment-3049426611)
in another PR #135386.
Default value of checker option `ModelPOSIX` is changed to `true`.
Documentation is updated.
This is a re-apply of commit 7af4e8bcc354d2bd7e46ecf547172b1f19ddde3e
that was reverted because a test failure (this is fixed now).
Some stream functions were recently added to `StreamChecker` that were
not modeled by `StdCLibraryFunctionsChecker`. To ensure consistency
these functions are added to the other checker too.
Some of the related tests are re-organized.
Main reason for this change is that these checkers were implemented in the same class
but had different dependency ordering. (NonNullParamChecker should run before StdCLibraryFunctionArgs
to get more special warning about null arguments, but the apiModeling.StdCLibraryFunctions was a modeling
checker that should run before other non-modeling checkers. The modeling checker changes state in a way
that makes it impossible to detect a null argument by NonNullParamChecker.)
To make it more simple, the modeling part is removed as separate checker and can be only used if
checker StdCLibraryFunctions is turned on, that produces the warnings too. Modeling the functions
without bug detection (for invalid argument) is not possible. The modeling of standard functions
does not happen by default from this change on.
Reviewed By: Szelethus
Differential Revision: https://reviews.llvm.org/D151225
Function declarations are moved into common header that can be reused
to avoid repetitions in different test files.
Some small problems in the tests were found and fixed.
Reviewed By: steakhal
Differential Revision: https://reviews.llvm.org/D149158
Previously, we would take a declaration like void f(void) and print it
as void f(). That's correct in C++ as far as it goes, but is incorrect
in C because that converts the function from having a prototype to one
which does not.
This turns out to matter for some of our tests that use the pretty
printer where we'd like to get rid of the K&R prototypes from the test
but can't because the test is checking the pretty printed function
signature, as done with the ARCMT tests.
without prototypes. This patch converts the function signatures to have
a prototype for the situations where the test is not specific to K&R C
declarations. e.g.,
void func();
becomes
void func(void);
This is the ninth batch of tests being updated (there are a
significant number of other tests left to be updated).
The `getenv()` function might return `NULL` just like any other function.
However, in case of `getenv()` a state-split seems justified since the
programmer should expect the failure of this function.
`secure_getenv(const char *name)` behaves the same way but is not handled
right now.
Note that `std::getenv()` is also not handled.
Reviewed By: martong
Differential Revision: https://reviews.llvm.org/D111245
It is no longer needed to add summaries of 'getline' for different
possible underlying types of ssize_t. We can just simply lookup the
type.
Differential Revision: https://reviews.llvm.org/D88092
Summary:
In this patch I am trying to get rid of the `Irrelevant` types from the
signatures of the functions from the standard C library. For that I've
introduced `lookupType()` to be able to lookup arbitrary types in the global
scope. This makes it possible to define the signatures precisely.
Note 1) `fread`'s signature is now fixed to have the proper `FILE *restrict`
type when C99 is the language.
Note 2) There are still existing `Irrelevant` types, but they are all from
POSIX. I am planning to address those together with the missing POSIX functions
(in D79433).
Reviewers: xazax.hun, NoQ, Szelethus, balazske
Subscribers: whisperity, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, gamesh411, Charusso, steakhal, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D80016
Summary:
Currently we match the summary signature based on the arguments in the CallExpr.
There are a few problems with this approach.
1) Variadic arguments are handled badly. Consider the below code:
int foo(void *stream, const char *format, ...);
void test_arg_constraint_on_variadic_fun() {
foo(0, "%d%d", 1, 2); // CallExpr
}
Here the call expression holds 4 arguments, whereas the function declaration
has only 2 `ParmVarDecl`s. So there is no way to create a summary that
matches the call expression, because the discrepancy in the number of
arguments causes a mismatch.
2) The call expression does not handle the `restrict` type qualifier.
In C99, fwrite's signature is the following:
size_t fwrite(const void *restrict, size_t, size_t, FILE *restrict);
However, in a call expression, like below, the type of the argument does not
have the restrict qualifier.
void test_fread_fwrite(FILE *fp, int *buf) {
size_t x = fwrite(buf, sizeof(int), 10, fp);
}
This can result in an unmatches signature, so the summary is not applied.
The solution is to match the summary against the referened callee
`FunctionDecl` that we can query from the `CallExpr`.
Further patches will continue with additional refactoring where I am going to
do a lookup during the checker initialization and the signature match will
happen there. That way, we will not check the signature during every call,
rather we will compare only two `FunctionDecl` pointers.
Reviewers: NoQ, Szelethus, gamesh411, baloghadamsoftware
Subscribers: whisperity, xazax.hun, kristof.beyls, szepet, rnkovacs, a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, steakhal, danielkiss, ASDenysPetrov, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77410
Summary:
This check was causing a crash in a test case where the 0th argument was
uninitialized ('Assertion `T::isKind(*this)' at line SVals.h:104). This
was happening since the argument was actually undefined, but the castAs
assumes the value is DefinedOrUnknownSVal.
The fix appears to be simply to check for an undefined value and skip
the check allowing the uninitalized value checker to detect the error.
I included a test case that I verified to produce the negative case
prior to the fix, and passes with the fix.
Reviewers: martong, NoQ
Subscribers: xazax.hun, szepet, rnkovacs, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, Charusso, ASDenysPetrov, baloghadamsoftware, dkrupp, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D77012
Because all our languages are C-based, there's no reason to
enable this checker only on UNIX targets.
Patch by Donát Nagy!
Differential Revision: https://reviews.llvm.org/D52722
llvm-svn: 343632
Summary:
This patch removes the constraint dropping when taint tracking is disabled.
It also voids the crash reported in D28953 by treating a SymSymExpr with non pointer symbols as an opaque expression.
Updated the regressions and verifying the big projects now; I'll update here when they're done.
Based on the discussion on the mailing list and the patches by @ddcc.
Reviewers: george.karpenkov, NoQ, ddcc, baloghadamsoftware
Reviewed By: george.karpenkov
Subscribers: delcypher, llvm-commits, rnkovacs, xazax.hun, szepet, a.sidorin, ddcc
Differential Revision: https://reviews.llvm.org/D48650
llvm-svn: 337167
Because standard functions can be defined differently on different platforms,
this commit introduces a method for constructing summaries with multiple
variants, whichever matches better. It is also useful for supporting overloads.
Differential Revision: https://reviews.llvm.org/D25940
llvm-svn: 285852
This checker does not emit reports, however it influences the analysis
by providing complete summaries for, or otherwise improving modeling of,
various standard library functions.
This should reduce the number of infeasible paths explored during analysis.
The custom function summary format used in this checker is superior to
body farms by causing less unnecessary state splits,
which would result in better analysis performance.
Differential Revision: https://reviews.llvm.org/D20811
llvm-svn: 284960
