summaryrefslogtreecommitdiffstats
path: root/clang/lib/StaticAnalyzer
Commit message (Collapse)AuthorAgeFilesLines
* [analyzer] [NFC] Factor out generating path diagnostics for a statement into ↵George Karpenkov2018-02-071-636/+648
| | | | | | | | a function Differential Revision: https://reviews.llvm.org/D42558 llvm-svn: 324507
* Revert r324166 "[analyzer] Add a checker for mmap()...".Artem Dergachev2018-02-032-76/+0
| | | | | | | | Due to Buildbot failures - most likely that's because target triples were not specified in the tests, even though the checker behaves differently with different target triples. llvm-svn: 324167
* [analyzer] Add a checker for mmap()s which are both writable and executable.Artem Dergachev2018-02-032-0/+76
| | | | | | | | | | | | | | This is a security check which is disabled by default but will be enabled whenever the user consciously enables the security package. 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. Patch by David Carlier! Differential Revision: https://reviews.llvm.org/D42645 llvm-svn: 324166
* [analyzer] Do not infer nullability inside function-like macros, even when ↵George Karpenkov2018-02-031-7/+103
| | | | | | | | | | | | | | | | | | | | | macro is explicitly returning NULL We already suppress such reports for inlined functions, we should then get the same behavior for macros. The underlying reason is that the same macro, can be called from many different contexts, and nullability can only be expected in _some_ of them. Assuming that the macro can return null in _all_ of them sometimes leads to a large number of false positives. E.g. consider the test case for the dynamic cast implementation in macro: in such cases, the bug report is unwanted. Tracked in rdar://36304776 Differential Revision: https://reviews.llvm.org/D42404 llvm-svn: 324161
* [analyzer] Fix transitions in check::PreStmt<MemberExpr> checker callback.Artem Dergachev2018-02-021-5/+4
| | | | | | | | | | | No in-tree checkers use this callback so far, hence no tests. But better fix this now than remember to fix this when the checkers actually appear. Patch by Henry Wong! Differential Revision: https://reviews.llvm.org/D42785 llvm-svn: 324053
* [analyzer] Expose return statement from CallExit program pointGeorge Karpenkov2018-02-023-5/+11
| | | | | | | | | | | | | | | | If the return statement is stored, we might as well allow querying against it. Also fix the bug where the return statement is not stored if there is no return value. This change un-merges two ExplodedNodes during call exit when the state is otherwise identical - the CallExitBegin node itself and the "Bind Return Value"-tagged node. And expose the return statement through getStatement helper function. Differential Revision: https://reviews.llvm.org/D42130 llvm-svn: 324052
* Remove the change which accidentally crept in into the cherry-pickGeorge Karpenkov2018-02-021-1/+0
| | | | llvm-svn: 324050
* [analyzer] Expose exploration strategy through analyzer options.George Karpenkov2018-02-023-5/+50
| | | | | | Differential Revision: https://reviews.llvm.org/D42774 llvm-svn: 324049
* [analyzer] Don't communicate evaluation failures through memregion hierarchy.Artem Dergachev2018-02-012-45/+42
| | | | | | | | | | | | | | | | | | | | | | | We use CXXTempObjectRegion exclusively as a bailout value for construction targets when we are unable to find the correct construction region. Sometimes it works correctly, but rather accidentally than intentionally. Now that we want to increase the amount of situations where it works correctly, the first step is to introduce a different way of communicating our failure to find the correct construction region. EvalCallOptions are introduced for this purpose. For now EvalCallOptions are communicating two kinds of problems: - We have been completely unable to find the correct construction site. - We have found the construction site correctly, and there's more than one of them (i.e. array construction which we currently don't support). Accidentally find and fix a test in which the new approach to communicating failures produces better results. Differential Revision: https://reviews.llvm.org/D42457 llvm-svn: 324018
* [analyzer] Extend SuppressInlineDefensiveChecksVisitor to all macros, ↵George Karpenkov2018-01-301-13/+7
| | | | | | | | | | | | including non-function-like ones No reason to treat function-like macros differently here. Tracked in rdar://29907377 Differential Revision: https://reviews.llvm.org/D42444 llvm-svn: 323827
* [analyzer] [NFC] Remove unused method visitItemsInWorkListGeorge Karpenkov2018-01-291-33/+0
| | | | | | Differential Revision: https://reviews.llvm.org/D42562 llvm-svn: 323696
* [NFC] fix trivial typos in comments and documentsHiroshi Inoue2018-01-261-1/+1
| | | | | | "in in" -> "in", "on on" -> "on" etc. llvm-svn: 323509
* [analyzer] Do not attempt to get the pointee of void*Alexander Shaposhnikov2018-01-241-0/+3
| | | | | | | | | | | | Do not attempt to get the pointee of void* while generating a bug report (otherwise it will trigger an assert inside RegionStoreManager::getBinding assert(!T->isVoidType() && "Attempting to dereference a void pointer!")). Test plan: make check-all Differential revision: https://reviews.llvm.org/D42396 llvm-svn: 323382
* [analyzer] Enable c++-allocator-inlining by default.Artem Dergachev2018-01-241-1/+1
| | | | | | | | | | | | | | | | | | | | | | This allows the analyzer to analyze ("inline") custom operator new() calls and, even more importantly, inline constructors of objects that were allocated by any operator new() - not necessarily a custom one. All changes in the tests in the current commit are intended improvements, even if they didn't carry any explicit FIXME flag. It is possible to restore the old behavior via -analyzer-config c++-allocator-inlining=false (this flag is supported by scan-build as well, and it can be into a clang --analyze invocation via -Xclang .. -Xclang ..). There is no intention to remove the old behavior for now. Differential Revision: https://reviews.llvm.org/D42219 rdar://problem/12180598 llvm-svn: 323373
* [analyzer] Assume that the allocated value is non-null before construction.Artem Dergachev2018-01-241-17/+32
| | | | | | | | | | | | | | | | | | I.e. not after. In the c++-allocator-inlining=true mode, we need to make the assumption that the conservatively evaluated operator new() has returned a non-null value. Previously we did this on CXXNewExpr, but now we have to do that before calling the constructor, because some clever constructors are sometimes assuming that their "this" is null and doing weird stuff. We would also crash upon evaluating CXXNewExpr when the allocator was inlined and returned null and had a throw specification; this is UB even for custom allocators, but we still need not to crash. Added more FIXME tests to ensure that eventually we fix calling the constructor for null return values. Differential Revision: https://reviews.llvm.org/D42192 llvm-svn: 323370
* [analyzer] Mark lines as relevant even if they weren't executed but have a ↵George Karpenkov2018-01-231-92/+113
| | | | | | | | label attached Differential Revision: https://reviews.llvm.org/D42320 llvm-svn: 323251
* [analyzer] Show full analyzer invocation for reproducibility in HTML reportsGeorge Karpenkov2018-01-231-1/+6
| | | | | | | | | | | | Analyzing problems which appear in scan-build results can be very difficult, as after the launch no exact invocation is stored, and it's super-hard to launch the debugger. With this patch, the exact analyzer invocation appears in the footer, and can be copied to debug/check reproducibility/etc. rdar://35980230 llvm-svn: 323245
* [analyzer] Protect against dereferencing a null pointerAlexander Shaposhnikov2018-01-221-3/+3
| | | | | | | | | | | | | The check (inside StackHintGeneratorForSymbol::getMessage) if (!N) return getMessageForSymbolNotFound() is moved to the beginning of the function. Differential revision: https://reviews.llvm.org/D42388 Test plan: make check-all llvm-svn: 323146
* [analyzer] Model and check unrepresentable left shiftsGabor Horvath2018-01-222-3/+25
| | | | | | | | Patch by: Reka Nikolett Kovacs Differential Revision: https://reviews.llvm.org/D41816 llvm-svn: 323115
* [NFC] fix trivial typos in commentsHiroshi Inoue2018-01-222-2/+2
| | | | | | "the the" -> "the" llvm-svn: 323078
* [analyzer] Provide a check name when MallocChecker enables CStringCheckerDevin Coughlin2018-01-201-1/+11
| | | | | | | | | Fix an assertion failure caused by a missing CheckName. The malloc checker enables "basic" support in the CStringChecker, which causes some CString bounds checks to be enabled. In this case, make sure that we have a valid CheckName for the BugType. llvm-svn: 323052
* [analyzer] a few helper methods for getting and comparing symbolic valuesGeorge Karpenkov2018-01-182-1/+21
| | | | | | | | API calls should express intent, and that's a motivation behind this patch. Differential Revision: https://reviews.llvm.org/D42218 llvm-svn: 322809
* [analyzer] NFC: RetainCount: Protect from dumping raw region to path notes.Artem Dergachev2018-01-181-2/+8
| | | | | | | | | | | | | | | | | | MemRegion::getString() is a wrapper around MemRegion::dump(), which is not user-friendly and should never be used for diagnostic messages. Actual cases where raw dumps were reaching the user were unintentionally fixed in r315736; these were noticed accidentally and shouldn't be reproducible anymore. For now RetainCountChecker only tracks pointers through variable regions, and for those dumps are "fine". However, we should still use a less dangerous method for producing our path notes. This patch replaces the dump with printing a variable name, asserting that this is indeed a variable. Differential Revision: https://reviews.llvm.org/D42015 llvm-svn: 322799
* [analyzer] operator new: Fix callback order for CXXNewExpr.Artem Dergachev2018-01-183-2/+53
| | | | | | | | | | | | | | | | PreStmt<CXXNewExpr> was never called. Additionally, under c++-allocator-inlining=true, PostStmt<CXXNewExpr> was called twice when the allocator was inlined: once after evaluating the new-expression itself, once after evaluating the allocator call which, for the lack of better options, uses the new-expression as the call site. This patch fixes both problems. Differential Revision: https://reviews.llvm.org/D41934 rdar://problem/12180598 llvm-svn: 322797
* [analyzer] operator new: Add a new ProgramPoint for check::NewAllocator.Artem Dergachev2018-01-183-7/+5
| | | | | | | | | | | | | | Add PostAllocatorCall program point to represent the moment in the analysis between the operator new() call and the constructor call. Pointer cast from "void *" to the correct object pointer type has already happened by this point. The new program point, unlike the previously used PostImplicitCall, contains a reference to the new-expression, which allows adding path diagnostics over it. Differential Revision: https://reviews.llvm.org/D41800 rdar://problem/12180598 llvm-svn: 322796
* [analyzer] Suppress "this" pointer escape during construction.Artem Dergachev2018-01-181-2/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | Pointer escape event notifies checkers that a pointer can no longer be reliably tracked by the analyzer. For example, if a pointer is passed into a function that has no body available, or written into a global, MallocChecker would no longer report memory leaks for such pointer. In case of operator new() under -analyzer-config c++-allocator-inlining=true, MallocChecker would start tracking the pointer allocated by operator new() only to immediately meet a pointer escape event notifying the checker that the pointer has escaped into a constructor (assuming that the body of the constructor is not available) and immediately stop tracking it. Even though it is theoretically possible for such constructor to put "this" into a global container that would later be freed, we prefer to preserve the old behavior of MallocChecker, i.e. a memory leak warning, in order to be able to find any memory leaks in C++ at all. In fact, c++-allocator-inlining *reduces* the amount of false positives coming from this-pointers escaping in constructors, because it'd be able to inline constructors in some cases. With other checkers working similarly, we simply suppress the escape event for this-value of the constructor, regardless of analyzer options. Differential Revision: https://reviews.llvm.org/D41797 rdar://problem/12180598 llvm-svn: 322795
* [analyzer] operator new: Fix path diagnostics around the operator call.Artem Dergachev2018-01-181-1/+4
| | | | | | | | | | | | Implements finding appropriate source locations for intermediate diagnostic pieces in path-sensitive bug reports that need to descend into an inlined operator new() call that was called via new-expression. The diagnostics have worked correctly when operator new() was called "directly". Differential Revision: https://reviews.llvm.org/D41409 rdar://problem/12180598 llvm-svn: 322791
* [analyzer] operator new: Add a new checker callback, check::NewAllocator.Artem Dergachev2018-01-175-31/+140
| | | | | | | | | | | | | | | The callback runs after operator new() and before the construction and allows the checker to access the casted return value of operator new() (in the sense of r322780) which is not available in the PostCall callback for the allocator call. Update MallocChecker to use the new callback instead of PostStmt<CXXNewExpr>, which gets called after the constructor. Differential Revision: https://reviews.llvm.org/D41406 rdar://problem/12180598 llvm-svn: 322787
* [analyzer] operator new: Fix ambigious type name.Artem Dergachev2018-01-171-2/+3
| | | | | | Hopefully fixes an MSVC buildbot failure. llvm-svn: 322781
* [analyzer] operator new: Fix memory space for the returned region.Artem Dergachev2018-01-172-18/+20
| | | | | | | | | | | | | | | Make sure that with c++-allocator-inlining=true we have the return value of conservatively evaluated operator new() in the correct memory space (heap). This is a regression/omission that worked well in c++-allocator-inlining=false. Heap regions are superior to regular symbolic regions because they have stricter aliasing constraints: heap regions do not alias each other or global variables. Differential Revision: https://reviews.llvm.org/D41266 rdar://problem/12180598 llvm-svn: 322780
* [analyzer] operator new: Model the cast of returned pointer into object type.Artem Dergachev2018-01-172-13/+41
| | | | | | | | | | | | | | | | | | | | | | | According to [basic.stc.dynamic.allocation], the return type of any C++ overloaded operator new() is "void *". However, type of the new-expression "new T()" and the type of "this" during construction of "T" are both "T *". Hence an implicit cast, which is not present in the AST, needs to be performed before the construction. This patch adds such cast in the case when the allocator was indeed inlined. For now, in the case where the allocator was *not* inlined we still use the same symbolic value (which is a pure SymbolicRegion of type "T *") because it is consistent with how we represent the casts and causes less surprise in the checkers after switching to the new behavior. The better approach would be to represent that value as a cast over a SymbolicRegion of type "void *", however we have technical difficulties conjuring such region without any actual expression of type "void *" present in the AST. Differential Revision: https://reviews.llvm.org/D41250 rdar://problem/12180598 llvm-svn: 322777
* [analyzer] NFC: Forbid array elements of void type.Artem Dergachev2018-01-172-3/+17
| | | | | | | | | | | | | | | Represent the symbolic value for results of pointer arithmetic on void pointers in a different way: instead of making void-typed element regions, make char-typed element regions. Add an assertion that ensures that no void-typed regions are ever constructed. This is a refactoring of internals that should not immediately affect the analyzer's (default) behavior. Differential Revision: https://reviews.llvm.org/D40939 llvm-svn: 322775
* [analyzer] operator new: Use the correct region for the constructor.Artem Dergachev2018-01-173-39/+159
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | The -analyzer-config c++-allocator-inlining experimental option allows the analyzer to reason about C++ operator new() similarly to how it reasons about regular functions. In this mode, operator new() is correctly called before the construction of an object, with the help of a special CFG element. However, the subsequent construction of the object was still not performed into the region of memory returned by operator new(). The patch fixes it. Passing the value from operator new() to the constructor and then to the new-expression itself was tricky because operator new() has no call site of its own in the AST. The new expression itself is not a good call site because it has an incorrect type (operator new() returns 'void *', while the new expression is a pointer to the allocated object type). Additionally, lifetime of the new expression in the environment makes it unsuitable for passing the value. For that reason, an additional program state trait is introduced to keep track of the return value. Finally this patch relaxes restrictions on the memory region class that are required for inlining the constructor. This change affects the old mode as well (c++-allocator-inlining=false) and seems safe because these restrictions were an overkill compared to the actual problems observed. Differential Revision: https://reviews.llvm.org/D40560 rdar://problem/12180598 llvm-svn: 322774
* [analyzer] introduce getSVal(Stmt *) helper on ExplodedNode, make sure the ↵George Karpenkov2018-01-1736-160/+108
| | | | | | | | | | | | | | | | | | helper is used consistently In most cases using `N->getState()->getSVal(E, N->getLocationContext())` is ugly, verbose, and also opens up more surface area for bugs if an inconsistent location context is used. This patch introduces a helper on an exploded node, and ensures consistent usage of either `ExplodedNode::getSVal` or `CheckContext::getSVal` across the codebase. As a result, a large number of redundant lines is removed. Differential Revision: https://reviews.llvm.org/D42155 llvm-svn: 322753
* [analyzer] Make isSubRegionOf reflexiveGeorge Karpenkov2018-01-175-10/+7
| | | | | | | | | All usages of isSubRegionOf separately check for reflexive case, and in any case, set theory tells us that each set is a subset of itself. Differential Revision: https://reviews.llvm.org/D42140 llvm-svn: 322752
* [analyzer] Better UI in html reports for displaying shortcuts helpGeorge Karpenkov2018-01-171-3/+44
| | | | | | | Make the help window accessible, but don't show by default. Use a different CSS class from macro. llvm-svn: 322750
* [analyzer] support a mode to only show relevant lines in HTML diagnosticsGeorge Karpenkov2018-01-173-17/+174
| | | | | | | | | | | | | | HTML diagnostics can be an overwhelming blob of pages of code. This patch adds a checkbox which filters this list down to only the lines *relevant* to the counterexample by e.g. skipping branches which analyzer has assumed to be infeasible at a time. The resulting amount of output is much smaller, and often fits on one screen, and also provides a much more readable diagnostics. Differential Revision: https://reviews.llvm.org/D41378 llvm-svn: 322612
* [analyzer] Don't flag strcpy of string literals into sufficiently large buffers.Artem Dergachev2018-01-121-0/+11
| | | | | | | | | | | | | | | In the security package, we have a simple syntactic check that warns about strcpy() being insecure, due to potential buffer overflows. Suppress that check's warning in the trivial situation when the source is an immediate null-terminated string literal and the target is an immediate sufficiently large buffer. Patch by András Leitereg! Differential Revision: https://reviews.llvm.org/D41384 llvm-svn: 322410
* [analyzer] [NFC] Minor refactoring of trackNullOrUndefValueGeorge Karpenkov2018-01-101-80/+97
| | | | | | | | | Simple refactoring attempt: factor out some code, remove some repetition, use auto where appropriate. Differential Revision: https://reviews.llvm.org/D41751 llvm-svn: 322151
* [analyzer] [NFC] minor FindLastStoreBRVisitor refactoringGeorge Karpenkov2018-01-101-112/+127
| | | | | | Differential Revision: https://reviews.llvm.org/D41790 llvm-svn: 322150
* [analyzer] suppress nullability inference from a macro when result is used ↵George Karpenkov2018-01-101-4/+14
| | | | | | | | | | | | | | | | | | in another macro The current code used to not suppress the report, if the dereference was performed in a macro, assuming it is that same macro. However, the assumption might not be correct, and XNU has quite a bit of code where dereference is actually performed in a different macro. As the code uses macro name and not a unique identifier it might be fragile, but in a worst-case scenario we would simply emit an extra diagnostic. rdar://36160245 Differential Revision: https://reviews.llvm.org/D41749 llvm-svn: 322149
* [analyzer] Fix some check's output plist not containing the check nameGabor Horvath2018-01-062-8/+17
| | | | | | Differential Revision: https://reviews.llvm.org/D41538 llvm-svn: 321933
* [analyzer] do not crash with assertion on processing locations of bodyfarmed ↵George Karpenkov2018-01-021-1/+0
| | | | | | | | | | | | | | | functions This addresses an issue introduced in r183451: since `removePiecesWithInvalidLocations` is called *after* `adjustCallLocations`, it is not necessary, and in fact harmful, to have this assertion in adjustCallLocations. Addresses rdar://36170689 Differential Revision: https://reviews.llvm.org/D41680 llvm-svn: 321682
* [analyzer] Add Javascript to analyzer HTML output to allow keyboard navigation.George Karpenkov2017-12-211-2/+93
| | | | | | Differential Revision: https://reviews.llvm.org/D41414 llvm-svn: 321320
* Re-commit r321223, which adds a printing policy to the ASTDumper.Aaron Ballman2017-12-211-328/+330
| | | | | | | | This allows you to dump C++ code that spells bool instead of _Bool, leaves off the elaborated type specifiers when printing struct or class names, and other C-isms. Fixes the -Wreorder issue and fixes the ast-dump-color.cpp test. llvm-svn: 321310
* [analyzer] Fix zero-initialization of stack VLAs under ObjC ARC.Artem Dergachev2017-12-211-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Using ARC, strong, weak, and autoreleasing stack variables are implicitly initialized with nil. This includes variable-length arrays of Objective-C object pointers. However, in the analyzer we don't zero-initialize them. We used to, but it accidentally regressed after r289618. Under ARC, the array variable's initializer within DeclStmt is an ImplicitValueInitExpr. Environment doesn't maintain any bindings for this expression kind - instead it always knows that it's a known constant (0 in our case), so it just returns the known value by calling SValBuilder::makeZeroVal() (see EnvironmentManager::getSVal(). Commit r289618 had introduced reasonable behavior of SValBuilder::makeZeroVal() for the arrays, which produces a zero-length compoundVal{}. When such value is bound to arrays, in RegionStoreManager::bindArray() "remaining" items in the array are default-initialized with zero, as in RegionStoreManager::setImplicitDefaultValue(). The similar mechanism works when an array is initialized by an initializer list that is too short, eg. int a[3] = { 1, 2 }; would result in a[2] initialized with 0. However, in case of variable-length arrays it didn't know if any more items need to be added, because, well, the length is variable. Add the default binding anyway, regardless of how many actually need to be added. We don't really care how many, because the default binding covers the whole array anyway. Differential Revision: https://reviews.llvm.org/D41478 rdar://problem/35477763 llvm-svn: 321290
* Reverting r321223 and its follow-up commit because of failing bots due to ↵Aaron Ballman2017-12-201-330/+328
| | | | | | Misc/ast-dump-color.cpp. llvm-svn: 321229
* Add a printing policy to the ASTDumper.Aaron Ballman2017-12-201-328/+330
| | | | | | This allows you to dump C++ code that spells bool instead of _Bool, leaves off the elaborated type specifiers when printing struct or class names, and other C-isms. llvm-svn: 321223
* [analyzer] De-duplicate path diagnostics for each exploded graph node.Artem Dergachev2017-12-201-0/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | The bugreporter::trackNullOrUndefValue() mechanism contains a system of bug reporter visitors that recursively call each other in order to track where a null or undefined value came from, where each visitor represents a particular tracking mechanism (track how the value was stored, track how the value was returned from a function, track how the value was constrained to null, etc.). Each visitor is only added once per value it needs to track. Almost. One exception from this rule would be FindLastStoreBRVisitor that has two operation modes: it contains a flag that indicates whether null stored values should be suppressed. Two instances of FindLastStoreBRVisitor with different values of this flag are considered to be different visitors, so they can be added twice and produce the same diagnostic twice. This was indeed the case in the affected test. With the current logic of this whole machinery, such duplication seems unavoidable. We should be able to safely add visitors with different flag values without constructing duplicate diagnostic pieces. Hence the effort in this commit to de-duplicate diagnostics regardless of what visitors have produced them. Differential Revision: https://reviews.llvm.org/D41258 llvm-svn: 321135
* [analyzer] trackNullOrUndefValue: always track through parentheses and casts.Artem Dergachev2017-12-201-6/+2
| | | | | | | | | | | | | | | When trying to figure out where a null or undefined value came from, parentheses and cast expressions are either completely irrelevant, or, in the case of lvalue-to-rvale cast, straightforwardly lead us in the right direction when we remove them. There is a regression that causes a certain diagnostic to appear twice in the path-notes.cpp test (changed to FIXME). It would be addressed in the next commit. Differential revision: https://reviews.llvm.org/D41254 llvm-svn: 321133
OpenPOWER on IntegriCloud