[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

Some code [0] consider that trailing arrays are flexible, whatever their size.
Support for these legacy code has b

[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

Some code [0] consider that trailing arrays are flexible, whatever their size.
Support for these legacy code has been introduced in
f8f632498307d22e10fab0704548b270b15f1e1e but it prevents evaluation of
__builtin_object_size and __builtin_dynamic_object_size in some legit cases.

Introduce -fstrict-flex-arrays=<n> to have stricter conformance when it is
desirable.

n = 0: current behavior, any trailing array member is a flexible array. The default.
n = 1: any trailing array member of undefined, 0 or 1 size is a flexible array member
n = 2: any trailing array member of undefined or 0 size is a flexible array member

This takes into account two specificities of clang: array bounds as macro id
disqualify FAM, as well as non standard layout.

Similar patch for gcc discuss here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

[0] https://docs.freebsd.org/en/books/developers-handbook/sockets/#sockets-essential-functions

show more ...

Revert "[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays"

This reverts D126864 and related fixes.

This reverts commit 572b08790a69f955ae0cbb1b4a7d4a215f15dad9.
Th

Revert "[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays"

This reverts D126864 and related fixes.

This reverts commit 572b08790a69f955ae0cbb1b4a7d4a215f15dad9.
This reverts commit 886715af962de2c92fac4bd37104450345711e4a.

show more ...

[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

Some code [0] consider that trailing arrays are flexible, whatever their size.
Support for these legacy code has b

[clang] Introduce -fstrict-flex-arrays=<n> for stricter handling of flexible arrays

Some code [0] consider that trailing arrays are flexible, whatever their size.
Support for these legacy code has been introduced in
f8f632498307d22e10fab0704548b270b15f1e1e but it prevents evaluation of
__builtin_object_size and __builtin_dynamic_object_size in some legit cases.

Introduce -fstrict-flex-arrays=<n> to have stricter conformance when it is
desirable.

n = 0: current behavior, any trailing array member is a flexible array. The default.
n = 1: any trailing array member of undefined, 0 or 1 size is a flexible array member
n = 2: any trailing array member of undefined or 0 size is a flexible array member
n = 3: any trailing array member of undefined size is a flexible array member (strict c99 conformance)

Similar patch for gcc discuss here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836

[0] https://docs.freebsd.org/en/books/developers-handbook/sockets/#sockets-essential-functions

show more ...

[analyzer] Relax constraints on const qualified regions

The arithmetic restriction seems to be artificial.
The comment below seems to be stale.
Thus, we remove both.

Depends on D127306.

Reviewed B

[analyzer] Relax constraints on const qualified regions

The arithmetic restriction seems to be artificial.
The comment below seems to be stale.
Thus, we remove both.

Depends on D127306.

Reviewed By: martong

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

show more ...

[analyzer] Treat system globals as mutable if they are not const

Previously, system globals were treated as immutable regions, unless it
was the `errno` which is known to be frequently modified.

D1

[analyzer] Treat system globals as mutable if they are not const

Previously, system globals were treated as immutable regions, unless it
was the `errno` which is known to be frequently modified.

D124244 wants to add a check for stores to immutable regions.
It would basically turn all stores to system globals into an error even
though we have no reason to believe that those mutable sys globals
should be treated as if they were immutable. And this leads to
false-positives if we apply D124244.

In this patch, I'm proposing to treat mutable sys globals actually
mutable, hence allocate them into the `GlobalSystemSpaceRegion`, UNLESS
they were declared as `const` (and a primitive arithmetic type), in
which case, we should use `GlobalImmutableSpaceRegion`.

In any other cases, I'm using the `GlobalInternalSpaceRegion`, which is
no different than the previous behavior.

---

In the tests I added, only the last `expected-warning` was different, compared to the baseline.
Which is this:
```lang=C++
void test_my_mutable_system_global_constraint() {
assert(my_mutable_system_global > 2);
clang_analyzer_eval(my_mutable_system_global > 2); // expected-warning {{TRUE}}
invalidate_globals();
clang_analyzer_eval(my_mutable_system_global > 2); // expected-warning {{UNKNOWN}} It was previously TRUE.
}
void test_my_mutable_system_global_assign(int x) {
my_mutable_system_global = x;
clang_analyzer_eval(my_mutable_system_global == x); // expected-warning {{TRUE}}
invalidate_globals();
clang_analyzer_eval(my_mutable_system_global == x); // expected-warning {{UNKNOWN}} It was previously TRUE.
}
```

---

Unfortunately, the taint checker will be also affected.
The `stdin` global variable is a pointer, which is assumed to be a taint
source, and the rest of the taint propagation rules will propagate from
it.
However, since mutable variables are no longer treated immutable, they
also get invalidated, when an opaque function call happens, such as the
first `scanf(stdin, ...)`. This would effectively remove taint from the
pointer, consequently disable all the rest of the taint propagations
down the line from the `stdin` variable.

All that said, I decided to look through `DerivedSymbol`s as well, to
acquire the memregion in that case as well. This should preserve the
previously existing taint reports.

Reviewed By: martong

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

show more ...

[analyzer][NFCi] Annotate major nonnull returning functions

This patch annotates the most important analyzer function APIs.
Also adds a couple of assertions for uncovering any potential issues
earli

[analyzer][NFCi] Annotate major nonnull returning functions

This patch annotates the most important analyzer function APIs.
Also adds a couple of assertions for uncovering any potential issues
earlier in the constructor; in those cases, the member functions were
already dereferencing the members unconditionally anyway.

Measurements showed no performance impact, nor crashes.

Reviewed By: martong

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

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 ...

[C2x] Disallow functions without prototypes/functions with identifier lists

WG14 has elected to remove support for K&R C functions in C2x. The
feature was introduced into C89 already deprecated, so

[C2x] Disallow functions without prototypes/functions with identifier lists

WG14 has elected to remove support for K&R C functions in C2x. The
feature was introduced into C89 already deprecated, so after this long
of a deprecation period, the committee has made an empty parameter list
mean the same thing in C as it means in C++: the function accepts no
arguments exactly as if the function were written with (void) as the
parameter list.

This patch implements WG14 N2841 No function declarators without
prototypes (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2841.htm)
and WG14 N2432 Remove support for function definitions with identifier
lists (http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2432.pdf).

It also adds The -fno-knr-functions command line option to opt into
this behavior in other language modes.

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

show more ...

[clang][analyzer] Add modeling of 'errno'.

Add a checker to maintain the system-defined value 'errno'.
The value is supposed to be set in the future by existing or
new checkers that evaluate errno-m

[clang][analyzer] Add modeling of 'errno'.

Add a checker to maintain the system-defined value 'errno'.
The value is supposed to be set in the future by existing or
new checkers that evaluate errno-modifying function calls.

Reviewed By: NoQ, steakhal

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

show more ...

Revert "[clang][analyzer] Add modeling of 'errno'."

This reverts commit 29b512ba322cb6dd2c45d5e07645e20db47fad0d.

This broke several build bots:

https://lab.llvm.org/buildbot/#/builders/86/builds/

Revert "[clang][analyzer] Add modeling of 'errno'."

This reverts commit 29b512ba322cb6dd2c45d5e07645e20db47fad0d.

This broke several build bots:

https://lab.llvm.org/buildbot/#/builders/86/builds/30183
https://lab.llvm.org/buildbot/#/builders/216/builds/488

show more ...

[clang][analyzer] Add modeling of 'errno'.

Add a checker to maintain the system-defined value 'errno'.
The value is supposed to be set in the future by existing or
new checkers that evaluate errno-m

[clang][analyzer] Add modeling of 'errno'.

Add a checker to maintain the system-defined value 'errno'.
The value is supposed to be set in the future by existing or
new checkers that evaluate errno-modifying function calls.

Reviewed By: NoQ, steakhal

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

show more ...

[NFC][analyzer] Return underlying strings directly instead of OS.str()

This avoids an unnecessary copy required by 'return OS.str()', allowing
instead for NRVO or implicit move. The .str() call (whi

[NFC][analyzer] Return underlying strings directly instead of OS.str()

This avoids an unnecessary copy required by 'return OS.str()', allowing
instead for NRVO or implicit move. The .str() call (which flushes the
stream) is no longer required since 65b13610a5226b84889b923bae884ba395ad084d,
which made raw_string_ostream unbuffered by default.

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

show more ...

[clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

Replace variable and functions names, as well as comments that contain whitelist with
more inclusive terms.

Revie

[clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

Replace variable and functions names, as well as comments that contain whitelist with
more inclusive terms.

Reviewed By: aaron.ballman, martong

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

show more ...

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) ||

[analyzer][NFC] Refactor llvm::isa<> usages in the StaticAnalyzer

It turns out llvm::isa<> is variadic, and we could have used this at a
lot of places.

The following patterns:
x && isa<T1>(x) || isa<T2>(x) ...
Will be replaced by:
isa_and_non_null<T1, T2, ...>(x)

Sometimes it caused further simplifications, when it would cause even
more code smell.

Aside from this, keep in mind that within `assert()` or any macro
functions, we need to wrap the isa<> expression within a parenthesis,
due to the parsing of the comma.

Reviewed By: martong

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

show more ...

[Sema, StaticAnalyzer] Use StringRef::contains (NFC)

[APInt] Stop using soft-deprecated constructors and methods in clang. NFC.

Stop using APInt constructors and methods that were soft-deprecated in
D109483. This fixes all the uses I found in clang.

[APInt] Stop using soft-deprecated constructors and methods in clang. NFC.

Stop using APInt constructors and methods that were soft-deprecated in
D109483. This fixes all the uses I found in clang.

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

show more ...

[APInt] Normalize naming on keep constructors / predicate methods.

This renames the primary methods for creating a zero value to `getZero`
instead of `getNullValue` and renames predicates like `isAl

[APInt] Normalize naming on keep constructors / predicate methods.

This renames the primary methods for creating a zero value to `getZero`
instead of `getNullValue` and renames predicates like `isAllOnesValue`
to simply `isAllOnes`. This achieves two things:

1) This starts standardizing predicates across the LLVM codebase,
following (in this case) ConstantInt. The word "Value" doesn't
convey anything of merit, and is missing in some of the other things.

2) Calling an integer "null" doesn't make any sense. The original sin
here is mine and I've regretted it for years. This moves us to calling
it "zero" instead, which is correct!

APInt is widely used and I don't think anyone is keen to take massive source
breakage on anything so core, at least not all in one go. As such, this
doesn't actually delete any entrypoints, it "soft deprecates" them with a
comment.

Included in this patch are changes to a bunch of the codebase, but there are
more. We should normalize SelectionDAG and other APIs as well, which would
make the API change more mechanical.

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

show more ...

[analyzer] SValBuilder should have an easy access to AnalyzerOptions

`SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()`
is quite a mouthful and might involve a few p

[analyzer] SValBuilder should have an easy access to AnalyzerOptions

`SVB.getStateManager().getOwningEngine().getAnalysisManager().getAnalyzerOptions()`
is quite a mouthful and might involve a few pointer indirections to get
such a simple thing like an analyzer option.

This patch introduces an `AnalyzerOptions` reference to the `SValBuilder`
abstract class, while refactors a few cases to use this /simpler/ accessor.

Reviewed By: martong, Szelethus

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

show more ...

[analyzer] Ignore single element arrays in getStaticSize() conditionally

Quoting https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html:
> In the absence of the zero-length array extension, in ISO C90

[analyzer] Ignore single element arrays in getStaticSize() conditionally

Quoting https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html:
> In the absence of the zero-length array extension, in ISO C90 the contents
> array in the example above would typically be declared to have a single
> element.

We should not assume that the size of the //flexible array member// field has
a single element, because in some cases they use it as a fallback for not
having the //zero-length array// language extension.
In this case, the analyzer should return `Unknown` as the extent of the field
instead.

Reviewed By: martong

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

show more ...

Revert "Revert "[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs""

This reverts commit df1f4e0cc6ec9a734aae41ffd48ee8b2007fcabb.

Now the test case explicitly specifies the target

Revert "Revert "[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs""

This reverts commit df1f4e0cc6ec9a734aae41ffd48ee8b2007fcabb.

Now the test case explicitly specifies the target triple.
I decided to use x86_64 for that matter, to have a fixed
bitwidth for `size_t`.

Aside from that, relanding the original changes of:
https://reviews.llvm.org/D105184

show more ...

Revert "[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs"

This reverts commit 360ced3b8fd2cfb9f2a26deb739e6c381e98b9a5.

[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs

Currently only `ConstantArrayType` is considered for flexible array
members (FAMs) in `getStaticSize()`.
However, `IncompleteArrayT

[analyzer] Ignore IncompleteArrayTypes in getStaticSize() for FAMs

Currently only `ConstantArrayType` is considered for flexible array
members (FAMs) in `getStaticSize()`.
However, `IncompleteArrayType` also shows up in practice as FAMs.

This patch will ignore the `IncompleteArrayType` and return Unknown
for that case as well. This way it will be at least consistent with
the current behavior until we start modeling them accurately.

I'm expecting that this will resolve a bunch of false-positives
internally, caused by the `ArrayBoundV2`.

Reviewed By: ASDenysPetrov

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

show more ...

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

[analyzer] DynamicSize: Store the dynamic size

This patch introduces a way to store the size.

Reviewed By: NoQ

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

[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 ...