[clang][analyzer] Added partial wide character support to CStringChecker

Support for functions wmemcpy, wcslen, wcsnlen is added to the checker.
Documentation and tests are updated and extended with

[clang][analyzer] Added partial wide character support to CStringChecker

Support for functions wmemcpy, wcslen, wcsnlen is added to the checker.
Documentation and tests are updated and extended with the new functions.

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D130091

show more ...

[analyzer] Fix use of length in CStringChecker

CStringChecker is using getByteLength to get the length of a string
literal. For targets where a "char" is 8-bits, getByteLength() and
getLength() will

[analyzer] Fix use of length in CStringChecker

CStringChecker is using getByteLength to get the length of a string
literal. For targets where a "char" is 8-bits, getByteLength() and
getLength() will be equal for a C string, but for targets where a "char"
is 16-bits getByteLength() returns the size in octets.

This is verified in our downstream target, but we have no way to add a
test case for this case since there is no target supporting 16-bit
"char" upstream. Since this cannot have a test case, I'm asserted this
change is "correct by construction", and visually inspected to be
correct by way of the following example where this was found.

The case that shows this fails using a target with 16-bit chars is here.
getByteLength() for the string literal returns 4, which fails when
checked against "char x[4]". With the change, the string literal is
evaluated to a size of 2 which is a correct number of "char"'s for a
16-bit target.

```
void strcpy_no_overflow_2(char *y) {
char x[4];
strcpy(x, "12"); // with getByteLength(), returns 4 using 16-bit chars
}
```

This change exposed that embedded nulls within the string are not
handled. This is documented as a FIXME for a future fix.

```
void strcpy_no_overflow_3(char *y) {
char x[3];
strcpy(x, "12\0");
}

```

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D129269

show more ...

[analyzer][NFC] Prefer using isa<> instead getAs<> in conditions

Depends on D125709

Reviewed By: martong

Differential Revision: https://reviews.llvm.org/D127742

[analyzer] Replace adjacent assumeInBound calls to assumeInBoundDual

This is to minimize superfluous assume calls.

Depends on D124758

Differential Revision: https://reviews.llvm.org/D124761

[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up s

[analyzer] Clean checker options from bool to DefaultBool (NFC)

A recent review emphasized the preference to use DefaultBool instead of
bool for checker options. This change is a NFC and cleans up some of the
instances where bool was used, and could be changed to DefaultBool.

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D123464

show more ...

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostrea

[clang] Add a raw_ostream operator<< overload for QualType

Under the hood this prints the same as `QualType::getAsString()` but cuts out the middle-man when that string is sent to another raw_ostream.

Also cleaned up all the call sites where this occurs.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D123926

show more ...

[analyzer] Avoid checking addrspace pointers in cstring checker

This change fixes an assert that occurs in the SMT layer when refuting a
finding that uses pointers of two different sizes. This was f

[analyzer] Avoid checking addrspace pointers in cstring checker

This change fixes an assert that occurs in the SMT layer when refuting a
finding that uses pointers of two different sizes. This was found in a
downstream build that supports two different pointer sizes, The CString
Checker was attempting to compute an overlap for the 'to' and 'from'
pointers, where the pointers were of different sizes.

In the downstream case where this was found, a specialized memcpy
routine patterned after memcpy_special is used. The analyzer core hits
on this builtin because it matches the 'memcpy' portion of that builtin.
This cannot be duplicated in the upstream test since there are no
specialized builtins that match that pattern, but the case does
reproduce in the accompanying LIT test case. The amdgcn target was used
for this reproducer. See the documentation for AMDGPU address spaces here
https://llvm.org/docs/AMDGPUUsage.html#address-spaces.

The assert seen is:

`*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same sort!"'

Ack to steakhal for reviewing the fix, and creating the test case.

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D118050

show more ...

[analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from src , which is actual

[analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from src , which is actually a bug but the CSA seems to give up at that point . I was curious about that and I pinged @steakhal on the discord and according to him this seems to be a genuine issue and needs to be fix. So I goes with fixing this bug and thanks to @steakhal who help me creating this patch. This feature seems to break some tests but this was the genuine problem and the broken tests also needs to fix in certain manner. I add a test but yeah we need more tests,I'll try to add more tests.Thanks

Reviewed By: steakhal, NoQ

Differential Revision: https://reviews.llvm.org/D120489

show more ...

[analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from src , which is actual

[analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

Few weeks back I was experimenting with reading the uninitialized values from src , which is actually a bug but the CSA seems to give up at that point . I was curious about that and I pinged @steakhal on the discord and according to him this seems to be a genuine issue and needs to be fix. So I goes with fixing this bug and thanks to @steakhal who help me creating this patch. This feature seems to break some tests but this was the genuine problem and the broken tests also needs to fix in certain manner. I add a test but yeah we need more tests,I'll try to add more tests.Thanks

Reviewed By: steakhal, NoQ

Differential Revision: https://reviews.llvm.org/D120489

show more ...

[analyzer][NFCi] Use the correct BugType in CStringChecker.

There is different bug types for different types of bugs but the **emitAdditionOverflowbug** seems to use bugtype **BT_NotCSting** but ac

[analyzer][NFCi] Use the correct BugType in CStringChecker.

There is different bug types for different types of bugs but the **emitAdditionOverflowbug** seems to use bugtype **BT_NotCSting** but actually it have to use **BT_AdditionOverflow** .

Reviewed By: steakhal

Differential Revision: https://reviews.llvm.org/D119462

show more ...

[clang][NFC] Inclusive terms: replace some uses of sanity in clang

Rewording of comments to avoid using `sanity test, sanity check`.

Reviewed By: aaron.ballman, Quuxplusone

Differential Revision:

[clang][NFC] Inclusive terms: replace some uses of sanity in clang

Rewording of comments to avoid using `sanity test, sanity check`.

Reviewed By: aaron.ballman, Quuxplusone

Differential Revision: https://reviews.llvm.org/D114025

show more ...

[analyzer][NFC] Switch to using CallDescription::matches() instead of isCalled()

This patch replaces each use of the previous API with the new one.
In variadic cases, it will use the ADL `matchesAny

[analyzer][NFC] Switch to using CallDescription::matches() instead of isCalled()

This patch replaces each use of the previous API with the new one.
In variadic cases, it will use the ADL `matchesAny(Call, CDs...)`
variadic function.
Also simplifies some code involving such operations.

Reviewed By: martong, xazax.hun

Differential Revision: https://reviews.llvm.org/D113591

show more ...

[analyzer][NFC] Separate CallDescription from CallEvent

`CallDescriptions` deserve its own translation unit.
This patch simply moves the corresponding parts.
Also includes the `CallDescription.h` wh

[analyzer][NFC] Separate CallDescription from CallEvent

`CallDescriptions` deserve its own translation unit.
This patch simply moves the corresponding parts.
Also includes the `CallDescription.h` where it's necessary.

Reviewed By: martong, xazax.hun, Szelethus

Differential Revision: https://reviews.llvm.org/D113587

show more ...

[clang] Rename StringRef _lower() method calls to _insensitive()

This is mostly a mechanical change, but a testcase that contains
parts of the StringRef class (clang/test/Analysis/llvm-conventions.c

[clang] Rename StringRef _lower() method calls to _insensitive()

This is mostly a mechanical change, but a testcase that contains
parts of the StringRef class (clang/test/Analysis/llvm-conventions.cpp)
isn't touched.

show more ...

[analyzer] DynamicSize: Rename 'size' to 'extent'

[Analyzer] [NFC] Parameter Regions

Currently, parameters of functions without their definition present cannot
be represented as regions because it would be difficult to ensure that the
same declarat

[Analyzer] [NFC] Parameter Regions

Currently, parameters of functions without their definition present cannot
be represented as regions because it would be difficult to ensure that the
same declaration is used in every case. To overcome this, we split
`VarRegion` to two subclasses: `NonParamVarRegion` and `ParamVarRegion`.
The latter does not store the `Decl` of the parameter variable. Instead it
stores the index of the parameter which enables retrieving the actual
`Decl` every time using the function declaration of the stack frame. To
achieve this we also removed storing of `Decl` from `DeclRegion` and made
`getDecl()` pure virtual. The individual `Decl`s are stored in the
appropriate subclasses, such as `FieldRegion`, `ObjCIvarRegion` and the
newly introduced `NonParamVarRegion`.

Differential Revision: https://reviews.llvm.org/D80522

show more ...

[analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

Summary:
I wanted to extend the diagnostics of the CStringChecker with taintedness.
This requires the CStringChecker to be r

[analyzer] NFCi: Refactor CStringChecker: use strongly typed internal API

Summary:
I wanted to extend the diagnostics of the CStringChecker with taintedness.
This requires the CStringChecker to be refactored to support a more flexible
reporting mechanism.

This patch does only refactorings, such:
- eliminates always false parameters (like WarnAboutSize)
- reduces the number of parameters
- makes strong types differentiating *source* and *destination* buffers
(same with size expressions)
- binds the argument expression and the index, making diagnostics accurate
and easy to emit
- removes a bunch of default parameters to make it more readable
- remove random const char* warning message parameters, making clear where
and what is going to be emitted

Note that:
- CheckBufferAccess now checks *only* one buffer, this removed about 100 LOC
code duplication
- not every function was refactored to use the /new/ strongly typed API, since
the CString related functions are really closely coupled monolithic beasts,
I will refactor them separately
- all tests are preserved and passing; only the message changed at some places.
In my opinion, these messages are holding the same information.

I would also highlight that this refactoring caught a bug in
clang/test/Analysis/string.c:454 where the diagnostic did not reflect reality.
This catch backs my effort on simplifying this monolithic CStringChecker.

Reviewers: NoQ, baloghadamsoftware, Szelethus, rengolin, Charusso

Reviewed By: NoQ

Subscribers: whisperity, xazax.hun, szepet, rnkovacs, a.sidorin,
mikhail.ramalho, donat.nagy, dkrupp, Charusso, martong, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D74806

show more ...

Remove llvm::Error include form Diagnostic.h

Saves ~400 related LLVM ADT. llvm/ADT/Error.h takes 90ms to parse.

$ diff -u <(sort thedeps-before.txt) <(sort thedeps-after.txt) \
| grep '^[-+] '

Remove llvm::Error include form Diagnostic.h

Saves ~400 related LLVM ADT. llvm/ADT/Error.h takes 90ms to parse.

$ diff -u <(sort thedeps-before.txt) <(sort thedeps-after.txt) \
| grep '^[-+] ' | sort | uniq -c | sort -nr
403 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/Error.h
403 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm-c/Error.h
397 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/Format.h
397 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/Debug.h
377 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/ADT/StringExtras.h
158 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm-c/ExternC.h
138 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/ErrorOr.h
13 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/Support/raw_ostream.h
13 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/ADT/SmallString.h
5 - /usr/local/google/home/rnk/llvm-project/llvm/include/llvm/ADT/Twine.h

show more ...

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patc

[analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions

Some checkers may not only depend on language options but also analyzer options.
To make this possible this patch changes the parameter of the shouldRegister*
function to CheckerManager to be able to query the analyzer options when
deciding whether the checker should be registered.

Differential Revision: https://reviews.llvm.org/D75271

show more ...

[analyzer] DynamicSize: Remove 'getExtent()' from regions

Summary:
This patch introduces a placeholder for representing the dynamic size of
regions. It also moves the `getExtent()` method of `SubReg

[analyzer] DynamicSize: Remove 'getExtent()' from regions

Summary:
This patch introduces a placeholder for representing the dynamic size of
regions. It also moves the `getExtent()` method of `SubRegions` to the
`MemRegionManager` as `getStaticSize()`.

Reviewed By: NoQ

Differential Revision: https://reviews.llvm.org/D69540

show more ...

Fix "pointer is null" static analyzer warnings. NFCI.

Use cast<> instead of dyn_cast<> and move into its users where its dereferenced immediately.

[analyzer] CStringChecker: Fix a crash on unknown value passed to strlcat.

Checkers should always account for unknown values.

Also use a slightly more high-level API that naturally avoids the probl

[analyzer] CStringChecker: Fix a crash on unknown value passed to strlcat.

Checkers should always account for unknown values.

Also use a slightly more high-level API that naturally avoids the problem.

show more ...

[analyzer] CStringChecker: Fix overly eager assumption that memcmp args overlap.

While analyzing code `memcmp(a, NULL, n);', where `a' has an unconstrained
symbolic value, the analyzer was emitting

[analyzer] CStringChecker: Fix overly eager assumption that memcmp args overlap.

While analyzing code `memcmp(a, NULL, n);', where `a' has an unconstrained
symbolic value, the analyzer was emitting a warning about the *first* argument
being a null pointer, even though we'd rather have it warn about the *second*
argument.

This happens because CStringChecker first checks whether the two argument
buffers are in fact the same buffer, in order to take the fast path.
This boils down to assuming `a == NULL' to true. Then the subsequent check
for null pointer argument "discovers" that `a' is null.

Don't take the fast path unless we are *sure* that the buffers are the same.
Otherwise proceed as normal.

Differential Revision: https://reviews.llvm.org/D71322

show more ...

[analyzer] CStringChecker: Improve warning messages.

Differential Revision: https://reviews.llvm.org/D71321

[analyzer] PR41729: CStringChecker: Improve strlcat and strlcpy modeling.

- Fix false positive reports of strlcat.
- The return value of strlcat and strlcpy is now correctly calculated.
- The result

[analyzer] PR41729: CStringChecker: Improve strlcat and strlcpy modeling.

- Fix false positive reports of strlcat.
- The return value of strlcat and strlcpy is now correctly calculated.
- The resulting string length of strlcat and strlcpy is now correctly
calculated.

Patch by Daniel Krupp!

Differential Revision: https://reviews.llvm.org/D66049

show more ...