[analyzer][NFC] Migrate nonloc::ConcreteInt to use APSIntPtr (2/4) (#120436)

[analyzer] Note last "fclose" call from "ensureStreamOpened" (#109112)

Patch by Arseniy Zaostrovnykh!

[analyzer] Fix StreamChecker crash in fread modeling (#108393)

In #93408
https://github.com/llvm/llvm-project/commit/69bc159142c6e4ed168e32a6168392d396f891de
I refined how invalidation is done for

[analyzer] Fix StreamChecker crash in fread modeling (#108393)

In #93408
https://github.com/llvm/llvm-project/commit/69bc159142c6e4ed168e32a6168392d396f891de
I refined how invalidation is done for `fread`. It can crash, if the
"size" or "count" parameters of "fread" is a perfectly constrained
negative value. In such cases, when it will try to allocate a
SmallVector with a negative size, which will cause a crash.

To mitigate this issue, let's just guard against negative values.

CPP-3247

show more ...

[analyzer][NFC] Eliminate a dyn_cast (#100719)

Response to the catch in this comment:
https://github.com/llvm/llvm-project/pull/94357/files/07f6daf2cf0f5d5bd4fc9950f2585a3f52b4ad2f#r1692084074

[analyzer] Fix crash of StreamChecker when eval calling 'fopen' (#100990)

Actually, on the failure branch of `fopen`, the resulting pointer could
alias with `stdout` iff `stdout` is already known t

[analyzer] Fix crash of StreamChecker when eval calling 'fopen' (#100990)

Actually, on the failure branch of `fopen`, the resulting pointer could
alias with `stdout` iff `stdout` is already known to be null.
We crashed in this case as the implementation assumed that the
state-split for creating the success and failure branches both should be
viable; thus dereferenced both of those states - leading to the crash.

To fix this, let's just only add this no-alias property for the success
path, and that's it :)

Fixes #100901

show more ...

[analyzer] Assume the result of 'fopen' can't alias with 'std{in,out,err}' (#100085)

'fopen' should return a new FILE handle, thus we should assume it can't
alias with commonly used FILE handles, s

[analyzer] Assume the result of 'fopen' can't alias with 'std{in,out,err}' (#100085)

'fopen' should return a new FILE handle, thus we should assume it can't
alias with commonly used FILE handles, such as with 'stdin', 'stdout' or
'stderr'.

This problem appears in code that handles either some input/output file
with stdin or stdout, as the business logic is basically the same no
matter the stream being used.
However, one would should only close the stream if it was opened via
'fopen'. Consequently, such code usually has a condition like `if (f &&
f != stdout)` to guard the `fclose()` call.

This patch brings this assumption, thus eliminates FPs for not taking
the guarded branch.

CPP-5306

show more ...

[analyzer] Fix crash in Stream checker when using void pointers (#97199)

We can get zero type size (thus div by zero crash) if the region is for a 'void*' pointer.
In this patch, let's just overrid

[analyzer] Fix crash in Stream checker when using void pointers (#97199)

We can get zero type size (thus div by zero crash) if the region is for a 'void*' pointer.
In this patch, let's just override the void type with a char type to avoid the crash.

Fixes
https://github.com/llvm/llvm-project/pull/93408#issuecomment-2189766510

show more ...

[analyzer] Add an ownership change visitor to StreamChecker (#94957)

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visi

[analyzer] Add an ownership change visitor to StreamChecker (#94957)

This is very similar to https://reviews.llvm.org/D105553, in fact, I
barely made any changes from MallocChecker's ownership visitor to this
one.

The new visitor emits a diagnostic note for function where a change in
stream ownership was expected (for example, it had a fclose() call), but
the ownership remained unchanged. This is similar to messages regarding
ordinary values ("Returning without writing to x").

show more ...

StreamChecker.cpp: Use isa<> (for #93408) [-Wunused-but-set-variable]

[analyzer] Refine invalidation caused by `fread` (#93408)

This change enables more accurate modeling of the write effects of
`fread`. In particular, instead of invalidating the whole buffer, in a

[analyzer] Refine invalidation caused by `fread` (#93408)

This change enables more accurate modeling of the write effects of
`fread`. In particular, instead of invalidating the whole buffer, in a
best-effort basis, we would try to invalidate the actually accesses
elements of the buffer. This preserves the previous value of the buffer
of the unaffected slots. As a result, diagnose more uninitialized buffer
uses for example.

Currently, this refined invalidation only triggers for `fread` if and
only if the `count` parameter and the buffer pointer's index component
are concrete or perfectly-constrained symbols.
Additionally, if the `fread` would read more than 64 elements, the whole
buffer is invalidated as before. This is to have safeguards against
performance issues.

Refer to the comments of the assertions in the following example to see
the changes in the diagnostics:

```c++
void demo() {
FILE *fp = fopen("/home/test", "rb+");
if (!fp) return;
int buffer[10]; // uninitialized
int read_items = fread(buffer+1, sizeof(int), 5, fp);
if (5 == read_items) {
int v1 = buffer[1]; // Unknown value but not garbage.
clang_analyzer_isTainted(v1); // expected-warning {{YES}} <-- Would be "NO" without this patch.
clang_analyzer_dump(v1); // expected-warning {{conj_}} <-- Not a "derived" symbol, so it's directly invalidated now.
int v0 = buffer[0]; // expected-warning {{Assigned value is garbage or undefined}} <-- Had no report here before.
(void)(v1 + v0);
} else {
// If 'fread' had an error.
int v0 = buffer[0]; // expected-warning {{Assigned value is garbage or undefined}} <-- Had no report here before.
(void)v0;
}
fclose(fp);
}
```

CPP-3247, CPP-3802

Co-authored by Marco Borgeaud (marco-antognini-sonarsource)

show more ...

[clang] Use StringRef::operator== instead of StringRef::equals (NFC) (#91844)

I'm planning to remove StringRef::equals in favor of
StringRef::operator==.

- StringRef::operator==/!= outnumber Str

[clang] Use StringRef::operator== instead of StringRef::equals (NFC) (#91844)

I'm planning to remove StringRef::equals in favor of
StringRef::operator==.

- StringRef::operator==/!= outnumber StringRef::equals by a factor of
24 under clang/ in terms of their usage.

- The elimination of StringRef::equals brings StringRef closer to
std::string_view, which has operator== but not equals.

- S == "foo" is more readable than S.equals("foo"), especially for
!Long.Expression.equals("str") vs Long.Expression != "str".

show more ...

[analyzer] Use explicit call description mode in more checkers (#90974)

This commit explicitly specifies the matching mode (C library function,
any non-method function, or C++ method) for the `Call

[analyzer] Use explicit call description mode in more checkers (#90974)

This commit explicitly specifies the matching mode (C library function,
any non-method function, or C++ method) for the `CallDescription`s
constructed in various checkers.

Some code was simplified to use `CallDescriptionSet`s instead of
individual `CallDescription`s.

This change won't cause major functional changes, but isn't NFC because
it ensures that e.g. call descriptions for a non-method function won't
accidentally match a method that has the same name.

Separate commits have already performed this change in other checkers:
- easy cases: e2f1cbae45f81f3cd9a4d3c2bcf69a094eb060fa
- MallocChecker: d6d84b5d1448e4f2e24b467a0abcf42fe9d543e9
- iterator checkers: 06eedffe0d2782922e63cc25cb927f4acdaf7b30
- InvalidPtr checker: 024281d4d26344f9613b9115ea1fcbdbdba23235

... and follow-up commits will handle the remaining checkers.

My goal is to ensure that the call description mode is always explicitly
specified and eliminate (or strongly restrict) the vague "may be either
a method or a simple function" mode that's the current default.

show more ...

[NFC][clang][analyzer] Initialize pointer field in StreamOperationEvaluator (#89837)

Add an initializer for StreamSym, which is a pointer. The pointers in
this class are set in the Init function, b

[NFC][clang][analyzer] Initialize pointer field in StreamOperationEvaluator (#89837)

Add an initializer for StreamSym, which is a pointer. The pointers in
this class are set in the Init function, but all should be initialized
in the constructor to avoid confusion and static verifier hits.

show more ...

[analyzer] Use CDM::CLibrary instead of isGlobalCFunction() (#88267)

This commit updates several checkers to use call descriptions with the
matching mode `CDM::CLibrary` instead of checking
`Call.

[analyzer] Use CDM::CLibrary instead of isGlobalCFunction() (#88267)

This commit updates several checkers to use call descriptions with the
matching mode `CDM::CLibrary` instead of checking
`Call.isGlobalCFunction()` after performing the match. This resolves
several TODOs in various checkers.

Note that both matching with `CDM::CLibrary` and calling
`isGlobalCFunction` leads to `CheckerContext::isCLibraryFunction()`
checks (so this change is close to being NFC), but if it is used via the
matching mode then the checker can automatically recognize the builtin
variants of the matched functions.

I'll also make similar changes in GenericTaintChecker, but that checker
has separate and inconsistent rules for handling the normal and the
builtin variant of several functions (e.g. `memcpy` and
`__builtin_memcpy`), so I'll put those changes into a separate commit.

show more ...

[clang][analyzer] Add "pedantic" mode to StreamChecker. (#87322)

The checker may create failure branches for all stream write operations
only if the new option "pedantic" is set to true.
Result of

[clang][analyzer] Add "pedantic" mode to StreamChecker. (#87322)

The checker may create failure branches for all stream write operations
only if the new option "pedantic" is set to true.
Result of the write operations is often not checked in typical code. If
failure branches are created the checker will warn for unchecked write
operations and generate a lot of "false positives" (these are valid
warnings but the programmer does not care about this problem).

show more ...

[clang][analyzer] Change modeling of `fseek` in StreamChecker. (#86919)

Until now function `fseek` returned nonzero on error, this is changed to
-1 only. And it does not produce EOF error any more.

[clang][analyzer] Change modeling of `fseek` in StreamChecker. (#86919)

Until now function `fseek` returned nonzero on error, this is changed to
-1 only. And it does not produce EOF error any more.
This complies better with the POSIX standard.

show more ...

[clang][analyzer] Model getline/getdelim preconditions and evaluation (#83027)

According to POSIX 2018.

1. lineptr, n and stream can not be NULL.
2. If *n is non-zero, *lineptr must point to a regi

[clang][analyzer] Model getline/getdelim preconditions and evaluation (#83027)

According to POSIX 2018.

1. lineptr, n and stream can not be NULL.
2. If *n is non-zero, *lineptr must point to a region of at least *n
bytes, or be a NULL pointer.

Additionally, if *lineptr is not NULL, *n must not be undefined.

show more ...

[clang][analyzer] Fix StreamChecker `ftell` and `fgetpos` at indeterminate file position. (#84191)

These functions should not be allowed if the file position is
indeterminate (they return the file

[clang][analyzer] Fix StreamChecker `ftell` and `fgetpos` at indeterminate file position. (#84191)

These functions should not be allowed if the file position is
indeterminate (they return the file position).
This condition is now checked, and tests are improved to check it.

show more ...

Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf" (#83281)

`va_list` is a platform-specific type. On some, it is a struct instead
of a pointer to a struct, so `lookupFn

Reapply "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf" (#83281)

`va_list` is a platform-specific type. On some, it is a struct instead
of a pointer to a struct, so `lookupFn` was ignoring calls to `vfprintf`
and `vfscanf`.

`stream.c` now runs in four different platforms to make sure the logic
works across targets.

show more ...

[clang][analyzer] Add StreamChecker note tags for "indeterminate stream position". (#83288)

If a stream operation fails the position can become "indeterminate".
This may cause warning from the chec

[clang][analyzer] Add StreamChecker note tags for "indeterminate stream position". (#83288)

If a stream operation fails the position can become "indeterminate".
This may cause warning from the checker at a later operation. The new
note tag shows the place where the position becomes "indeterminate",
this is where a failure occurred.

show more ...

Revert "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)"

This reverts commit ffe7049b543adb9739261d28a60d4a47a00aa2e0.

This commit breaks on e.g. arm:
Example:
https:/

Revert "[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)"

This reverts commit ffe7049b543adb9739261d28a60d4a47a00aa2e0.

This commit breaks on e.g. arm:
Example:
https://lab.llvm.org/buildbot/#/builders/245/builds/21177/steps/5/logs/FAIL__Clang__stream_c

```
******************** TEST 'Clang :: Analysis/stream.c' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
RUN: at line 1: /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang -cc1 -internal-isystem /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/lib/clang/19/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c
+ /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/bin/clang -cc1 -internal-isystem /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/lib/clang/19/include -nostdsysteminc -analyze -analyzer-constraints=range -setup-static-analyzer -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c
error: 'expected-warning' diagnostics expected but not seen:
File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c Line 147: Stream pointer might be NULL
File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c Line 153: Stream pointer might be NULL
error: 'expected-warning' diagnostics seen but not expected:
File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c Line 148: Stream pointer might be NULL [alpha.unix.Stream]
File /home/tcwg-buildbot/worker/clang-armv8-quick/llvm/clang/test/Analysis/stream.c Line 154: Stream pointer might be NULL [alpha.unix.Stream]
4 errors generated.
--
********************
```

show more ...

[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)

Model `getc` and `putc` as equivalent to `fgetc` and `fputc` respectively.

Model `vfscanf` and `vfprintf` as `fscanf`

[clang][analyzer] StreamChecker: Model getc, vfscanf, putc, vfprintf (#82476)

Model `getc` and `putc` as equivalent to `fgetc` and `fputc` respectively.

Model `vfscanf` and `vfprintf` as `fscanf` and `fprintf`, except that
`vfscanf` can not invalidate the parameters due to the indirection via a
`va_list`. Nevertheless, we can still track EOF and errors as for `fscanf`.

show more ...

[clang][analyzer] Fix argument invalidations in StreamChecker. (#79470)

Specific arguments passed to stream handling functions are changed by
the function, this means these should be invalidated ("

[clang][analyzer] Fix argument invalidations in StreamChecker. (#79470)

Specific arguments passed to stream handling functions are changed by
the function, this means these should be invalidated ("escaped") by the
analyzer. This change adds the argument invalidation (in specific cases)
to the checker.

show more ...

[clang][analyzer] Change modeling of 'fileno' in checkers. (#81842)

Function 'fileno' fails only if invalid pointer is passed, this is a
case that is often ignored in source code. The failure case

[clang][analyzer] Change modeling of 'fileno' in checkers. (#81842)

Function 'fileno' fails only if invalid pointer is passed, this is a
case that is often ignored in source code. The failure case leads to
many "false positive" reports when `fileno` returns -1 and this is not
checked in the program. Because this, the function is now assumed
to not fail (this is assumption that the passed file pointer is correct).
The change affects `StdCLibraryFunctionsChecker` and
`StreamChecker`.

show more ...

[clang][analyzer] Simplify code of StreamChecker - part 2 (NFC). (#82228)

Continuation of commit 42b5037, apply changes to the remaining
functions.
Code for function `fflush` was not changed, beca

[clang][analyzer] Simplify code of StreamChecker - part 2 (NFC). (#82228)

Continuation of commit 42b5037, apply changes to the remaining
functions.
Code for function `fflush` was not changed, because it is more special
compared to the others.

show more ...