Jordan Rose [Thu, 18 Apr 2013 16:33:46 +0000 (16:33 +0000)]
[analyzer] "Force" LazyCompoundVals on bind when they are simple enough.
The analyzer uses LazyCompoundVals to represent rvalues of aggregate types,
most importantly structs and arrays. This allows us to efficiently copy
around an entire struct, rather than doing a memberwise load every time a
struct rvalue is encountered. This can also keep memory usage down by
allowing several structs to "share" the same snapshotted bindings.
However, /lookup/ through LazyCompoundVals can be expensive, especially
since they can end up chaining back to the original value. While we try
to reuse LazyCompoundVals whenever it's safe, and cache information about
this transitivity, the fact is it's sometimes just not a good idea to
perpetuate LazyCompoundVals -- the tradeoffs just aren't worth it.
This commit changes RegionStore so that binding a LazyCompoundVal to struct
will do a memberwise copy if the struct is simple enough. Today's definition
of "simple enough" is "up to N scalar members" (see below), but that could
easily be changed in the future. This is enough to bring the test case in
PR15697 back down to a manageable analysis time (within 20% of its original
time, in an unfair test where the new analyzer is not compiled with LTO).
The actual value of "N" is controlled by a new -analyzer-config option,
'region-store-small-struct-limit'. It defaults to "2", meaning structs with
zero, one, or two scalar members will be considered "simple enough" for
this code path.
It's worth noting that a more straightforward implementation would do this
on load, not on bind, and make use of the structure we already have for this:
CompoundVal. A long time ago, this was actually how RegionStore modeled
aggregate-to-aggregate copies, but today it's only used for compound literals.
Unfortunately, it seems that we've special-cased LazyCompoundVal in certain
places (such as liveness checks) but failed to similarly special-case
CompoundVal in all of them. Until we're confident that CompoundVal is
handled properly everywhere, this solution is safer, since the entire
optimization is just an implementation detail of RegionStore.
Jordan Rose [Thu, 18 Apr 2013 16:33:40 +0000 (16:33 +0000)]
[analyzer] Don't crash if we cache out after making a temporary region.
A C++ overloaded operator may be implemented as an instance method, and
that instance method may be called on an rvalue object, which has no
associated region. The analyzer handles this by creating a temporary region
just for the evaluation of this call; however, it is possible that /by
creating the region/, the analyzer ends up in a previously-explored state.
In this case we don't need to continue along this path.
This doesn't actually show any behavioral change now, but it starts being
used with the next commit and prevents an assertion failure there.
Richard Trieu [Thu, 18 Apr 2013 01:04:37 +0000 (01:04 +0000)]
Switch the note order for -Woverloaded-shift-op-parentheses so that the note
with the silence fix-it comes first. This is more consistent with the rest
of the warnings in -Wparentheses.
Anna Zaks [Thu, 18 Apr 2013 00:15:15 +0000 (00:15 +0000)]
[analyzer] Tweak getDerefExpr more to track DeclRefExprs to references.
In the committed example, we now see a note that tells us when the pointer
was assumed to be null.
This is the only case in which getDerefExpr returned null (failed to get
the dereferenced expr) throughout our regression tests. (There were multiple
occurrences of this one.)
Anna Zaks [Wed, 17 Apr 2013 22:29:47 +0000 (22:29 +0000)]
[analyzer] Allow TrackConstraintBRVisitor to work when the value it’s tracking is not live in the last node of the path
We always register the visitor on a node in which the value we are tracking is live and constrained. However,
the visitation can restart at a node, later on the path, in which the value is under constrained because
it is no longer live. Previously, we just silently stopped tracking in that case.
[Modules] Use global index to improve typo correction performance
Typo correction for an unqualified name needs to walk through all of the identifier tables of all modules.
When we have a global index, just walk its identifier table only.
[document parsing]: When tag declaration (but not definition!)
is part of the decl-specifier-seq of some other declaration,
it doesn't get comment. // rdar://12390371
Jordan Rose [Wed, 17 Apr 2013 19:09:18 +0000 (19:09 +0000)]
Fix off-by-one error in #pragma clang system_header.
The system_header pragma (from GCC) is implemented using line notes in the
source manager. However, a line note's line number specifies the number
not for the current line, but for the next line. This was making all
line numbers appear off by one after the pragma.
Jordan Rose [Wed, 17 Apr 2013 18:03:48 +0000 (18:03 +0000)]
[analyzer] Don't warn for returning void expressions in void blocks.
This was slightly tricky because BlockDecls don't currently store an
inferred return type. However, we can rely on the fact that blocks with
inferred return types will have return statements that match the inferred
type.
Unified token breaking logic: support for line comments.
Summary:
Added BreakableLineComment, moved common code from
BreakableBlockComment to newly added BreakableComment. As a side-effect of the
rewrite, found another problem with escaped newlines and had to change
code which removes trailing whitespace from line comments not to break after
this patch.
Also,
- abstract out the indirect/in memory/in registers decisions into the CGCXXABI
- fix handling of empty struct arguments for '-cxx-abi microsoft'
- add/fix tests
Andy Gibbs [Wed, 17 Apr 2013 08:06:46 +0000 (08:06 +0000)]
Extended VerifyDiagnosticConsumer to also verify source file for diagnostic.
VerifyDiagnosticConsumer previously would not check that the diagnostic and
its matching directive referenced the same source file. Common practice was
to create directives that referenced other files but only by line number,
and this led to problems such as when the file containing the directive
didn't have enough lines to match the location of the diagnostic in the
other file, leading to bizarre file formatting and other oddities.
This patch causes VerifyDiagnosticConsumer to match source files as well as
line numbers. Therefore, a new syntax is made available for directives, for
example:
This extends the @line feature where "file" is the file where the diagnostic
is generated. The @line syntax is still available and uses the current file
for the diagnostic. "file" can be specified either as a relative or absolute
path - although the latter has less usefulness, I think! The #include search
paths will be used to locate the file and if it is not found an error will be
generated.
The new check is not optional: if the directive is in a different file to the
diagnostic, the file must be specified. Therefore, a number of test-cases
have been updated with regard to this.
Eric Christopher [Wed, 17 Apr 2013 07:19:56 +0000 (07:19 +0000)]
Add a bit of a hack to deal with a failing testcase on darwin10 bots.
We currently emit an error message when you try to use thread local
storage on targets that don't support it and testing C++11 thread
locals will trip this. We don't want to xfail the test for all darwin
hosts so add a quick hack to check for darwin10 and disable the
test based on that. Only checking darwin10 because anything earlier
is really old and I don't have a list of what other hosts don't
support tls handy.
Jordan Rose [Wed, 17 Apr 2013 00:57:39 +0000 (00:57 +0000)]
[analyzer] Merge C++ status page into Open Projects.
Also, add a few random extra open projects.
Most of C++ support is done; we don't need the status page anymore. We're
hoping that the C++-related open projects are the only major pieces of
functionality we don't model at this point.
Use the extra info in global method pool to speed up looking for ObjC overridden methods.
When we are in a implementation, we check the global method pool whether there were category
methods with the same selector. If there were none (common case) we don't need to do lookups for
overridden methods again.
Note that for an interface method (if we don't encounter its implementation), it is considered that
it overrides methods that were declared before it, not for category methods introduced after it.
This is tradeoff in favor of performance, since it is expensive to do lookups in case there was a
category, and moving the global method pool to ASTContext (so we can check it) would increase complexity.
Enhance the ObjC global method pool to record whether there were 0, 1, or >= 2 methods (with a particular selector) inside categories.
This is done by extending ObjCMethodList (which is only used by the global method pool) to have 2 extra bits of information.
We will later take advantage of this info in global method pool for the overridden methods calculation.
This is an opt-in tweak for leak diagnostics to reference the allocation
site if the diagnostic consumer only wants a pithy amount of information,
and not the entire path.
This is a strawman enhancement that I expect to see some experimentation
with over the next week, and can go away if we don't want it.
Currently it is only used by RetainCountChecker, but could be used
by MallocChecker if and when we decide this should stay in.
John McCall [Tue, 16 Apr 2013 21:29:40 +0000 (21:29 +0000)]
objc_autoreleasePoolPop() can throw if a -dealloc does.
Model it as throwing so that the exception can be caught.
This is generally not expected to have significant code-size
impact because the contents of the @autoreleasepool block
are very likely to contain a call, very likely at the same
cleanup level as the @autoreleasepool itself.
This patch causes OpInst records to be silently identified with their Non-Op
inst counterparts so that the same test generation infrastructure can be used to
generate tests.
Factor CheckerManager to be able to pass AnalyzerOptions to checkers
during checker registration. There are no immediate clients of this,
but this provides a way for checkers to query the options table
at startup instead.
Remove unused "getConfig()" method. A new way is to have high-level
APIs that access the configuration table without clients reasoning
about the string table. The string table is an implementation
detail.
Changes necessary to arm_neon.td for the generation of Neon tests.
This is the first of six patches to add to the arm neon tablegen
generator the capability of generating tests to verify that the various
ARM intrinsics are implemented properly.
The changes include such items as:
1. Adding attributes to the Inst record so that additional metadata that is only
needed for the tests can be specified in TableGen.
2. Adding wrapper classes for operator (i.e. ``Op'') intrinsics which before
were simply notates as Inst. This allows us to classify what sort of test
to generate for said intrinsic and further since the classes do not effect
the behavior of the Inst base class, allow for normal functioning.
Benjamin Kramer [Tue, 16 Apr 2013 19:08:41 +0000 (19:08 +0000)]
rewrite-includes: Rewrite __has_include(_next) to get rid of a host dependency.
This broke e.g. compiling a crash report from a glibc system on Darwin. Sadly,
the implementation had to game the lexer a lot as we're not using a real
preprocessor here. It also doesn't handle special cases like arbitrary macros in
__has_include, but since this macro isn't common outside of clang's headers we
can get away with that.
This patch implements parsing ‘#pragma clang __debug’ as a first step for
implementing captured statements. Captured statements are a mechanism for
doing outlining in the AST.
see http://lists.cs.uiuc.edu/pipermail/cfe-dev/2013-January/027540.html.
Daniel Dunbar [Tue, 16 Apr 2013 18:21:19 +0000 (18:21 +0000)]
[Modules] Convert module specific -fno-modules-autolink into -fno-autolink.
- There is no reason to have a modules specific flag for disabling
autolinking. Instead, convert the existing flag into -fno-autolink (which
should cover other autolinking code generation paths like #pragmas if and
when we support them).
Objective-C IRGen. Use llvm::WeakVH
for caching couple of global symbols used
for generation of CF/NS string meta-data
so they are not released prematuely in certain
corner cases. // rdar:// 13598026.
Reviewed by John M.
Anna Zaks [Mon, 15 Apr 2013 22:38:07 +0000 (22:38 +0000)]
[analyzer] Do not crash when processing binary "?:" in C++
When computing the value of ?: expression, we rely on the last expression in
the previous basic block to be the resulting value of the expression. This is
not the case for binary "?:" operator (GNU extension) in C++. As the last
basic block has the expression for the condition subexpression, which is an
R-value, whereas the true subexpression is the L-value.
Note the operator evaluation just happens to work in C since the true
subexpression is an R-value (like the condition subexpression). CFG is the
same in C and C++ case, but the AST nodes are different, which the LValue to
Rvalue conversion happening after the BinaryConditionalOperator evaluation.
Changed the logic to only use the last expression from the predecessor only
if it matches either true or false subexpression. Note, the logic needed
fortification anyway: L and R were passed but not even used by the function.
Also, change the conjureSymbolVal to correctly compute the type, when the
expression is an LG-value.
Jordan Rose [Mon, 15 Apr 2013 22:03:38 +0000 (22:03 +0000)]
[analyzer] Don't assert on a temporary of pointer-to-member type.
While we don't do anything intelligent with pointers-to-members today,
it's perfectly legal to need a temporary of pointer-to-member type to, say,
pass by const reference. Tweak an assertion to allow this.
Jordan Rose [Mon, 15 Apr 2013 20:39:45 +0000 (20:39 +0000)]
[analyzer] Re-enable using global regions as a symbolic base.
Now that we're invalidating global regions properly, we want to continue
taking advantage of a particular optimization: if all global regions are
invalidated together, we can represent the bindings of each region with
a "derived region value" symbol. Essentially, this lazily links each
global region with a single symbol created at invalidation time, rather
than binding each region with a new symbolic value.
We used to do this, but haven't been for a while; the previous commit
re-enabled this code path, and this handles the fallout.
Jordan Rose [Mon, 15 Apr 2013 20:39:37 +0000 (20:39 +0000)]
[analyzer] Tests: move system functions into system header simulator files.
Some checkers ascribe different behavior to functions declared in system
headers, so when working with standard library functions it's probably best
to always have them in a standard location.
Test change only (no functionality change), but necessary for the next commit.
[PCH/test] Make test/PCH/cxx-typeid.cpp self-contained by including the relevant standard library declarations
instead of depending on a system header inclusion.
Unified token breaking logic for strings and block comments.
Summary:
Both strings and block comments are broken into lines in
breakProtrudingToken. Logic specific for strings or block comments is abstracted
in implementations of the BreakToken interface. Among other goodness, this
change fixes placement of backslashes after a block comment inside a
preprocessor directive (see removed FIXMEs in unit tests).
The code is far from being polished, and some parts of it will be changed for
line comments support.
Rafael Espindola [Mon, 15 Apr 2013 12:49:13 +0000 (12:49 +0000)]
Remove hasExternalLinkageUncached.
It was being used correctly, but it is a very dangerous API to have around.
Instead, move the logic from the filtering to when we are deciding if we should
link two decls.
Rafael Espindola [Mon, 15 Apr 2013 12:38:20 +0000 (12:38 +0000)]
Fix the storage class of method instantiations.
We keep the "as written" storage class, but that is a fuzzy concept for
instantiations. With this patch instantiations of methods of class templates
now get a storage class that is based on the semantics of isStatic(). With this
can simplify isStatic() itself.
projects / clang / log