Summary:
VisitReturnStmt would create a new block with including Dtors, so the Dtors created
in VisitCompoundStmts would be in an unreachable block.
Example:
struct S {
~S();
};
void f()
{
S s;
return;
}
void g()
{
S s;
}
Before this patch, f has one additional unreachable block containing just the
destructor of S. With this patch, both f and g have the same blocks.
Reviewers: krememek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D13973
llvm-svn: 253107
Function__builtin_signbit returns wrong value for type ppcf128 on big endian
machines. This patch fixes how value is generated in that case.
Patch by Aleksandar Beserminji.
Differential Revision: http://reviews.llvm.org/D14149
llvm-svn: 252307
This checker looks for unsafe constructs in vforked process:
function calls (excluding whitelist), memory write and returns.
This was originally motivated by a vfork-related bug in xtables package.
Patch by Yury Gribov.
Differential revision: http://reviews.llvm.org/D14014
llvm-svn: 252285
Update RegionStoreManager::getBinding() to return UnknownVal when trying to get
the binding for a BlockDataRegion. Previously, getBinding() would try to cast the
BlockDataRegion to a TypedValueRegion and crash. This happened when a block
was passed as a parameter to an inlined function for which
StackHintGeneratorForSymbol::getMessage() tried to generate a stack hint message.
rdar://problem/21291971
llvm-svn: 252185
This commit creates a new 'optin' top-level checker package and moves several of
the localizability checkers into it.
This package is for checkers that are not alpha and that would normally be on by
default but where the driver does not have enough information to determine when
they are applicable. The localizability checkers fit this criterion because the
driver cannot determine whether a project is localized or not -- this is best
determined at the IDE or build-system level.
This new package is *not* intended for checkers that are too noisy to be on by
default.
The hierarchy under 'optin' mirrors that in 'alpha': checkers under 'optin'
should be organized in the hierarchy they would have had if they were truly top
level (e.g., optin.osx.cocoa.MyOptInChecker).
Differential Revision: http://reviews.llvm.org/D14303
llvm-svn: 252080
Summary:
Dear All,
We have been looking at the following problem, where any code after the constant bound loop is not analyzed because of the limit on how many times the same block is visited, as described in bugzillas #7638 and #23438. This problem is of interest to us because we have identified significant bugs that the checkers are not locating. We have been discussing a solution involving ranges as a longer term project, but I would like to propose a patch to improve the current implementation.
Example issue:
```
for (int i = 0; i < 1000; ++i) {...something...}
int *p = 0;
*p = 0xDEADBEEF;
```
The proposal is to go through the first and last iterations of the loop. The patch creates an exploded node for the approximate last iteration of constant bound loops, before the max loop limit / block visit limit is reached. It does this by identifying the variable in the loop condition and finding the value which is “one away” from the loop being false. For example, if the condition is (x < 10), then an exploded node is created where the value of x is 9. Evaluating the loop body with x = 9 will then result in the analysis continuing after the loop, providing x is incremented.
The patch passes all the tests, with some modifications to coverage.c, in order to make the ‘function_which_gives_up’ continue to give up, since the changes allowed the analysis to progress past the loop.
This patch does introduce possible false positives, as a result of not knowing the state of variables which might be modified in the loop. I believe that, as a user, I would rather have false positives after loops than do no analysis at all. I understand this may not be the common opinion and am interested in hearing your views. There are also issues regarding break statements, which are not considered. A more advanced implementation of this approach might be able to consider other conditions in the loop, which would allow paths leading to breaks to be analyzed.
Lastly, I have performed a study on large code bases and I think there is little benefit in having “max-loop” default to 4 with the patch. For variable bound loops this tends to result in duplicated analysis after the loop, and it makes little difference to any constant bound loop which will do more than a few iterations. It might be beneficial to lower the default to 2, especially for the shallow analysis setting.
Please let me know your opinions on this approach to processing constant bound loops and the patch itself.
Regards,
Sean Eveson
SN Systems - Sony Computer Entertainment Group
Reviewers: jordan_rose, krememek, xazax.hun, zaks.anna, dcoughlin
Subscribers: krememek, xazax.hun, cfe-commits
Differential Revision: http://reviews.llvm.org/D12358
llvm-svn: 251621
The analyzer assumes that system functions will not free memory or modify the
arguments in other ways, so we assume that arguments do not escape when
those are called. However, this may lead to false positive leak errors. For
example, in code like this where the pointers added to the rb_tree are freed
later on:
struct alarm_event *e = calloc(1, sizeof(*e));
<snip>
rb_tree_insert_node(&alarm_tree, e);
Add a heuristic to assume that calls to system functions taking void*
arguments allow for pointer escape.
llvm-svn: 251449
This patch adds hashes to the plist and html output to be able to identfy bugs
for suppressing false positives or diff results against a baseline. This hash
aims to be resilient for code evolution and is usable to identify bugs in two
different snapshots of the same software. One missing piece however is a
permanent unique identifier of the checker that produces the warning. Once that
issue is resolved, the hashes generated are going to change. Until that point
this feature is marked experimental, but it is suitable for early adoption.
Differential Revision: http://reviews.llvm.org/D10305
Original patch by: Bence Babati!
llvm-svn: 251011
Prevent invalidation of `this' when a method is const; fixing PR 21606.
A patch by Sean Eveson!
Differential Revision: http://reviews.llvm.org/D13099
llvm-svn: 250237
These test updates almost exclusively around the change in behavior
around enum: enums without a definition are considered incomplete except
when targeting MSVC ABIs. Since these tests are interested in the
'incomplete-enum' behavior, restrict them to %itanium_abi_triple.
llvm-svn: 249660
Change the analyzer's modeling of memcpy to be more precise when copying into fixed-size
array fields. With this change, instead of invalidating the entire containing region the
analyzer now invalidates only offsets for the array itself when it can show that the
memcpy stays within the bounds of the array.
This addresses false positive memory leak warnings of the kind reported by
krzysztof in https://llvm.org/bugs/show_bug.cgi?id=22954
(This is the second attempt, now with assertion failures resolved.)
A patch by Pierre Gousseau!
Differential Revision: http://reviews.llvm.org/D12571
llvm-svn: 248516
This patch ignores malloc-overflow bug in two cases:
Case1:
x = a/b; where n < b
malloc (x*n); Then x*n will not overflow.
Case2:
x = a; // when 'a' is a known value.
malloc (x*n);
Also replaced isa with dyn_cast.
Reject multiplication by zero cases in MallocOverflowSecurityChecker
Currently MallocOverflowSecurityChecker does not catch cases like:
malloc(n * 0 * sizeof(int));
This patch rejects such cases.
Two test cases added. malloc-overflow2.c has an example inspired from a code
in linux kernel where the current checker flags a warning while it should not.
A patch by Aditya Kumar!
Differential Revision: http://reviews.llvm.org/D9924
llvm-svn: 248446
Various improvements to the localization checker:
* Adjusted copy to be consistent with diagnostic text in other Apple
API checkers.
* Added in ~150 UIKit / AppKit methods that require localized strings in
UnlocalizedStringsChecker.
* UnlocalizedStringChecker now checks for UI methods up the class hierarchy and
UI methods that conform for a certain Objective-C protocol.
* Added in alpha version of PluralMisuseChecker and some regression tests. False
positives are still not ideal.
(This is the second attempt, with the memory issues on Linux resolved.)
A patch by Kulpreet Chilana!
Differential Revision: http://reviews.llvm.org/D12417
llvm-svn: 248432
Various improvements to the localization checker:
* Adjusted copy to be consistent with diagnostic text in other Apple
API checkers.
* Added in ~150 UIKit / AppKit methods that require localized strings in
UnlocalizedStringsChecker.
* UnlocalizedStringChecker now checks for UI methods up the class hierarchy and
UI methods that conform for a certain Objective-C protocol.
* Added in alpha version of PluralMisuseChecker and some regression tests. False
positives are still not ideal.
A patch by Kulpreet Chilana!
Differential Revision: http://reviews.llvm.org/D12417
llvm-svn: 248350
Currently realloc(ptr, 0) is treated as free() which seems to be not correct. C
standard (N1570) establishes equivalent behavior for malloc(0) and realloc(ptr,
0): "7.22.3 Memory management functions calloc, malloc, realloc: If the size of
the space requested is zero, the behavior is implementation-defined: either a
null pointer is returned, or the behavior is as if the size were some nonzero
value, except that the returned pointer shall not be used to access an object."
The patch equalizes the processing of malloc(0) and realloc(ptr,0). The patch
also enables unix.Malloc checker to detect references to zero-allocated memory
returned by realloc(ptr,0) ("Use of zero-allocated memory" warning).
A patch by Антон Ярцев!
Differential Revision: http://reviews.llvm.org/D9040
llvm-svn: 248336
This fixes PR16833, in which the analyzer was using large amounts of memory
for switch statements with large case ranges.
rdar://problem/14685772
A patch by Aleksei Sidorin!
Differential Revision: http://reviews.llvm.org/D5102
llvm-svn: 248318
Summary:
`TypeTraitExpr`s are not supported by the ExprEngine today. Analyzer
creates a sink, and aborts the block. Therefore, certain bugs that
involve type traits intrinsics cannot be detected (see PR24710).
This patch creates boolean `SVal`s for `TypeTraitExpr`s, which are
evaluated by the compiler.
Test within the patch is a summary of PR24710.
Reviewers: zaks.anna, dcoughlin, krememek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D12482
llvm-svn: 248314
Summary:
Name `Out` refers to the parameter. It is moved into the member `Out`
in ctor-init. Dereferencing null pointer will crash clang, if user
passes '-analyzer-viz-egraph-ubigraph' argument.
Reviewers: zaks.anna, krememek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D12119
llvm-svn: 248050
The analyzer trims unnecessary nodes from the exploded graph before reporting
path diagnostics. However, in some cases it can trim all nodes (including the
error node), leading to an assertion failure (see
https://llvm.org/bugs/show_bug.cgi?id=24184).
This commit addresses the issue by adding two new APIs to CheckerContext to
explicitly create error nodes. Unless the client provides a custom tag, these
APIs tag the node with the checker's tag -- preventing it from being trimmed.
The generateErrorNode() method creates a sink error node, while
generateNonFatalErrorNode() creates an error node for a path that should
continue being explored.
The intent is that one of these two methods should be used whenever a checker
creates an error node.
This commit updates the checkers to use these APIs. These APIs
(unlike addTransition() and generateSink()) do not take an explicit Pred node.
This is because there are not any error nodes in the checkers that were created
with an explicit different than the default (the CheckerContext's Pred node).
It also changes generateSink() to require state and pred nodes (previously
these were optional) to reduce confusion.
Additionally, there were several cases where checkers did check whether a
generated node could be null; we now explicitly check for null in these places.
This commit also includes a test case written by Ying Yi as part of
http://reviews.llvm.org/D12163 (that patch originally addressed this issue but
was reverted because it introduced false positive regressions).
Differential Revision: http://reviews.llvm.org/D12780
llvm-svn: 247859
In Objective-C, method calls with nil receivers are essentially no-ops. They
do not fault (although the returned value may be garbage depending on the
declared return type and architecture). Programmers are aware of this
behavior and will complain about a false alarm when the analyzer
diagnoses API violations for method calls when the receiver is known to
be nil.
Rather than require each individual checker to be aware of this behavior
and suppress a warning when the receiver is nil, this commit
changes ExprEngineObjC so that VisitObjCMessage skips calling checker
pre/post handlers when the receiver is definitely nil. Instead, it adds a
new event, ObjCMessageNil, that is only called in that case.
The CallAndMessageChecker explicitly cares about this case, so I've changed it
to add a callback for ObjCMessageNil and moved the logic in PreObjCMessage
that handles nil receivers to the new callback.
rdar://problem/18092611
Differential Revision: http://reviews.llvm.org/D12123
llvm-svn: 247653
Add an option (-analyzer-config min-blocks-for-inline-large=14) to control the function
size the inliner considers as large, in relation to "max-times-inline-large". The option
defaults to the original hard coded behaviour, which I believe should be adjustable with
the other inlining settings.
The analyzer-config test has been modified so that the analyzer will reach the
getMinBlocksForInlineLarge() method and store the result in the ConfigTable, to ensure it
is dumped by the debug checker.
A patch by Sean Eveson!
Differential Revision: http://reviews.llvm.org/D12406
llvm-svn: 247463
This is making our internal build bot fail because it results in extra warnings being
emitted past what should be sink nodes. (There is actually an example of this in the
updated malloc.c test in the reverted commit.)
I'm working on a patch to fix the original issue by adding a new checker API to explicitly
create error nodes. This API will ensure that error nodes are always tagged in order to
prevent them from being reclaimed.
This reverts commit r246188.
llvm-svn: 247103
Change the analyzer's modeling of memcpy to be more precise when copying into fixed-size
array fields. With this change, instead of invalidating the entire containing region the
analyzer now invalidates only offsets for the array itself when it can show that the
memcpy stays within the bounds of the array.
This addresses false positive memory leak warnings of the kind reported by
krzysztof in https://llvm.org/bugs/show_bug.cgi?id=22954
A patch by Pierre Gousseau!
Differential Revision: http://reviews.llvm.org/D11832
llvm-svn: 246345
The assertion is caused by reusing a “filler” ExplodedNode as an error node.
The “filler” nodes are only used for intermediate processing and are not
essential for analyzer history, so they can be reclaimed when the
ExplodedGraph is trimmed by the “collectNode” function. When a checker finds a
bug, they generate a new transition in the ExplodedGraph. The analyzer will
try to reuse the existing predecessor node. If it cannot, it creates a new
ExplodedNode, which always has a tag to uniquely identify the creation site.
The assertion is caused when the analyzer reuses a “filler” node.
In the test case, some “filler” nodes were reused and then reclaimed later
when the ExplodedGraph was trimmed. This caused an assertion because the node
was needed to generate the report. The “filler” nodes should not be reused as
error nodes. The patch adds a constraint to prevent this happening, which
solves the problem and makes the test cases pass.
Differential Revision: http://reviews.llvm.org/D11433
Patch by Ying Yi!
llvm-svn: 246188
Matthias Gehre