This patch is a band-aid. A proper solution would be too change
trackNullOrUndefValue to only try to dereference the pointer when it is
relevant to the problem.
Differential Revision: https://reviews.llvm.org/D52435
llvm-svn: 342920
Assuming strlcat is used with strlcpy we check as we can if the last argument does not equal os not larger than the buffer.
Advising the proper usual pattern.
Reviewers: george.karpenkov, NoQ, MaskRay
Reviewed By: MaskRay
Differential Revision: https://reviews.llvm.org/D49722
llvm-svn: 342832
Modify the RetainCountChecker to perform state "adjustments" in
checkEndFunction, as performing work in PreStmt<ReturnStmt> does not
work with destructors.
The previous version made an implicit assumption that no code runs
after the return statement is executed.
rdar://43945028
Differential Revision: https://reviews.llvm.org/D52338
llvm-svn: 342770
This patch defines a new substitution and uses it to reduce
duplication in the Clang Analyzer test cases.
Differential Revision: https://reviews.llvm.org/D52036
llvm-svn: 342365
iThis patch aims to fix derefencing, which has been debated for months now.
Instead of working with SVals, the function now relies on TypedValueRegion.
Differential Revision: https://reviews.llvm.org/D51057
llvm-svn: 342213
This patch adds support for the following operations in the iterator checkers: assign, clear, insert, insert_after, emplace, emplace_after, erase and erase_after. This affects mismatched iterator checks ("this" and parameter must match) and invalidation checks (according to the standard).
Differential Revision: https://reviews.llvm.org/D32904
llvm-svn: 341794
This patch adds support for the following operations in the iterator checkers: push_back, push_front, emplace_back, emplace_front, pop_back and pop_front. This affects iterator range checks (range is extended after push and emplace and reduced after pop operations) and invalidation checks (according to the standard).
Differential Revision: https://reviews.llvm.org/D32902
llvm-svn: 341793
New check added to the checker which checks whether iterator parameters of template functions typed by the same template parameter refer to the same container.
Differential Revision: https://reviews.llvm.org/D32845
llvm-svn: 341790
Ubigraph project has been dead since about 2008, and to the best of my
knowledge, no one was using it.
Previously, I wasn't able to launch the existing binary at all.
Differential Revision: https://reviews.llvm.org/D51655
llvm-svn: 341601
Return value of dyn_cast_or_null should be checked before use.
Otherwise we may put a null pointer into the map as a key and eventually
crash in checkDeadSymbols.
Differential Revision: https://reviews.llvm.org/D51385
llvm-svn: 341092
Introduce a new MemRegion sub-class, CXXDerivedObjectRegion, which is
the opposite of CXXBaseObjectRegion, to represent such casts. Such region is
a bit weird because it is by design bigger than its super-region.
But it's not harmful when it is put on top of a SymbolicRegion
that has unknown extent anyway.
Offset computation for CXXDerivedObjectRegion and proper modeling of casts
still remains to be implemented.
Differential Revision: https://reviews.llvm.org/D51191
llvm-svn: 340984
Don't try to understand what's going on when there's a C++ method called eg.
CFRetain().
Refactor the checker a bit, to use more modern APIs.
Differential Revision: https://reviews.llvm.org/D50866
llvm-svn: 340982
The analyzer doesn't make use of them anyway and they seem to have
pretty weird AST from time to time, so let's just skip them for now.
Fixes a crash reported as pr37769.
Differential Revision: https://reviews.llvm.org/D50855
llvm-svn: 340977
The analyzer doesn't make use of them anyway and they seem to have
pretty weird AST from time to time, so let's just skip them for now.
Fixes pr37769.
Differential Revision: https://reviews.llvm.org/D50824
llvm-svn: 340975
By making sure the returned value from getKnownSVal is consistent with
the value used inside expression engine.
PR38427
Differential Revision: https://reviews.llvm.org/D51252
llvm-svn: 340965
We add check for invalidation of iterators. The only operation we handle here
is the (copy) assignment.
Differential Revision: https://reviews.llvm.org/D32747
llvm-svn: 340805
Tracking those can help to provide much better diagnostics in many cases.
In general, most of the visitor machinery should be refactored to allow
tracking the origin of arbitrary values.
rdar://36039765
Differential Revision: https://reviews.llvm.org/D51131
llvm-svn: 340475
For the following example:
struct Base {
int x;
};
// In a different translation unit
struct Derived : public Base {
Derived() {}
};
For a call to Derived::Derived(), we'll receive a note that
this->x is uninitialized. Since x is not a direct field of Derived,
it could be a little confusing. This patch aims to fix this, as well
as the case when the derived object has a field that has the name as
an inherited uninitialized data member:
struct Base {
int x; // note: uninitialized field 'this->Base::x'
};
struct Derived : public Base {
int x = 5;
Derived() {}
};
Differential Revision: https://reviews.llvm.org/D50905
llvm-svn: 340272
Now that it has it's own file, it makes little sense for
isPointerOrReferenceUninit to be this large, so I moved
dereferencing to a separate function.
Differential Revision: https://reviews.llvm.org/D50509
llvm-svn: 340265
A lot of code in RetainCountChecker deals with GC mode.
Given that GC mode is deprecated, Apple does not ship runtime for it,
and modern compiler toolchain does not support it, it makes sense to
remove the code dealing with it in order to aid understanding of
RetainCountChecker.
Differential Revision: https://reviews.llvm.org/D50747
llvm-svn: 340091
Once CFG-side support for argument construction contexts landed in r338436,
the analyzer could make use of them to evaluate argument constructors properly.
When evaluated as calls, constructors of arguments now use the variable region
of the parameter as their target. The corresponding stack frame does not yet
exist when the parameter is constructed, and this stack frame is created
eagerly.
Construction of functions whose body is unavailable and of virtual functions
is not yet supported. Part of the reason is the analyzer doesn't consistently
use canonical declarations o identify the function in these cases, and every
re-declaration or potential override comes with its own set of parameter
declarations. Also it is less important because if the function is not
inlined, there's usually no benefit in inlining the argument constructor.
Differential Revision: https://reviews.llvm.org/D49443
llvm-svn: 339745
CXXTemporaryObjectExpr is a sub-class of CXXConstructExpr. If it has arguments
that are structures passed by value, their respective constructors need to be
handled by providing a ConstructionContext, like for regular function calls and
for regular constructors.
Differential Revision: https://reviews.llvm.org/D50487
llvm-svn: 339727
- Assuming strlcat is used with strlcpy we check as we can if the last argument does not equal os not larger than the buffer.
- Advising the proper usual pattern.
Reviewers: NoQ, george.karpenkov
Reviewed By: george.karpenkov
Differential Revision: https://reviews.llvm.org/D49722
llvm-svn: 339641
Current testing setup for analyzer tests with Z3 is rather inconvenient:
There's no way to run the analyzer tests separately (I use
LIT_FILTER=Analysis ninja check-clang, but a direct target is nicer).
When Clang is built with Z3 support, there's no way to *not* run tests
with Z3 solver, and this is often desired, as tests with Z3 solver take
a very long time.
This patch introduces two extra targets:
- check-clang-analyzer
- check-clang-analyzer-z3
which solve those problems.
Differential Revision: https://reviews.llvm.org/D50594
llvm-svn: 339629
Before this patch, FieldChainInfo used a spaghetti: it took care of way too many cases,
even though it was always meant as a lightweight wrapper around
ImmutableList<const FieldRegion *>.
This problem is solved by introducing a lightweight polymorphic wrapper around const
FieldRegion *, FieldNode. It is an interface that abstracts away special cases like
pointers/references, objects that need to be casted to another type for a proper note
messages.
Changes to FieldChainInfo:
* Now wraps ImmutableList<const FieldNode &>.
* Any pointer/reference related fields and methods were removed
* Got a new add method. This replaces it's former constructors as a way to create a
new FieldChainInfo objects with a new element.
Changes to FindUninitializedField:
* In order not to deal with dynamic memory management, when an uninitialized field is
found, the note message for it is constructed and is stored instead of a
FieldChainInfo object. (see doc around addFieldToUninits).
Some of the test files are changed too, from now on uninitialized pointees of references
always print "uninitialized pointee" instead of "uninitialized field" (which should've
really been like this from the beginning).
I also updated every comment according to these changes.
Differential Revision: https://reviews.llvm.org/D50506
llvm-svn: 339599
If we get an item from a dictionary, we know that the item is non-null
if and only if the key is non-null.
This patch is a rather hacky way to record this implication, because
some logic needs to be duplicated from the solver.
And yet, it's pretty simple, performant, and works.
Other possible approaches:
- Record the implication, in future rely on Z3 to pick it up.
- Generalize the current code and move it to the constraint manager.
rdar://34990742
Differential Revision: https://reviews.llvm.org/D50124
llvm-svn: 339482
Some of the analyzer tests check the exact plist output, in order to
verify that the diagnostics produced is correct.
Current testing setup has many issues:
plist output clobbers tests, making them harder to read
it is impossible to debug test failures given error messages from FileCheck.
The only recourse is manually creating the files and using the diff
again, it is impossible to update the tests given the error message:
the only process is a tedious manual one,
going from a separate plist file to CHECK directives.
This patch offers a much better approach of using "diff" directly in place of FileCheck,
and moving tests to separate files.
Generated using the following script:
```
import os
import glob
import re
import subprocess
diagnostics_key = "// CHECK: <key>diagnostics</key>"
def process_file(f, data):
idx = data.index(diagnostics_key)
plist_out_f = 'ExpectedOutputs/plists/%s.plist' % f
plist_out_folder = os.path.join('ExpectedOutputs/plists/', os.path.dirname(f))
plist_data = data[idx:]
plist_data = plist_data.replace('// CHECK: ', '')
plist_data = plist_data.replace('// CHECK-NEXT: ', '')
plist_data += "</dict>\n</plist>\n"
data = data[:idx]
ptn = re.compile("FileCheck --?input-file(=| )(%t|%t\.plist) %s")
if not ptn.findall(data):
print "none found =/ skipping..."
return
data = ptn.sub(lambda m: "tail -n +11 %s | diff -u -w - %%S/../%s" % (m.group(2), plist_out_f), data)
with open(f, 'w') as out_f:
out_f.write(data)
subprocess.check_call(["mkdir", "-p", plist_out_folder])
with open(plist_out_f, 'w') as out_f:
out_f.write(plist_data)
def main():
files = glob.glob("**/*.*")
for f in files:
with open(f) as f_handler:
data = f_handler.read()
if diagnostics_key in data:
print "Converting %s" %f
process_file(f, data)
if __name__ == "__main__":
main()
```
Differential Revision: https://reviews.llvm.org/D50545
llvm-svn: 339475
Lambdas can affect static locals even without an explicit capture.
rdar://39537031
Differential Revision: https://reviews.llvm.org/D50368
llvm-svn: 339459
This patch fixed an issue where the dynamic type of pointer/reference
object was known by the analyzer, but wasn't obtained in the checker,
which resulted in false negatives. This should also increase reliability
of the checker, as derefencing is always done now according to the
dynamic type (even if that happens to be the same as the static type).
Special thanks to Artem Degrachev for setting me on the right track.
Differential Revision: https://reviews.llvm.org/D49199
llvm-svn: 339240
As of now, all constructor calls are ignored that are being called
by a constructor. The point of this was not to analyze the fields
of an object, so an uninitialized field wouldn't be reported
multiple times.
This however introduced false negatives when the two constructors
were in no relation to one another -- see the test file for a neat
example for this with singletons. This patch aims so fix this issue.
Differential Revision: https://reviews.llvm.org/D48436
llvm-svn: 339237
Summary:
The loop-widening code processes c++ methods looking for `this` pointers. In
the case of static methods (which do not have `this` pointers), an assertion
was triggering. This patch avoids trying to process `this` pointers for
static methods, and thus avoids triggering the assertion .
Reviewers: dcoughlin, george.karpenkov, NoQ
Reviewed By: NoQ
Subscribers: NoQ, xazax.hun, szepet, a.sidorin, mikhail.ramalho, cfe-commits
Differential Revision: https://reviews.llvm.org/D50408
llvm-svn: 339201
Even for a checker being in alpha, some reports about pointees held so little
value to the user that it's safer to disable pointer/reference chasing for now.
It can be enabled with a new flag, in which case checker should function as it
has always been. This can be set with `CheckPointeeInitialization`.
Differential Revision: https://reviews.llvm.org/D49438
llvm-svn: 339135
It now actually produces a signed APSInt when the QualType passed into it is
signed, which is what any caller would expect.
Fixes a couple of crashes.
Differential Revision: https://reviews.llvm.org/D50363
llvm-svn: 339088
The change in the AST in r338135 caused us to accidentally support
inlining constructors of operator implicit arguments. Previously they were
hard to support because they were treated as arguments in expressions
but not in declarations, but now they can be transparently treated as
simple temporaries.
Add tests and comments to explain how it now works.
Differential Revision: https://reviews.llvm.org/D49627
llvm-svn: 339087
Objects local to a function are destroyed right after the statement returning
(part of) them is executed in the analyzer. This patch enables MallocChecker to
warn in these cases.
Differential Revision: https://reviews.llvm.org/D49361
llvm-svn: 338780
The CoreEngine only gives us a ReturnStmt if the last element in the
CFGBlock is a CFGStmt, otherwise the ReturnStmt is nullptr.
This patch adds support for the case when the last element is a
CFGAutomaticObjDtor, by returning its TriggerStmt as a ReturnStmt.
Differential Revision: https://reviews.llvm.org/D49811
llvm-svn: 338777
Because of incomplete support for CXXDefaultArgExpr, we cannot yet commit to
asserting that the same destructor won't be elided twice.
Suppress the assertion failure for now. Proper support is still an open problem.
Differential Revision: https://reviews.llvm.org/D49213
llvm-svn: 338441
In r330377 and r338425 we have already identified what constitutes function
argument constructors and added stubs in order to prevent confusing them
with other temporary object constructors.
Now we implement a ConstructionContext sub-class to carry all the necessary
information about the construction site, namely call expression and argument
index.
On the analyzer side, the patch interacts with the recently implemented
pre-C++17 copy elision support in an interesting manner. If on the CFG side we
didn't find a construction context for the elidable constructor, we build
the CFG as if the elidable constructor is not elided, and the non-elided
constructor within it is a simple temporary. But the same problem may occur
in the analyzer: if the elidable constructor has a construction context but
the analyzer doesn't implement such context yet, the analyzer should also
try to skip copy elision and still inline the non-elided temporary constructor.
This was implemented by adding a "roll back" mechanism: when elision fails,
roll back the changes and proceed as if it's a simple temporary. The approach
is wonky, but i'm fine with that as long as it's merely a defensive mechanism
that should eventually go away once all construction contexts become supported.
Differential Revision: https://reviews.llvm.org/D48681.
llvm-svn: 338436
Like any normal funciton, Objective-C message can return a C++ object
in Objective-C++. Such object would require a construction context.
This patch, therefore, is an extension of r327343 onto Objective-C++.
Differential Revision: https://reviews.llvm.org/D48608
llvm-svn: 338426
CFG now correctly identifies construction context for temporaries constructed
for the purpose of passing into a function as an argument.
Such context is still not fully implemented because the information it provides
is not rich enough: it doens't contain information about argument index.
It will be addresssed later.
This patch is an extension of r330377 to C++ construct-expressions and
Objective-C message expressions which aren't call-expressions but require
similar handling. C++ new-expressions with placement arguments still remain to
be handled.
Differential Revision: https://reviews.llvm.org/D49826
llvm-svn: 338425
This fix is similar to r337769 and addresses a regression caused by r337167.
When an operation between a nonloc::LocAsInteger and a non-pointer symbol
is performed, the LocAsInteger-specific part of information is lost.
When the non-pointer symbol is collapsing into a constant, we cannot easily
re-evaluate the result, because we need to recover the missing
LocAsInteger-specific information (eg., integer type, or the very fact that
this pointer was at some point converted to an integer).
Add one more defensive check to prevent crashes on trying to simplify a
SymSymExpr with different Loc-ness of operands.
Differential Revision:
llvm-svn: 338420
When emitting a bug report, it is important to highlight which argument of the
call-expression is causing the problem.
Before:
warning: Null pointer argument in call to string comparison function
strcmp(a, b);
^~~~~~~~~~~~
After:
warning: Null pointer argument in call to string comparison function
strcmp(a, b);
^ ~
Affects other output modes as well, not just text.
Differential Revision: https://reviews.llvm.org/D50028
llvm-svn: 338333
Do not warn when the other message-send-expression is correctly wrapped
in a different autorelease pool.
Differential Revision: https://reviews.llvm.org/D49921
llvm-svn: 338314
According to the standard, pointers referring to the elements of a
`basic_string` may be invalidated if they are used as an argument to
any standard library function taking a reference to non-const
`basic_string` as an argument. This patch makes InnerPointerChecker warn
for these cases.
Differential Revision: https://reviews.llvm.org/D49656
llvm-svn: 338259
The note is added in the following situation:
- We are throwing a nullability-related warning on an IVar
- The path goes through a method which *could have* (syntactically
determined) written into that IVar, but did not
rdar://42444460
Differential Revision: https://reviews.llvm.org/D49689
llvm-svn: 338149
in some member function calls.
Specifically, when calling a conversion function, we would fail to
create the AST node representing materialization of the class object.
llvm-svn: 338135
A checker for detecting leaks resulting from allocating temporary
autoreleasing objects before starting the main run loop.
Checks for two antipatterns:
1. ObjCMessageExpr followed by [[NARunLoop mainRunLoop] run] in the same
autorelease pool.
2. ObjCMessageExpr followed by [[NARunLoop mainRunLoop] run] in no
autorelease pool.
Happens-before relationship is modeled purely syntactically.
rdar://39299145
Differential Revision: https://reviews.llvm.org/D49528
llvm-svn: 337876
The note is added in the following situation:
- We are throwing a nullability-related warning on an IVar
- The path goes through a method which *could have* (syntactically
determined) written into that IVar, but did not
rdar://42444460
Differential Revision: https://reviews.llvm.org/D49689
llvm-svn: 337864
Remove an assertion in RangeConstraintManager that expects such symbols to never
appear, while admitting that the constraint manager doesn't yet handle them.
Differential Revision: https://reviews.llvm.org/D49703
llvm-svn: 337769
This reinstates r337627, reverted in r337671, with a fix to correctly
handle the lvalueness of array subscript expressions on pointers.
llvm-svn: 337726
Patch https://reviews.llvm.org/rC329780 not only rearranges comparisons but
also binary expressions. This latter behavior is not protected by the analyzer
option. Hower, since no complexity threshold is enforced to the symbols this
may result in exponential execution time if the expressions are too complex:
https://bugs.llvm.org/show_bug.cgi?id=38208. For a quick fix we extended the
analyzer option to also cover the additive cases.
This is only a temporary fix, the final solution should be enforcing the
complexity threshold to the symbols.
Differential Revision: https://reviews.llvm.org/D49536
llvm-svn: 337678
This reverts commit r337627.
After the change, clang started producing invalid warning on the following code:
struct foo {
foo(char *x) : x_(&x[10]) {}
private:
char *x_;
};
1.cpp:2:21: warning: initializing pointer member 'x_' with the stack address of parameter 'x' [-Wdangling-field]
llvm-svn: 337671
The last argument is expected to be the destination buffer size (or less).
Detects if it points to destination buffer size directly or via a variable.
Detects if it is an integral, try to detect if the destination buffer can receive the source length.
Updating bsd-string.c unit tests as it make it fails now.
Reviewers: george.karpenpov, NoQ
Reviewed By: george.karpenkov
Differential Revision: https://reviews.llvm.org/D48884
llvm-svn: 337499
DanglingInternalBufferChecker.
A pointer referring to the elements of a basic_string may be invalidated
by calling a non-const member function, except operator[], at, front,
back, begin, rbegin, end, and rend. The checker now warns if the pointer
is used after such operations.
Differential Revision: https://reviews.llvm.org/D49360
llvm-svn: 337463
This change implements C++ DR1696, which makes initialization of a
reference member of a class from a temporary object ill-formed. The
standard wording here is imprecise, but we interpret it as meaning that
any time a mem-initializer would result in lifetime extension, the
program is ill-formed.
This reinstates r337226, reverted in r337255, with a fix for the
InitializedEntity alignment problem that was breaking ARM buildbots.
llvm-svn: 337329
This change breaks on ARM because pointers to clang::InitializedEntity are only
4 byte aligned and do not have 3 bits to store values. A possible solution
would be to change the fields in clang::InitializedEntity to enforce a bigger
alignment requirement.
The error message is
llvm/include/llvm/ADT/PointerIntPair.h:132:3: error: static_assert failed "PointerIntPair with integer size too large for pointer"
static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
include/llvm/ADT/PointerIntPair.h:73:13: note: in instantiation of template class 'llvm::PointerIntPairInfo<const clang::InitializedEntity *, 3, llvm::PointerLikeTypeTraits<const clang::InitializedEntity *> >' requested here
Value = Info::updateInt(Info::updatePointer(0, PtrVal),
llvm/include/llvm/ADT/PointerIntPair.h:51:5: note: in instantiation of member function 'llvm::PointerIntPair<const clang::InitializedEntity *, 3, (anonymous namespace)::LifetimeKind, llvm::PointerLikeTypeTraits<const clang::InitializedEntity *>, llvm::PointerIntPairInfo<const clang::InitializedEntity *, 3, llvm::PointerLikeTypeTraits<const clang::InitializedEntity *> > >::setPointerAndInt' requested here
setPointerAndInt(PtrVal, IntVal);
^
llvm/tools/clang/lib/Sema/SemaInit.cpp:6237:12: note: in instantiation of member function 'llvm::PointerIntPair<const clang::InitializedEntity *, 3, (anonymous namespace)::LifetimeKind, llvm::PointerLikeTypeTraits<const clang::InitializedEntity *>, llvm::PointerIntPairInfo<const clang::InitializedEntity *, 3, llvm::PointerLikeTypeTraits<const clang::InitializedEntity *> > >::PointerIntPair' requested here
return {Entity, LK_Extended};
Full log here:
http://lab.llvm.org:8011/builders/clang-cmake-armv7-global-isel/builds/1330http://lab.llvm.org:8011/builders/clang-cmake-armv7-full/builds/1394
llvm-svn: 337255
Copy-constructors and move-constructors may have default arguments. It is
incorrect to assert that they only have one argument, i.e. the reference to the
object being copied or moved. Remove the assertion.
Differential Revision: https://reviews.llvm.org/D49215
llvm-svn: 337229
The canonical representation of pointer &SymRegion{$x} casted to boolean is
"$x != 0", not "$x". Assertion added in r337227 catches that.
Differential Revision: https://reviews.llvm.org/D48232
llvm-svn: 337228
This change implements C++ DR1696, which makes initialization of a
reference member of a class from a temporary object ill-formed. The
standard wording here is imprecise, but we interpret it as meaning that
any time a mem-initializer would result in lifetime extension, the
program is ill-formed.
llvm-svn: 337226
Only suppress those cases where the null which came from the macro is
relevant to the bug, and was not overwritten in between.
rdar://41497323
Differential Revision: https://reviews.llvm.org/D48856
llvm-svn: 337213
Initializing a semaphore with a different constant most likely signals a different intent
rdar://41802552
Differential Revision: https://reviews.llvm.org/D48911
llvm-svn: 337212
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
It was not possible to disable alpha.unix.cstring.OutOfBounds checker's reports
since unix.Malloc checker always implicitly enabled the filter. Moreover if the
checker was disabled from command line (-analyzer-disable-checker ..) the out
of bounds warnings were nevertheless emitted under different checker names such
as unix.cstring.NullArg, or unix.Malloc.
This patch fixes the case sot that Malloc checker only enables implicitly the
underlying modeling of strcpy, memcpy etc. but not the warning messages that
would have been emmitted by alpha.unix.cstring.OutOfBounds
Patch by: Dániel Krupp
Differential Revision: https://reviews.llvm.org/D48831
llvm-svn: 337000
Previously, the checker only tracked one raw pointer symbol for each
container object. But member functions returning a pointer to the
object's inner buffer may be called on the object several times. These
pointer symbols are now collected in a set inside the program state map
and thus all of them is checked for use-after-free problems.
Differential Revision: https://reviews.llvm.org/D49057
llvm-svn: 336835
DanglingInternalBufferChecker now tracks use-after-free problems related
to the incorrect usage of std::basic_string::data().
Differential Revision: https://reviews.llvm.org/D48532
llvm-svn: 336497
Add a bug visitor to DanglingInternalBufferChecker that places a note
at the point where the dangling pointer was obtained. The visitor is
handed over to MallocChecker and attached to the report there.
Differential Revision: https://reviews.llvm.org/D48522
llvm-svn: 336495
Extend MallocBugVisitor to place a note at the point where objects with
AF_InternalBuffer allocation family are destroyed.
Differential Revision: https://reviews.llvm.org/D48521
llvm-svn: 336489
Summary: In the provided test case the PathDiagnostic compare function was not able to find a difference.
Reviewers: xazax.hun, NoQ, dcoughlin, george.karpenkov
Reviewed By: george.karpenkov
Subscribers: a_sidorin, szepet, rnkovacs, a.sidorin, mikhail.ramalho, cfe-commits
Differential Revision: https://reviews.llvm.org/D48474
llvm-svn: 336275
In order to better support consumers of the plist output that don't
parse note entries just yet, a 'NotesAsWarnings' flag was added.
If it's set to true, all notes will be converted to warnings.
Differential Revision: https://reviews.llvm.org/D48285
llvm-svn: 335964
After r335814, the constraint manager is no longer generating a false bug report
about the division by zero in the test case.
This patch removes the expected false bug report.
llvm-svn: 335932
Add handling of the begin() funcion of containers to the iterator checkers,
together with the pre- and postfix ++ and -- operators of the iterators. This
makes possible the checking of iterators dereferenced ahead of the begin of the
container.
Differential Revision: https://reviews.llvm.org/D32642
llvm-svn: 335835
If range [m .. n] is stored for symbolic expression A - B, then we can deduce the range for B - A which is [-n .. -m]. This is only true for signed types, unless the range is [0 .. 0].
Differential Revision: https://reviews.llvm.org/D35110
llvm-svn: 335814
The ProgramState::assumeInBound() API is used by checkers to make an assumption
that a certain array index is within the array's bounds (i.e. is greater than or
equal to 0 and is less than the length of the array). When the type of the
index was unspecified by the caller, it assumed that the type is 'int', which
caused some indices and sizes to truncate during calculations.
Use ArrayIndexTy by default instead, which is used by the analyzer to represent
index types and is currently hardcoded to long long.
Patch by Bevin Hansson!
Differential Revision: https://reviews.llvm.org/D46944
llvm-svn: 335803
r335795 adds copy elision information to CFG. This commit allows static analyzer
to elide elidable copy constructors by constructing the objects that were
previously subject to elidable copy directly in the target region of the copy.
The chain of elided constructors may potentially be indefinitely long. This
only happens when the object is being returned from a function which in turn is
returned from another function, etc.
NRVO is not supported yet.
Differential Revision: https://reviews.llvm.org/D47671
llvm-svn: 335800
When a temporary object is materialized and through that obtain lifetime that
is longer than the duration of the full-expression, it does not require a
temporary object destructor; it will be destroyed in a different manner.
Therefore it's not necessary to include CXXBindTemporaryExpr into the
construction context for such temporary in the CFG only to make clients
throw it away.
Differential Revision: https://reviews.llvm.org/D47667
llvm-svn: 335798
When an object's class provides no destructor, it's less important to
materialize that object properly because we don't have to model the destructor
correctly, so previously we skipped the support for these syntax patterns.
Additionally, fix support for construction contexts of "static temporaries"
(temporaries that are lifetime-extended by static references) because
it turned out that we only had tests for them without destructors, which caused
us to regress when we re-introduced the construction context for such
temporaries.
Differential Revision: https://reviews.llvm.org/D47658
llvm-svn: 335796
Before C++17 copy elision was optional, even if the elidable copy/move
constructor had arbitrary side effects. The elidable constructor is present
in the AST, but marked as elidable.
In these cases CFG now contains additional information that allows its clients
to figure out if a temporary object is only being constructed so that to pass
it to an elidable constructor. If so, it includes a reference to the elidable
constructor's construction context, so that the client could elide the
elidable constructor and construct the object directly at its final destination.
Differential Revision: https://reviews.llvm.org/D47616
llvm-svn: 335795
In the current implementation, we run visitors until the fixed point is
reached.
That is, if a visitor adds another visitor, the currently processed path
is destroyed, all diagnostics is discarded, and it is regenerated again,
until it's no longer modified.
This pattern has a few negative implications:
- This loop does not even guarantee to terminate.
E.g. just imagine two visitors bouncing a diagnostics around.
- Performance-wise, e.g. for sqlite3 all visitors are being re-run at
least 10 times for some bugs.
We have already seen a few reports where it leads to timeouts.
- If we want to add more computationally intense visitors, this will
become worse.
- From architectural standpoint, the current layout requires copying
visitors, which is conceptually wrong, and can be annoying (e.g. no
unique_ptr on visitors allowed).
The proposed change is a much simpler architecture: the outer loop
processes nodes upwards, and whenever the visitor is added it only
processes current nodes and above, thus guaranteeing termination.
Differential Revision: https://reviews.llvm.org/D47856
llvm-svn: 335666
ExprWithCleanups wraps full-expressions that require temporary destructors
and highlights the moment of time in which these destructors need to be called
(i.e., "at the end of the full-expression...").
Such expressions don't necessarily return an object; they may return anything,
including a null or undefined value.
When the analyzer tries to understand where the null or undefined value came
from in order to present better diagnostics to the user, it will now skip
any ExprWithCleanups it encounters and look into the expression itself.
Differential Revision: https://reviews.llvm.org/D48204
llvm-svn: 335559
Conservative evaluation of a C++ method call would invalidate the object,
as long as the method is not const or the object has mutable fields.
When checking for mutable fields, we need to scan the type of the object on
which the method is called, which may be more specific than the type of the
object on which the method is defined, hence we look up the type from the
this-argument expression.
If arrow syntax or implicit-this syntax is used, this-argument expression
has pointer type, not record type, and lookup accidentally failed for that
reason. Obtain object type correctly.
Differential Revision: https://reviews.llvm.org/D48460
llvm-svn: 335555
This checker analyzes C++ constructor calls, and reports uninitialized fields.
Due to the nature of this problem (uninitialized fields after an object
construction), this checker doesn't search for bugs, but rather is a tool to
enforce a specific programming model where every field needs to be initialized.
This checker lands in alpha for now, and a number of followup patches will be
made to reduce false negatives and to make it easier for the user to understand
what rules the checker relies on, eg. whether a derived class' constructor is
responsible for initializing inherited data members or whether it should be
handled in the base class' constructor.
Differential Revision: https://reviews.llvm.org/D45532
llvm-svn: 334935
Not contexts themselves, but rather support for them in the analyzer.
Such construction contexts appear when C++17 mandatory copy elision occurs
while returning an object from a function, and presence of a destructor causes
a CXXBindTemporaryExpr to appear in the AST.
Additionally, such construction contexts may be chained, because a return-value
construction context doesn't really explain where the object is being returned
into, but only points to the parent stack frame, where the object may be
consumed by literally anything including another return statement. This
behavior is now modeled correctly by the analyzer as long as the object is not
returned beyond the boundaries of the analysis.
Differential Revision: https://reviews.llvm.org/D47405
llvm-svn: 334684
Not contexts themselves, but rather support for them in the analyzer.
Such construction contexts appear when C++17 mandatory copy elision occurs
during initialization, and presence of a destructor causes a
CXXBindTemporaryExpr to appear in the AST.
Similar C++17-specific constructors for return values are still to be supported.
Differential Revision: https://reviews.llvm.org/D47351
llvm-svn: 334683
The reasoning behind this change is similar to the previous commit, r334681.
Because members are already in scope when construction occurs, we are not
suffering from liveness problems, but we still want to figure out if the object
was constructed with construction context, because in this case we'll be able
to avoid trivial copy, which we don't always model perfectly. It'd also have
more importance when copy elision is implemented.
This also gets rid of the old CFG look-behind mechanism.
Differential Revision: https://reviews.llvm.org/D47350
llvm-svn: 334682
The very idea of construction context implies that first the object is
constructed, and then later, in a separate moment of time, the constructed
object goes into scope, i.e. becomes "live".
Most construction contexts require path-sensitive tracking of the constructed
object region in order to compute the outer expressions accordingly before
the object becomes live.
Semantics of simple variable construction contexts don't immediately require
that such tracking happens in path-sensitive manner, but shortcomings of the
analyzer force us to track it path-sensitively as well. Namely, whether
construction context was available at all during construction is a
path-sensitive information. Additionally, path-sensitive tracking takes care of
our liveness problems that kick in as the temporal gap between construction and
going-into-scope becomes larger (eg., due to copy elision).
Differential Revision: https://reviews.llvm.org/D47305
llvm-svn: 334681
Loop widening can invalidate a reference. If the analyzer attempts to visit the
destructor to a non-existent reference, it will crash. This patch ensures that
the reference is preserved.
https://reviews.llvm.org/D47044
llvm-svn: 334554
Rename AlternateExtensive to Extensive.
In 2013, five years ago, we have switched to AlternateExtensive
diagnostics by default, and Extensive was available under unused,
undocumented flag.
This change remove the flag, renames the Alternate
diagnostic to Extensive (as it's no longer Alternate), and ports the
test.
Differential Revision: https://reviews.llvm.org/D47670
llvm-svn: 334524
This check will mark raw pointers to C++ standard library container internal
buffers 'released' when the objects themselves are destroyed. Such information
can be used by MallocChecker to warn about use-after-free problems.
In this first version, 'std::basic_string's are supported.
Differential Revision: https://reviews.llvm.org/D47135
llvm-svn: 334348
Summary:
Since Z3 tests have been not been running [1] some tests needed to be
updated. I also added a regression test for [1].
[1] https://reviews.llvm.org/D47722
Reviewers: george.karpenkov, NoQ, ddcc
Reviewed By: george.karpenkov
Subscribers: mikhail.ramalho, dcoughlin, xazax.hun, szepet, zzheng, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D47726
llvm-svn: 334067
Temporary object constructor inlining was disabled in r326240 for code like
const int &x = A().x;
because automatic destructor for the lifetime-extended object A() was not
working correctly in CFG.
CFG was fixed in r333941, so inlining can be re-enabled. CFG for lifetime
extension through aggregates still needs to be fixed.
Differential Revision: https://reviews.llvm.org/D44239
llvm-svn: 333946
In code like
const int &x = A().x;
automatic destructor for the object A() lifetime-extended by reference 'x' was
not present in the clang CFG due to ad-hoc pattern-matching in
getReferenceInitTemporaryType().
Re-use skipRValueSubobjectAdjustments() again to find the lifetime-extended
object in the AST and emit the correct destructor.
Lifetime extension through aggregates with references still needs to be covered.
Differential Revision: https://reviews.llvm.org/D44238
llvm-svn: 333941
Summary: This is a prototype of a bug reporter visitor that invalidates bug reports by re-checking constraints of certain states on the bug path using the Z3 constraint manager backend. The functionality is available under the `crosscheck-with-z3` analyzer config flag.
Reviewers: george.karpenkov, NoQ, dcoughlin, rnkovacs
Reviewed By: george.karpenkov
Subscribers: rnkovacs, NoQ, george.karpenkov, dcoughlin, xbolva00, ddcc, mikhail.ramalho, MTC, fhahn, whisperity, baloghadamsoftware, szepet, a.sidorin, gsd, dkrupp, xazax.hun, cfe-commits
Differential Revision: https://reviews.llvm.org/D45517
llvm-svn: 333903
Summary: Clang does not have a corresponding QualType for a 1-bit APSInt, so use the BoolTy and extend the APSInt. Split from D35450. Fixes PR37622.
Reviewers: george.karpenkov, NoQ
Subscribers: mikhail.ramalho, xazax.hun, szepet, rnkovacs, cfe-commits, a.sidorin
Differential Revision: https://reviews.llvm.org/D47603
llvm-svn: 333704
When neither LHS nor RHS of a binary operator expression can be simplified,
return the original expression instead of re-evaluating the binary operator.
Such re-evaluation was causing recusrive re-simplification which caused
the algorithmic complexity to explode.
Differential Revision: https://reviews.llvm.org/D47155
llvm-svn: 333670
Previously, the checker was using the nullability of the expression,
which is nonnull IFF both receiver and method are annotated as _Nonnull.
However, the receiver could be known to the analyzer to be nonnull
without being explicitly marked as _Nonnull.
rdar://40635584
Differential Revision: https://reviews.llvm.org/D47510
llvm-svn: 333612
Summary: If the access is out of bounds, return UndefinedVal. If it is missing an explicit init, return the implicit zero value it must have.
Reviewers: NoQ, xazax.hun, george.karpenkov
Reviewed By: NoQ
Subscribers: szepet, rnkovacs, a.sidorin, cfe-commits
Differential Revision: https://reviews.llvm.org/D46823
llvm-svn: 333417
These functions are obsolete. The analyzer would advice to replace them with
memcmp(), memcpy() or memmove(), or memset().
Patch by Tom Rix!
Differential Revision: https://reviews.llvm.org/D41881
llvm-svn: 333326
Because template parameter lists were not displayed
in the plist output, it was difficult to decide in
some cases whether a given checker found a true or a
false positive. This patch aims to correct this.
Differential Revision: https://reviews.llvm.org/D46933
llvm-svn: 333275
Again, strlc* does not return a pointer so the zero size case doest not fit.
Reviewers: NoQ, george.karpenkov
Reviewed by: NoQ
Differential Revision: https://reviews.llvm.org/D47007
llvm-svn: 333060
Since there is no perfect way bind the non-zero value with the default binding, this patch only considers the case where buffer's offset is zero and the char value is 0. And according to the value for overwriting, decide how to update the string length.
Reviewers: dcoughlin, NoQ, xazax.hun, a.sidorin, george.karpenkov
Reviewed By: NoQ
Differential Revision: https://reviews.llvm.org/D44934
llvm-svn: 332463
Previously plist-html output produced multi-file HTML reports
but only single-file Plist reports.
Change plist-html output to produce multi-file Plist reports as well.
Differential Revision: https://reviews.llvm.org/D46902
llvm-svn: 332417
A common pattern is that the code in the block does not write into the
variable explicitly, but instead passes it to a helper function which
performs the write.
Differential Revision: https://reviews.llvm.org/D46772
llvm-svn: 332300
We weren't invalidating our unions correctly. The previous behavior in
invalidateRegionsWorker::VisitCluster() was to direct-bind an UnknownVal
to the union (at offset 0).
For that reason we were never actually loading default bindings from our unions,
because there never was any default binding to load, and the value
that is presumed when there's no default binding to load
is usually completely incorrect (eg. UndefinedVal for stack unions).
The new behavior is to default-bind a conjured symbol (of irrelevant type)
to the union that's being invalidated, similarly to what we do for structures
and classes. Then it becomes safe to load the value properly.
Differential Revision: https://reviews.llvm.org/D45241
llvm-svn: 331563
C allows us to write any bytes into any memory region. When loading weird bytes
from memory regions of known types, the analyzer is required to make sure that
the loaded value makes sense by casting it to an appropriate type.
Fix such cast for loading values that represent void pointers from non-void
pointer type places.
Differential Revision: https://reviews.llvm.org/D46415
llvm-svn: 331562
The bindDefault() API of the ProgramState allows setting a default value
for reads from memory regions that were not preceded by writes.
It was used for implementing C++ zeroing constructors (i.e. default constructors
that boil down to setting all fields of the object to 0).
Because differences between zeroing consturctors and other forms of default
initialization have been piling up (in particular, zeroing constructors can be
called multiple times over the same object, probably even at the same offset,
requiring a careful and potentially slow cleanup of previous bindings in the
RegionStore), we split the API in two: bindDefaultInitial() for modeling
initial values and bindDefaultZero() for modeling zeroing constructors.
This fixes a few assertion failures from which the investigation originated.
The imperfect protection from both inability of the RegionStore to support
binding extents and lack of information in ASTRecordLayout has been loosened
because it's, well, imperfect, and it is unclear if it fixing more than it
was breaking.
Differential Revision: https://reviews.llvm.org/D46368
llvm-svn: 331561
Many glvalue expressions aren't of their respective reference type -
they are simply glvalues of their value type.
This was causing problems when we were trying to obtain type of the original
expression while evaluating certain glvalue bit-casts.
Fixed by artificially forging a reference type to provide to the casting
procedure.
Differential Revision: https://reviews.llvm.org/D46224
llvm-svn: 331558
When loading from a variable or a field that is declared as constant,
the analyzer will try to inspect its initializer and constant-fold it.
Upon success, the analyzer would skip normal load and return the respective
constant.
The new behavior also applies to fields/elements of brace-initialized structures
and arrays.
Patch by Rafael Stahl!
Differential Revision: https://reviews.llvm.org/D45774
llvm-svn: 331556
Summary: Add `TaintBugVisitor` to the ArrayBoundV2, DivideZero, VLASize to be able to indicate where the taint information originated from.
Reviewers: NoQ, george.karpenkov, xazax.hun, a.sidorin
Reviewed By: NoQ
Subscribers: szepet, rnkovacs, cfe-commits, MTC
Differential Revision: https://reviews.llvm.org/D46007
llvm-svn: 331345
Avoid crash when the sub-expression of operator delete[] is of array type.
This is not the same as simply using a delete[] syntax.
We're still not properly calling destructors in this case in the analyzer.
Differential Revision: https://reviews.llvm.org/D46146
llvm-svn: 331014
If 'A' is a C++ aggregate with a reference field of type 'C', in code like
A a = { C() };
C() is lifetime-extended by 'a'. The analyzer wasn't expecting this pattern and
crashing. Additionally, destructors aren't added in the CFG for this case,
so for now we shouldn't be inlining the constructor for C().
Differential Revision: https://reviews.llvm.org/D46037
llvm-svn: 330882
Normally the analyzer begins path-sensitive analysis from functions within
the main file, even though the path is allowed to go through any functions
within the translation unit.
When a recent version of WebKit is compiled, the "unified sources" technique
is used, that assumes #including multiple code files into a single main file.
Such file would have no functions defined in it, so the analyzer wouldn't be
able to find any entry points for path-sensitive analysis.
This patch pattern-matches unified file names that are similar to those
used by WebKit and allows the analyzer to find entry points in the included
code files. A more aggressive/generic approach is being planned as well.
Differential Revision: https://reviews.llvm.org/D45839
llvm-svn: 330876
Note diagnostic pieces are an additional way of highlighting code sections to
the user. They aren't part of the normal path diagnostic sequence. They can
also be attached to path-insensitive reports.
Notes are already supported by the text output and scan-build.
Expanding our machine-readable plist output format to be able to represent notes
opens up the possibility for various analyzer GUIs to pick them up.
Patch by Umann Kristóf!
Differential Revision: https://reviews.llvm.org/D45407
llvm-svn: 330766
This patch introduces the ability to test an arbitrary sequence of imports
between a given set of virtual source files. This should finally allow
us to write simple tests and fix annoying issues inside ASTImporter
that cause failures in CSA CTU. This is done by refactoring
ASTImporterTest functions and introducing `testImportSequence` facility.
As a side effect, `testImport` facility was generalized a bit more. It
should now allow import of non-decl AST nodes; however, there is still no
test using this ability.
As a "test for test", there is also a fix for import anonymous TagDecls
referred by typedef. Before this patch, the setting of typedef for anonymous
structure was delayed; however, this approach misses the corner case if
an enum constant is imported directly. In this patch, typedefs for
anonymous declarations are imported right after the anonymous declaration
is imported, without any delay.
Thanks to Adam Balogh for suggestions included into this patch.
Differential Revision: https://reviews.llvm.org/D44079
llvm-svn: 330704
Printing of ConcreteInts with size >64 bits resulted in assertion failure
in get[Z|S]ExtValue() because these methods are only allowed to be used
with integers of 64 max bit width. This patch fixes the issue.
llvm-svn: 330605
Loop condition variables, eg.
while (shared_ptr<int> P = getIntPtr()) { ... })
weren't handled in r324794 because they don't go through the common
CFGBuilder::VisitDeclStmt method. Which means that they regressed
after r324800.
Fix the regression by duplicating the necessary construction context scan in
the loop visiting code.
Differential Revision: https://reviews.llvm.org/D45706
llvm-svn: 330382
If a pointer cast fails (evaluates to an UnknownVal, i.e. not implemented in the
analyzer) and such cast is in fact the last use of the pointer, the pointer
symbol is no longer referenced by the program state and a leak is
(mis-)diagnosed.
"Escape" the pointer upon a failed cast, i.e. inform the checker that we can no
longer reliably track it.
Differential Revision: https://reviews.llvm.org/D45698
llvm-svn: 330380
Function argument constructors (that are used for passing objects into functions
by value) are completely unlike temporary object constructors, but we were
treating them as such because they are also wrapped into a CXXBindTemporaryExpr.
This patch adds a partial construction context layer for call argument values,
but doesn't proceed to transform it into an actual construction context yet.
This is tells the clients that we aren't supporting these constructors yet.
Differential Revision: https://reviews.llvm.org/D45650
llvm-svn: 330377
r315736 added support for the misplaced CF_RETURNS_RETAINED annotation on
CFRetain() wrappers. It works by trusting the function's name (seeing if it
confirms to the CoreFoundation naming convention) rather than the annotation.
There are more false positives caused by users using a different naming
convention, namely starting the function name with "retain" or "release"
rather than suffixing it with "retain" or "release" respectively.
Because this isn't according to the naming convention, these functions
are usually inlined and the annotation is therefore ignored, which is correct.
But sometimes we run out of inlining stack depth and the function is
evaluated conservatively and then the annotation is trusted.
Add support for the "alternative" naming convention and test the situation when
we're running out of inlining stack depth.
rdar://problem/18270122
Differential Revision: https://reviews.llvm.org/D45117
llvm-svn: 330375
Summary:
`this` pointer is not an l-value, although we have modeled `CXXThisRegion` for `this` pointer, we can only bind it once, which is when we start to inline method. And this patch fixes https://bugs.llvm.org/show_bug.cgi?id=35506.
In addition, I didn't find any other cases other than loop-widen that could invalidate `this` pointer.
Reviewers: NoQ, george.karpenkov, a.sidorin, seaneveson, szepet
Reviewed By: NoQ
Subscribers: xazax.hun, rnkovacs, cfe-commits, MTC
Differential Revision: https://reviews.llvm.org/D45491
llvm-svn: 330095
Expression rearrangement in SValBuilder (see rL329780) crashes with an assert if the type of the integer is different from the type of the symbol. This fix adds a check that prevents rearrangement in such cases.
Differential Revision: https://reviews.llvm.org/D45557
llvm-svn: 330064
Since the range-based constraint manager (default) is weak in handling comparisons where symbols are on both sides it is wise to rearrange them to have symbols only on the left side. Thus e.g. A + n >= B + m becomes A - B >= m - n which enables the constraint manager to store a range m - n .. MAX_VALUE for the symbolic expression A - B. This can be used later to check whether e.g. A + k == B + l can be true, which is also rearranged to A - B == l - k so the constraint manager can check whether l - k is in the range (thus greater than or equal to m - n).
The restriction in this version is the the rearrangement happens only if both the symbols and the concrete integers are within the range [min/4 .. max/4] where min and max are the minimal and maximal values of their type.
The rearrangement is not enabled by default. It has to be enabled by using -analyzer-config aggressive-relational-comparison-simplification=true.
Co-author of this patch is Artem Dergachev (NoQ).
Differential Revision: https://reviews.llvm.org/D41938
llvm-svn: 329780
Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:
archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
methode
nd
optin
ot
pres
statics
te
thru
Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few
files that have dubious fixes reverted.)
Differential revision: https://reviews.llvm.org/D44188
llvm-svn: 329399
removeUnneededCalls() is responsible for removing path diagnostic pieces within
functions that don't contain "interesting" events. It makes bug reports
much tidier.
When a stack frame is known to be interesting, the function doesn't descend
into it to prune anything within it, even other callees that are totally boring.
Fix the function to prune boring callees in interesting stack frames.
Differential Revision: https://reviews.llvm.org/D45117
llvm-svn: 329102
Summary:
The original implementation in the `LoopUnrolling.cpp` didn't consider the case where the counter is unsigned. This case is only handled in `simpleCondition()`, but this is not enough, we also need to deal with the unsinged counter with the counter initialization.
Since `IntegerLiteral` is `signed`, there is a `ImplicitCastExpr<IntegralCast>` in `unsigned counter = IntergerLiteral`. This patch add the `ignoringParenImpCasts()` in the `IntegerLiteral` matcher.
Reviewers: szepet, a.sidorin, NoQ, george.karpenkov
Reviewed By: szepet, george.karpenkov
Subscribers: xazax.hun, rnkovacs, cfe-commits, MTC
Differential Revision: https://reviews.llvm.org/D45086
llvm-svn: 328919
C++ structured bindings for non-tuple-types are defined in a peculiar
way, where the resulting declaration is not a VarDecl, but a
BindingDecl.
That means a lot of existing machinery stops working.
rdar://36912381
Differential Revision: https://reviews.llvm.org/D44956
llvm-svn: 328910
Otherwise the default triple for x86-windows-msvc2015 auto-inserts
__attribute__((thiscall)) to some calls.
Fixes the respective buildbot.
llvm-svn: 328903
Pointer arithmetic on null or undefined pointers results in null or undefined
pointers. This is obvious for undefined pointers; for null pointers it follows
from our incorrect-but-somehow-working approach that declares that 0 (Loc)
doesn't necessarily represent a pointer of numeric address value 0, but instead
it represents any pointer that will cause a valid "null pointer dereference"
issue when dereferenced.
For now we've been seeing through pointer arithmetic at the original dereference
expression, i.e. in bugreporter::getDerefExpr(), but not during further
investigation of the value's origins in bugreporter::trackNullOrUndefValue().
The patch fixes it.
Differential Revision: https://reviews.llvm.org/D45071
llvm-svn: 328896
Sometimes template instantiation causes CXXBindTemporaryExpr to be missing in
its usual spot. In CFG, temporary destructors work by relying on
CXXBindTemporaryExprs, so they won't work in this case.
Avoid the crash and notify the clients that we've encountered an unsupported AST
by failing to provide the ill-formed construction context for the temporary.
Differential Revision: https://reviews.llvm.org/D44955
llvm-svn: 328895
Not enough work has been done so far to ensure correctness of construction
contexts in the CFG when C++17 copy elision is in effect, so for now we
should drop construction contexts in the CFG and in the analyzer when
they seem different from what we support anyway.
This includes initializations with conditional operators and return values
across multiple stack frames.
Differential Revision: https://reviews.llvm.org/D44854
llvm-svn: 328893
Extended the matched assignment operators when checking for bound changes in a body of the loop by using the freshly added isAssignmentOperator matcher.
This covers all the (current) possible assignments, tests added as well.
Differential Revision: https://reviews.llvm.org/D38921
llvm-svn: 328619
Changes the analyzer to believe that methods annotated with _Nonnull
from system frameworks indeed return non null objects.
Local methods with such annotation are still distrusted.
rdar://24291919
Differential Revision: https://reviews.llvm.org/D44341
llvm-svn: 328282
When a temporary is constructed with a proper construction context, it should
be safe to inline the destructor. We have added suppressions for some of the
common false positives caused by such inlining, so there should be - and from my
observations there indeed is - more benefit than harm from enabling destructor
inlining.
Differential Revision: https://reviews.llvm.org/D44721
llvm-svn: 328258
CXXCtorInitializer-based constructors are also affected by the C++17 mandatory
copy elision, like variable constructors and return value constructors.
Extend r328248 to support those.
Differential Revision: https://reviews.llvm.org/D44763
llvm-svn: 328255
Function return values can be constructed directly in variables or passed
directly into return statements, without even an elidable copy in between.
This is how the C++17 mandatory copy elision AST behaves. The behavior we'll
have in such cases is the "old" behavior that we've had before we've
implemented destructor inlining and proper lifetime extension support.
Differential Revision: https://reviews.llvm.org/D44755
llvm-svn: 328253
In C++17 copy elision is mandatory for variable and return value constructors
(as long as it doesn't involve type conversion) which results in AST that does
not contain elidable constructors in their usual places. In order to provide
construction contexts in this scenario we need to cover more AST patterns.
This patch makes the CFG prepared for these scenarios by:
- Fork VariableConstructionContext and ReturnedValueConstructionContext into
two different sub-classes (each) one of which indicates the C++17 case and
contains a reference to an extra CXXBindTemporaryExpr.
- Allow CFGCXXRecordTypedCall element to accept VariableConstructionContext and
ReturnedValueConstructionContext as its context.
Differential Revision: https://reviews.llvm.org/D44597
llvm-svn: 328248
If a memory region (or an SVal that represents a pointer to that memory region)
is a (direct or indirect, not necessarily proper) sub-region of a SymbolicRegion
then it is said to have a symbolic base.
For now SVal::symbol_iterator explores the symbol within a symbolic region
only when the SVal represents a pointer to the symbolic region itself,
not to any of its sub-regions.
This behavior is not indended by any user of symbol_iterator; all users who
cared about such behavior were expecting the iterator to descend into the
symbolic base of an arbitrary region, find the parent symbol of the symbolic
base region, and iterate over that symbol. Lack of such behavior resulted in
bugs demonstarted by the test cases.
Hence the decision to change the API to behave more intuitively.
Differential Revision: https://reviews.llvm.org/D44347
llvm-svn: 328247
r326249 wasn't quite enough because we often run out of inlining stack depth
limit and for that reason fail to see the atomics we're looking for.
Add a more straightforward false positive suppression that is based on the name
of the class. I.e. if we're releasing a pointer in a destructor of a "something
shared/intrusive/reference/counting something ptr/pointer something", then any
use-after-free or double-free that occurs later would likely be a false
positive.
rdar://problem/38013606
Differential Revision: https://reviews.llvm.org/D44281
llvm-svn: 328066
When the loop has a null terminator statement and sets 'widen-loops=true', 'invalidateRegions' will constructs the 'SymbolConjured' with null 'Stmt'. And this will lead to a crash in 'IteratorChecker.cpp'. This patch use 'dyn_cast_or_null<>' instead of 'dyn_cast<>' in IteratorChecker.cpp.
Differential Revision: https://reviews.llvm.org/D44606
llvm-svn: 327962
Also use the opportunity to clean up the code and remove unnecessary duplication.
rdar://37625895
Differential Revision: https://reviews.llvm.org/D44594
llvm-svn: 327926
For other regions, the error message contains a good indication of the
problem, and there, in general, nothing helpful we can print.
Error pointer to the problematic expression seems enough.
rdar://37323555
Differential Revision: https://reviews.llvm.org/D44409
llvm-svn: 327727
Call expressions that return objects by an lvalue reference or an rvalue
reference have a value type in the AST but wear an auxiliary flag of being an
lvalue or an xvalue respectively.
Use the helper method for obtaining the actual return type of the function.
Fixes a crash.
Differential Revision: https://reviews.llvm.org/D44273
llvm-svn: 327352
Properly perform destruction and lifetime extension of such temporaries.
C++ object-type return values of conservatively evaluated functions are now
represented as compound values of well-defined temporary object regions. The
function creates a region that represents the temporary object and will later
be used for destruction or materialization, invalidates it, and returns the
invalidated compound value of the object.
Differential Revision: https://reviews.llvm.org/D44131
llvm-svn: 327348
This patch uses the newly added CFGCXXRecordTypedCall element at the call site
of the caller to construct the return value within the callee directly into the
caller's stack frame. This way it is also capable of populating the temporary
destructor and lifetime extension maps for the temporary, which allows
temporary destructors and lifetime extension to work correctly.
This patch does not affect temporaries that were returned from conservatively
evaluated functions.
Differential Revision: https://reviews.llvm.org/D44124
llvm-svn: 327345
This patch adds a new CFGStmt sub-class, CFGCXXRecordTypedCall, which replaces
the regular CFGStmt for the respective CallExpr whenever the CFG has additional
information to provide regarding the lifetime of the returned value.
This additional call site information is represented by a ConstructionContext
(which was previously used for CFGConstructor elements) that provides references
to CXXBindTemporaryExpr and MaterializeTemporaryExpr that surround the call.
This corresponds to the common C++ calling convention solution of providing
the target address for constructing the return value as an auxiliary implicit
argument during function call.
One of the use cases for such extra context at the call site would be to perform
any sort of inter-procedural analysis over the CFG that involves functions
returning objects by value. In this case the elidable constructor at the return
site would construct the object explained by the context at the call site, and
its lifetime would also be managed by the caller, not the callee.
The extra context would also be useful for properly handling the return-value
temporary at the call site, even if the callee is not being analyzed
inter-procedurally.
Differential Revision: https://reviews.llvm.org/D44120
llvm-svn: 327343
This patch adds two new CFG elements CFGScopeBegin and CFGScopeEnd that indicate
when a local scope begins and ends respectively. We use first VarDecl declared
in a scope to uniquely identify it and add CFGScopeBegin and CFGScopeEnd elements
into corresponding basic blocks.
Differential Revision: https://reviews.llvm.org/D16403
llvm-svn: 327258
mprotect() allows setting memory access flags similarly to mmap(),
causing similar security issues if these flags are needlessly broad.
Patch by David Carlier!
Differential Revision: https://reviews.llvm.org/D44250
llvm-svn: 327098
Implicit constructor conversions such as A a = B() are represented by
surrounding the constructor for B() with an ImplicitCastExpr of
CK_ConstructorConversion kind, similarly to how explicit constructor conversions
are surrounded by a CXXFunctionalCastExpr. Support this syntax pattern when
extracting the construction context for the implicit constructor that
performs the conversion.
Differential Revision: https://reviews.llvm.org/D44051
llvm-svn: 327096
Previously, iteration through nil objects which resulted from
objc-messages being set to nil were modeled incorrectly.
There are a couple of notes about this patch:
In principle, ExprEngineObjC might be left untouched IFF osx.loops
checker is enabled.
I however think that we should not do something
completely incorrect depending on what checkers are left on.
We should evaluate and potentially remove altogether the isConsumedExpr
performance heuristic, as it seems very fragile.
rdar://22205149
Differential Revision: https://reviews.llvm.org/D44178
llvm-svn: 326982
Proper modeling still remains to be done.
Note that BindingDecl#getHoldingVar() is almost always null, and this
should probably be handled by dealing with DecompositionDecl beforehand.
rdar://36852163
Differential Revision: https://reviews.llvm.org/D44183
llvm-svn: 326951
Summary:
There is a problem with analyzer that a wrong value is given when modeling the increment operator of the operand with type bool. After `rL307604` is applied, a unsigned overflow may occur.
Example:
```
void func() {
bool b = true;
// unsigned overflow occur, 2 -> 0 U1b
b++;
}
```
The use of an operand of type bool with the ++ operators is deprecated but valid untill C++17. And if the operand of the increment operator is of type bool, it is set to true.
This patch includes two parts:
- If the operand of the increment operator is of type bool or type _Bool, set to true.
- Modify `BasicValueFactory::getTruthValue()`, use `getIntWidth()` instead `getTypeSize()` and use `unsigned` instead `signed`.
Reviewers: alexshap, NoQ, dcoughlin, george.karpenkov
Reviewed By: NoQ
Subscribers: xazax.hun, szepet, a.sidorin, cfe-commits, MTC
Differential Revision: https://reviews.llvm.org/D43741
llvm-svn: 326776
Summary:
GenericTaintChecker can't recognize stdin in some cases. The reason is that `if (PtrTy->getPointeeType() == C.getASTContext().getFILEType()` does not hold when stdin is encountered.
My platform is ubuntu16.04 64bit, gcc 5.4.0, glibc 2.23. The definition of stdin is as follows:
```
__BEGIN_NAMESPACE_STD
/* The opaque type of streams. This is the definition used elsewhere. */
typedef struct _IO_FILE FILE;
___END_NAMESPACE_STD
...
/* The opaque type of streams. This is the definition used elsewhere. */
typedef struct _IO_FILE __FILE;
...
/* Standard streams. */
extern struct _IO_FILE *stdin; /* Standard input stream. */
extern struct _IO_FILE *stdout; /* Standard output stream. */
extern struct _IO_FILE *stderr; /* Standard error output stream. */
```
The type of stdin is as follows AST:
```
ElaboratedType 0xc911170'struct _IO_FILE'sugar
`-RecordType 0xc911150'struct _IO_FILE'
`-CXXRecord 0xc923ff0'_IO_FILE'
```
`C.getASTContext().GetFILEType()` is as follows AST:
```
TypedefType 0xc932710 'FILE' sugar
|-Typedef 0xc9111c0 'FILE'
`-ElaboratedType 0xc911170 'struct _IO_FILE' sugar
`-RecordType 0xc911150 'struct _IO_FILE'
`-CXXRecord 0xc923ff0 '_IO_FILE'
```
So I think it's better to use `getCanonicalType()`.
Reviewers: zaks.anna, NoQ, george.karpenkov, a.sidorin
Reviewed By: zaks.anna, a.sidorin
Subscribers: a.sidorin, cfe-commits, xazax.hun, szepet, MTC
Differential Revision: https://reviews.llvm.org/D39159
llvm-svn: 326709
```
if (NSNumber* x = ...)
```
is a reasonable pattern in objc++, we should not warn on it.
rdar://35152234
Differential Revision: https://reviews.llvm.org/D44044
llvm-svn: 326619
Don't enable c++-temp-dtor-inlining by default yet, due to this reference
counting pointe problem.
Otherwise the new mode seems stable and allows us to incrementally fix C++
problems in much less hacky ways.
Differential Revision: https://reviews.llvm.org/D43804
llvm-svn: 326461
Originally submitted as r326323 and r326324.
Reverted in r326432.
Reverting the commit was a mistake.
The breakage was due to invalid build files in our internal buildsystem,
CMakeLists did not have any cyclic dependencies.
llvm-svn: 326439
Also revert "[analyzer] Fix a compiler warning"
This reverts commits r326323 and r326324.
Reason: the commits introduced a cyclic dependency in the build graph.
This happens to work with cmake, but breaks out internal integrate.
llvm-svn: 326432
This is a security check that warns when both PROT_WRITE and PROT_EXEC are
set during mmap(). If mmap()ed memory is both writable and executable, it makes
it easier for the attacker to execute arbitrary code when contents of this
memory are compromised. Some applications require such mmap()s though, such as
different sorts of JIT.
Re-applied after a revert in r324167.
Temporarily stays in the alpha package because it needs a better way of
determining macro values that are not immediately available in the AST.
Patch by David Carlier!
Differential Revision: https://reviews.llvm.org/D42645
llvm-svn: 326405
For now. We should also add support for ConstructorConversion casts as presented
in the attached test case, but this requires more changes because AST around
them seems different.
The check was originally present but was accidentally lost during r326021.
Differential Revision: https://reviews.llvm.org/D43840
llvm-svn: 326402
The aim of this patch is to be minimal to enable incremental development of
the feature on the top of the tree. This patch should be an NFC when the
feature is turned off. It is turned off by default and still considered as
experimental.
Technical details are available in the EuroLLVM Talk:
http://llvm.org/devmtg/2017-03//2017/02/20/accepted-sessions.html#7
Note that the initial prototype was done by A. Sidorin et al.: http://lists.llvm.org/pipermail/cfe-dev/2015-October/045730.html
Contributions to the measurements and the new version of the code: Peter Szecsi, Zoltan Gera, Daniel Krupp, Kareem Khazem.
Differential Revision: https://reviews.llvm.org/D30691
llvm-svn: 326323
When a class forgets to initialize a field in the constructor, and then gets
copied around, a warning is emitted that the value assigned to a specific field
is undefined.
When the copy/move constructor is implicit (not written out in the code) but not
trivial (is not a trivial memory copy, eg. because members have an explicit copy
constructor), the body of such constructor is auto-generated in the AST.
In this case the checker's warning message is squeezed at the top of
the class declaration, and it gets hard to guess which field is at fault.
Fix the warning message to include the name of the field.
Differential Revision: https://reviews.llvm.org/D43798
llvm-svn: 326258
Throw away MallocChecker warnings that occur after releasing a pointer within a
destructor (or its callees) after performing C11 atomic fetch_add or fetch_sub
within that destructor (or its callees).
This is an indication that the destructor's class is likely a
reference-counting pointer. The analyzer is not able to understand that the
original reference count is usually large enough to avoid most use-after-frees.
Even when the smart pointer is a local variable, we still have these false
positives that this patch suppresses, because the analyzer doesn't currently
support atomics well enough.
Differential Revision: https://reviews.llvm.org/D43791
llvm-svn: 326249
The SVal for any empty C++ object is an UnknownVal. Because RegionStore does
not have binding extents, binding an empty object to an UnknownVal may
potentially overwrite existing bindings at the same offset.
Therefore, when performing a trivial copy of an empty object, don't try to
take the value of the object and bind it to the copy. Doing nothing is accurate
enough, and it doesn't screw any existing bindings.
Differential Revision: https://reviews.llvm.org/D43714
llvm-svn: 326247
Sometimes it is not known at compile time which temporary objects will be
constructed, eg. 'x ? A() : B()' or 'C() || D()'. In this case we track which
temporary was constructed to know how to properly call the destructor.
Once the construction context for temporaries was introduced, we moved the
tracking code to the code that investigates the construction context.
Bring back the old mechanism because construction contexts are not always
available yet - eg. in the case where a temporary is constructed without a
constructor expression, eg. returned from a function by value. The mechanism
should still go away eventually.
Additionally, fix a bug in the temporary cleanup code for the case when
construction contexts are not available, which could lead to temporaries
staying in the program state and increasing memory consumption.
Differential Revision: https://reviews.llvm.org/D43666
llvm-svn: 326246
If a variable or an otherwise a concrete typed-value region is being
placement-new'ed into, its dynamic type may change in arbitrary manners. And
when the region is used, there may be a third type that's different from both
the static and the dynamic type. It cannot be *completely* different from the
dynamic type, but it may be a base class of the dynamic type - and in this case
there isn't (and shouldn't be) any indication anywhere in the AST that there is
a derived-to-base cast from the dynamic type to the third type.
Perform a generic cast (evalCast()) from the third type to the dynamic type
in this case. From the point of view of the SVal hierarchy, this would have
produced non-canonical SVals if we used such generic cast in the normal case,
but in this case there doesn't seem to be a better option.
Differential Revision: https://reviews.llvm.org/D43659
llvm-svn: 326245
Automatic destructors are missing in the CFG in situations like
const int &x = C().x;
For now it's better to disable construction inlining, because inlining
constructors while doing nothing on destructors is very bad.
Differential Revision: https://reviews.llvm.org/D43689
llvm-svn: 326240
This patch uses the reference to MaterializeTemporaryExpr stored in the
construction context since r326014 in order to model that expression correctly.
When modeling MaterializeTemporaryExpr, instead of copying the raw memory
contents from the sub-expression's rvalue to a completely new temporary region,
that we conjure up for the lack of better options, we now have the better
option to recall the region into which the object was originally constructed
and declare that region to be the value of the expression, which is semantically
correct.
This only works when the construction context is available, which is worked on
independently.
The temporary region's liveness (in the sense of removeDeadBindings) is extended
until the MaterializeTemporaryExpr is resolved, in order to keep the store
bindings around, because it wouldn't be referenced from anywhere else in the
program state.
Differential Revision: https://reviews.llvm.org/D43497
llvm-svn: 326236
Fixes https://bugs.llvm.org/show_bug.cgi?id=36474
In general, getSVal API should be changed so that it does not crash on
some non-obvious conditions.
It should either be updated to require a type, or to return Optional<SVal>.
Differential Revision: https://reviews.llvm.org/D43801
llvm-svn: 326233
George Karpenkov