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.
John McCall [Tue, 21 Aug 2012 04:10:00 +0000 (04:10 +0000)]
When performing a trivial copy of a C++ type, we must be careful not
to overwrite objects that might have been allocated into the type's
tail padding. This patch is missing some potential optimizations where
the destination is provably a complete object, but it's necessary for
correctness.
Richard Smith [Tue, 21 Aug 2012 03:11:53 +0000 (03:11 +0000)]
Avoid binding a reference to a dereferenced pointer in diagnostic serialization
if a diagnostic is emitted outside of any source file. The fix mirrors the
corresponding code in TextDiagnosticPrinter. This required moving the
functional parts of SDiagRenderer into SDiagWriter so they can be reused in the
non-rendering codepath.
John McCall [Tue, 21 Aug 2012 02:47:43 +0000 (02:47 +0000)]
Screw around with ObjCRuntime some more, changing the
diagnostics for bad deployment targets and adding a few
more predicates. Includes a patch by Jonathan Schleifer
to enable ARC for ObjFW.
Sam Panzer [Tue, 21 Aug 2012 00:52:01 +0000 (00:52 +0000)]
Better diagnostics for range-based for loops with bad range types.
The old error message stating that 'begin' was an undeclared identifier
is replaced with a new message explaining that the error is in the range
expression, along with which of the begin() and end() functions was
problematic if relevant.
Additionally, if the range was a pointer type or defines operator*,
attempt to dereference the range, and offer a FixIt if the modified range
works.
Jordan Rose [Tue, 21 Aug 2012 00:27:33 +0000 (00:27 +0000)]
[analyzer] Assume that reference symbols are non-null.
By doing this in the constraint managers, we can ensure that ANY reference
whose value we don't know gets the effect, even if it's not a top-level
parameter.
Dmitri Gribenko [Mon, 20 Aug 2012 23:39:06 +0000 (23:39 +0000)]
DeclPrinter: add terse output mode and lots of tests
Add a flag PrintingPolicy::DontRecurseInDeclContext to provide "terse" output
from DeclPrinter. The motivation is to use DeclPrinter to print declarations
in user-friendly format, without overwhelming user with inner detail of the
declaration being printed.
Also add many tests for DeclPrinter. There are quite a few things that we
print incorrectly: search for WRONG in DeclPrinterTest.cpp -- and these tests
check our output against incorrect output, so that we can fix/refactor/rewrite
the DeclPrinter later.
John McCall [Mon, 20 Aug 2012 23:36:59 +0000 (23:36 +0000)]
Fix a pair of bugs relating to properties in ARC.
First, when synthesizing an explicitly strong/retain/copy property
of Class type, don't pretend during compatibility checking that the
property is actually assign. Instead, resolve incompatibilities
by secretly changing the type of *implicitly* __unsafe_unretained
Class ivars to be strong. This is moderately evil but better than
what we were doing.
Second, when synthesizing the setter for a strong property of
non-retainable type, be sure to use objc_setProperty. This is
possible when the property is decorated with the NSObject
attribute. This is an ugly, ugly corner of the language, and
we probably ought to deprecate it.
The first is rdar://problem/12039404; the second was noticed by
inspection while fixing the first.
Jordan Rose [Mon, 20 Aug 2012 22:15:44 +0000 (22:15 +0000)]
[analyzer] More tests for "release and stop tracking".
Under GC, a release message is ignored, so "release and stop tracking" just
becomes "stop tracking". But CFRelease is still honored. This is the main
difference between ns_consumed and cf_consumed.
Manuel Klimek [Mon, 20 Aug 2012 20:54:03 +0000 (20:54 +0000)]
Adds the most requested doc for the AST matchers: a reference
of matchers, categorized by type and fully expanded for the
context in which they can be used.
I used a script to generate this documentation which I'll want
to be scrunitized by a code review before checking it in.
Jordan Rose [Mon, 20 Aug 2012 20:01:15 +0000 (20:01 +0000)]
[analyzer] Add a test for "release and stop tracking" behavior.
This is used to handle functions and methods that consume an argument
(annotated with the ns_consumed or cf_consumed attribute), but then the
argument's retain count may be further modified in a callback. We want
to warn about over-releasing, but we can't really track the object afterwards.
Jordan Rose [Mon, 20 Aug 2012 20:01:13 +0000 (20:01 +0000)]
Code-complete 'weak' for properties under ARC-with-weak-references (or GC)
Also, suggest 'readonly' even if the property has been given an ownership
attribute ('strong', 'weak', etc). This is used when properties are declared
readonly in the public interface but readwrite in a class extension.
Jordan Rose [Mon, 20 Aug 2012 18:43:42 +0000 (18:43 +0000)]
[analyzer] Replace boolean IsSink parameters with 'generateSink' methods.
Generating a sink is significantly different behavior from generating a
normal node, and a simple boolean parameter can be rather opaque. Per
offline discussion with Anna, adding new generation methods is the
clearest way to communicate intent.
Jordan Rose [Mon, 20 Aug 2012 17:04:45 +0000 (17:04 +0000)]
[analyzer] The result of && or || is always a 1 or 0.
Forgetting to at least cast the result was giving us Loc/NonLoc problems
in SValBuilder (hitting an assertion). But the standard (both C and C++)
does actually guarantee that && and || will result in the actual values
1 and 0, typed as 'int' in C and 'bool' in C++, and we can easily model that.
Manuel Klimek [Mon, 20 Aug 2012 16:37:20 +0000 (16:37 +0000)]
Fix comments and variable naming:
- use InnerMatcher consistently, fix style violations on the way
- doxygenify code snippets across all comments
- start doxygenifying code references in text
- addeed missing Usable as: sections
Tobias Grosser [Mon, 20 Aug 2012 10:38:16 +0000 (10:38 +0000)]
[cindex.py] Cache the number of chunks in CompletionString
Without this patch, lib.clang_getNumCompletionChunks is called at
each _iteration_ of a 'for chunk in CompletionString' loop. Now we
call it just once.
Jordan Rose [Sat, 18 Aug 2012 16:58:52 +0000 (16:58 +0000)]
Allow -verify to be used with files that don't have an associated FileEntry.
In Debug builds, VerifyDiagnosticConsumer checks any files with diagnostics
to make sure we got the chance to parse them for directives (expected-warning
and friends). This check previously relied on every parsed file having a
FileEntry, which broke the cling interpreter's test suite.
This commit changes the extra debug checking to mark a file as unparsed
as soon as we see a diagnostic from that file. At the very end, any files
that are still marked as unparsed are checked for directives, and a fatal
error is emitted (as before) if we find out that there were directives we
missed. -verify directives should always live in actual parsed files, not
in PCH or AST files.
Patch by Andy Gibbs, with slight modifications by me.
John McCall [Sat, 18 Aug 2012 04:51:52 +0000 (04:51 +0000)]
When mangling a negative number, remember that negating it does not
always yield a positive number. Just print the negated result as an
unsigned number.
Richard Smith [Sat, 18 Aug 2012 00:55:03 +0000 (00:55 +0000)]
PR41111, PR5925, PR13210: Teach tentative parsing to annotate identifiers and
nested names as id-expressions, using the annot_primary_expr annotation, where
possible. This removes some redundant lookups, and also allows us to
typo-correct within tentative parsing, and to carry on disambiguating past an
identifier which we can determine will fail lookup as both a type and as a
non-type, allowing us to disambiguate more declarations (and thus offer
improved error recovery for such cases).
This also introduces to the parser the notion of a tentatively-declared name,
which is an identifier which we *might* have seen a declaration for in a
tentative parse (but only if we end up disambiguating the tokens as a
declaration). This is necessary to correctly disambiguate cases where a
variable is used within its own initializer.
Jordan Rose [Sat, 18 Aug 2012 00:30:23 +0000 (00:30 +0000)]
[analyzer] Treat C++ 'throw' as a sink.
Our current handling of 'throw' is all CFG-based: it jumps to a 'catch' block
if there is one and the function exit block if not. But this doesn't really
get the right behavior when a function is inlined: execution will continue on
the caller's side, which is always the wrong thing to do.
Even within a single function, 'throw' completely skips any destructors that
are to be run. This is essentially the same problem as @finally -- a CFGBlock
that can have multiple entry points, whose exit points depend on whether it
was entered normally or exceptionally.
Representing 'throw' as a sink matches our current (non-)handling of @throw.
It's not a perfect solution, but it's better than continuing analysis in an
inconsistent or even impossible state.
Jordan Rose [Sat, 18 Aug 2012 00:30:20 +0000 (00:30 +0000)]
[analyzer] Treat @throw as a sink (stop processing).
The CFG approximates @throw as a return statement, but that's not good
enough in inlined functions. Moreover, since Objective-C exceptions are
usually considered fatal, we should be suppressing leak warnings like we
do for calls to noreturn functions (like abort()).
The comments indicate that we were probably intending to do this all along;
it may have been inadvertantly changed during a refactor at one point.
Jordan Rose [Sat, 18 Aug 2012 00:30:16 +0000 (00:30 +0000)]
[analyzer] Remove obsolete GenericNodeBuilderRefCount from RetainCountChecker.
This was once an adapter class between callbacks that had CheckerContexts
and those that don't, but for a while now it's essentially just been a
wrapper around a ProgramPointTag. We can just pass the tag around instead.
c: privide deprecated warning when __private_extern__ storage
specifier is unsed in a declaration; as it may not make the symbol
local to linkage unit as intended. Suggest using "hidden" visibility
attribute instead. // rdar://7703982
Daniel Dunbar [Fri, 17 Aug 2012 18:43:50 +0000 (18:43 +0000)]
darwin/driver: Support using SDKROOT to define the default for -isysroot.
- The SDKROOT environment variable is the de facto way to set the default SDK
for a number of tools, join forces with them.
Douglas Gregor [Fri, 17 Aug 2012 05:40:05 +0000 (05:40 +0000)]
Make the spacing of the code completion result for NSDictionary
literals match the spacing introduced by the ObjC modernizer. Fixes
the rest of <rdar://problem/11889572>.
Douglas Gregor [Fri, 17 Aug 2012 05:26:33 +0000 (05:26 +0000)]
When we need the complete set of visible declarations from a
declaration context, check whether the primary context---not the
current context---has any external visible declarations. Fixes
PR13616.
Douglas Gregor [Fri, 17 Aug 2012 05:12:08 +0000 (05:12 +0000)]
Don't do jump-scope checking when code completion is enabled. It's
both a waste of time, and prone to crash due to the use of the
error-recovery path in parser. Fixes <rdar://problem/12103608>, which
has been driving me nuts.
Richard Smith [Fri, 17 Aug 2012 04:17:54 +0000 (04:17 +0000)]
Fix undefined behavior in debug info emission: operator* on WeakVH returns a
reference, so &* on an empty WeakVH binds a reference to a dereferenced null
pointer. So don't do that; we have a perfectly good implicit conversion to
Value*.
projects / clang / log