Jordan Rose [Fri, 24 Aug 2012 16:34:31 +0000 (16:34 +0000)]
[analyzer] If we dereference a NULL that came from a function, show the return.
More generally, any time we try to track where a null value came from, we
should show if it came from a function. This usually isn't necessary if
the value is symbolic, but if the value is just a constant we previously
just ignored its origin entirely. Now, we'll step into the function and
recursively add a visitor to the returned expression.
James Dennett [Fri, 24 Aug 2012 06:59:51 +0000 (06:59 +0000)]
Allow RecursiveASTVisitor to visit CXXCtorInitializer objects for which
isWritten() returns false, if shouldVisitImplicitCode() returns true.
Previously those CXXCtorInitializers were always skipped.
In order to make this change easier to test, this patch also extends the
test class template ExpectedLocationVisitor to support arbitrary numbers
of expected matches and disallowed matches.
Ted Kremenek [Fri, 24 Aug 2012 06:49:34 +0000 (06:49 +0000)]
Go ahead and show experimental checkers in the scan-build "-h" output.
They are labeled as not being enabled-by-default, and how else
are users going to test them.
Daniel Jasper [Fri, 24 Aug 2012 05:12:34 +0000 (05:12 +0000)]
Rename the ASTMatchers to better match AST nodes. Now, all
ASTMatchers have the same name as the corresponding AST nodes
but are lower case. The only exceptions are the "CXX" prefixes
which are not copied over to the matcher names as the goal is to
actually remove these prefixes from the AST node names.
Anna Zaks [Fri, 24 Aug 2012 01:39:13 +0000 (01:39 +0000)]
[analyzer] Remove unnecessary code.
This code has been added a while ago and removing it does not trigger
any test failures. The false positives it was trying to suppress are
probably handled by other logic (ex: special handling of delegates).
Richard Smith [Fri, 24 Aug 2012 00:54:33 +0000 (00:54 +0000)]
New -fcatch-undefined-behavior features:
* when checking that a pointer or reference refers to appropriate storage for a type, also check the alignment and perform a null check
* check that references are bound to appropriate storage
* check that 'this' has appropriate storage in member accesses and member function calls
Anna Zaks [Fri, 24 Aug 2012 00:06:12 +0000 (00:06 +0000)]
[analyzer] Make analyzer less aggressive when dealing with [self init].
With inlining, retain count checker starts tracking 'self' through the
init methods. The analyser results were too noisy if the developer
did not follow 'self = [super init]' pattern (which is common
especially in older code bases) - we reported self init anti-pattern AND
possible use-after-free. This patch teaches the retain count
checker to assume that [super init] does not fail when it's not consumed
by another expression. This silences the retain count warning that warns
about possibility of use-after-free when init fails, while preserving
all the other checking on 'self'.
Jordan Rose [Thu, 23 Aug 2012 23:16:34 +0000 (23:16 +0000)]
[scan-build] Accept -fno-objc-arc as well as -fobjc-arc.
This is how Xcode lets individual files be marked as non-ARC when the rest
of the project is ARC-enabled, so this is necessary for scan-build xcodebuild.
Jordan Rose [Thu, 23 Aug 2012 23:01:43 +0000 (23:01 +0000)]
[analyzer] For now, treat pointers-to-members as non-null void * symbols.
Until we have full support for pointers-to-members, we can at least
approximate some of their use by tracking null and non-null values.
We thus treat &A::m_ptr as a non-null void * symbol, and MemberPointer(0)
as a pointer-sized null constant.
This enables support for what is sometimes called the "safe bool" idiom,
demonstrated in the test case.
Jordan Rose [Thu, 23 Aug 2012 23:01:39 +0000 (23:01 +0000)]
[analyzer] Handle UserDefinedConversion casts in C++.
This is trivial; the UserDefinedConversion always wraps a CXXMemberCallExpr
for the appropriate conversion function, so it's just a matter of
propagating that value to the CastExpr itself.
Dmitri Gribenko [Thu, 23 Aug 2012 22:40:40 +0000 (22:40 +0000)]
Attaching comments to decls: since it was decided that Decl::isImplicit should
not be set for implicit instantiations, remove the FIXME. This should be the
last bit for PR13634. The actual fix happened in r162238.
Motivation: it might be misleading to mark implicit instantiations as
Decl::isImplicit = true. Because then, in order to be consistent, we should
mark all instantiated members as implicit. But the user did actually type the
declaration for the member, but the compiler played with it a little bit.
Chad Rosier [Thu, 23 Aug 2012 21:55:11 +0000 (21:55 +0000)]
[ms-inline asm] Add a few helper function to the MSAsmStmt class that are needed
by CodeGen.
In the long-term, much of the codegen logic will be shared between the GNU-style
and MS-style inline assembly, but for now I'm replicating this logic to avoid
regressions with the GNU-style.
Ted Kremenek [Thu, 23 Aug 2012 20:46:57 +0000 (20:46 +0000)]
Change a bunch of cases where we do "getAs<...>->doSomething()" to
"castAs<...>->doSomething()". The analyzer was flagging these
as potential null dereferences, which is technically true. The
invariants appear to be that these casts should never fail, so
let's use castAs<> instead and avoid a runtime check.
Richard Smith [Thu, 23 Aug 2012 20:19:14 +0000 (20:19 +0000)]
When disambiguating an expression-statement from a declaraton-statement, if the
statement starts with an identifier for which name lookup will fail either way,
look at later tokens to disambiguate in order to improve error recovery.
Jordan Rose [Thu, 23 Aug 2012 18:10:53 +0000 (18:10 +0000)]
[analyzer] Support C++ default arguments if they are literal values.
A CXXDefaultArgExpr wraps an Expr owned by a ParmVarDecl belonging to the
called function. In general, ExprEngine and Environment ought to treat this
like a ParenExpr or other transparent wrapper expression, with the inside
expression evaluated first.
However, if we call the same function twice, we'd produce a CFG that contains
the same wrapped expression twice, and we're not set up to handle that. I've
added a FIXME to the CFG builder to come back to that, but meanwhile we can
at least handle expressions that don't need to be explicitly evaluated:
literals. This probably handles many common uses of default parameters:
true/false, null, etc.
Douglas Gregor [Thu, 23 Aug 2012 05:05:18 +0000 (05:05 +0000)]
array_pod_sort on the addresses of declaration pointers leads to
inconsistent ordering of results; instead, use use SmallPtrSet to
eliminate duplicates.
Richard Smith [Thu, 23 Aug 2012 04:53:18 +0000 (04:53 +0000)]
Fix undefined behavior: don't call ObjCInterfaceDecl::isArcWeakrefUnavailable
on a null pointer. (This function happens to work for a null 'this' pointer, so
no test.)
Eli Friedman [Thu, 23 Aug 2012 03:10:17 +0000 (03:10 +0000)]
Use the alignment from lvalue emission to more accurately compute the alignment
of a pointer for builtin emission, instead of just depending on the type of the
pointee. <rdar://problem/11314941>.
Dmitri Gribenko [Wed, 22 Aug 2012 22:56:08 +0000 (22:56 +0000)]
Comment parsing: parse "<blah" as an HTML tag only if "blah" is a known tag
name. This should reduce the amount of warning false positives about bad HTML
in comments when the comment author intended to put a reference to a template.
This change will also enable us parse the comment as intended in these cases.
The checker adds assumptions that the return values from the known APIs
are non-nil. Teach the checker about NSArray/NSMutableArray/NSOrderedSet
objectAtIndex, objectAtIndexedSubscript.
Chad Rosier [Wed, 22 Aug 2012 20:30:58 +0000 (20:30 +0000)]
[ms-inline asm] Compute the token range for each instruction within the asm.
Eventually, we'll need a way of mapping tokens (and their IdentifierInfo*) to
the operands computed by buildMSAsmPieces().
Ted Kremenek [Wed, 22 Aug 2012 19:47:13 +0000 (19:47 +0000)]
Remove BasicConstraintManager. It hasn't been in active service for a while.
As part of this change, I discovered that a few of our tests were not testing
the RangeConstraintManager. Luckily all of those passed when I moved them
over to use that constraint manager.
Benjamin Kramer [Wed, 22 Aug 2012 18:50:01 +0000 (18:50 +0000)]
Make ceil/floor/nearbyint/rint/round const even with -fmath-errno.
The conditions described by POSIX can never happen with IEEE-754 floats.
When the function is const we can emit a single sse4.1 instruction for
it, without losing anything :)
Benjamin Kramer [Wed, 22 Aug 2012 18:16:02 +0000 (18:16 +0000)]
Math builtin definition tweaks.
There were missed optimizations when the system headers didn't have attributes
in place, specifically:
- Add copysign, exp2, log2, nearbyint, rint and trunc to the list.
These are functions that get inlined by LLVM's optimizer, but only when they
have the right attributes.
- Mark copysign, fabs, fmax, fmin and trunc const unconditionally.
Previously these were only const with -fno-math-errno, but they never set
errno per POSIX.
For ceil/floor/nearbyint/round I'm not aware of any implementation that sets
errno, but POSIX says it may signal overflow so I left them alone for now.
Jordan Rose [Wed, 22 Aug 2012 17:13:22 +0000 (17:13 +0000)]
[analyzer] Per feedback, re-structure the docs for ExprInspection checks.
Also, remove the FIXME about merging -analyzer-stats and the debug.Stats
checker. This would be a bad idea because simply running debug.Stats can
affect the output of -analyzer-stats.
Ted Kremenek [Wed, 22 Aug 2012 01:20:05 +0000 (01:20 +0000)]
Review, comment, and reformat IPA.txt, including feedback comments.
Formatting includes:
- removing line wraps (Emacs Cmd-Q), to make text easier to read
- provide useful indentation
- call out caveats and notes more explictly
Stylistically, I prefer the document talk in 3rd person instead of "we". The
term "we" is unambiguous, and sometimes refers to different things. I've passed
over the existing paragraphs and made them speak more about specific entities
that compose the analyzer and what they do (e.g., ExprEngine) instead of "we"
referring to the analyzer.
Further, I have substituted some vague concepts such as "state" or "program
state" and replaced them with their precise implementation counterparts (e.g.,
ProgramState). This makes the document more technically precise throughout the
entire narrative, which would sometimes use vague terms and other times precise
terms.
I've placed several comments within the document, which can be seen with
***TMK/COMMENT***, which indicate places that need to be enhanced or clarified,
or called out as questions about intended bheavior.
Chad Rosier [Tue, 21 Aug 2012 23:09:21 +0000 (23:09 +0000)]
[ms-inline asm] Remove the patchMSAsmStrings function. After some discussion
between Bob, Jim, Eric and I, we've decided to take a slightly different
approach.
Chad Rosier [Tue, 21 Aug 2012 21:56:39 +0000 (21:56 +0000)]
[ms-inline asm] Have buildMSAsmString build a vector of unmodified AsmStrings.
Add a new static function, buildMSAsmPieces, that will break these strings down
into mnemonic and operands. Upon a match failure, the idea is to use the
ErrorInfo from MatchInstructionImpl to inspect the mnemonic/operand and
decide a course of action. Unfortunately, there's no easy way to test this at
the moment.
objective-C: Change rules for overriding properties in
class extensions a little. clang now allows readonly property
with no ownership rule (assign, unsafe_unretained, weak, retain,
strong, or copy) with a readwrite property with an ownership rule.
// rdar://12103400
Jordan Rose [Tue, 21 Aug 2012 21:44:21 +0000 (21:44 +0000)]
[analyzer] Set the default IPA mode to 'basic-inlining', which excludes C++.
Under -analyzer-ipa=basic-inlining, only C functions, blocks, and C++ static
member functions are inlined -- essentially, the calls that behave like simple
C function calls. This is essentially the behavior in Xcode 4.4.
C++ support still has some rough edges, and we don't want users to be worried
about them if they download and run their own checker. (In particular, the
massive number of false positives for analyzing LLVM comes from inlining
defensively-written code in contexts where more aggressive assumptions are
implicitly made. This problem is not unique to C++, but it is exacerbated by
the higher proportion of code that lives in header files in C++.)
The eventual goal is to be comfortable enough with C++ support (and simple
Objective-C support) to advance to -analyzer-ipa=inlining as the default
behavior. See the IPA design notes for more details.
Jordan Rose [Tue, 21 Aug 2012 21:44:07 +0000 (21:44 +0000)]
[analyzer] -analyzer-ipa=inlining is now the default. Remove it from tests.
The actual change here is a little more complicated than the summary above.
What we want to do is have our generic inlining tests run under whatever
mode is the default. However, there are some tests that depend on the
presence of C++ inlining, which still has some rough edges. These tests have
been explicitly marked as -analyzer-ipa=inlining in preparation for a new
mode that limits inlining to C functions and blocks. This will be the
default until the false positives for C++ have been brought down to
manageable levels.
Jordan Rose [Tue, 21 Aug 2012 20:52:19 +0000 (20:52 +0000)]
[analyzer] Push "references are non-null" knowledge up to the common parent.
This reduces duplication across the Basic and Range constraint managers, and
keeps their internals free of dealing with the semantics of C++. It's still
a little unfortunate that the constraint manager is dealing with this at all,
but this is pretty much the only place to put it so that it will apply to all
symbolic values, even when embedded in larger expressions.
projects / clang / log