David Chisnall [Sun, 3 Mar 2013 16:02:42 +0000 (16:02 +0000)]
Improve C11 atomics support:
- Generate atomicrmw operations in most of the cases when it's sensible to do
so.
- Don't crash in several common cases (and hopefully don't crash in more of
them).
- Add some better tests.
We now generate significantly better code for things like:
_Atomic(int) x;
...
x++;
On MIPS, this now generates a 4-instruction ll/sc loop, where previously it
generated about 30 instructions in two nested loops. On x86-64, we generate a
single lock incl, instead of a lock cmpxchgl loop (one instruction instead of
ten).
Inlining brought a few "null pointer use" false positives, which occur because
the callee defensively checks if a pointer is NULL, whereas the caller knows
that the pointer cannot be NULL in the context of the given call.
This is a first attempt to silence these warnings by tracking the symbolic value
along the execution path in the BugReporter. The new visitor finds the node
in which the symbol was first constrained to NULL. If the node belongs to
a function on the active stack, the warning is reported, otherwise, it is
suppressed.
There are several areas for follow up work, for example:
- How do we differentiate the cases where the first check is followed by
another one, which does happen on the active stack?
Also, this only silences a fraction of null pointer use warnings. For example, it
does not do anything for the cases where NULL was assigned inside a callee.
Jordan Rose [Sat, 2 Mar 2013 00:49:47 +0000 (00:49 +0000)]
CMake: -Wno-nested-anon-types for Clang.
In LLVM, -pedantic is not set unless LLVM_ENABLE_PEDANTIC is set.
However, Clang's CMakeLists.txt unilaterally adds -pedantic to the run
line, so we need to disable -Wnested-anon-types explicitly.
Jordan Rose [Fri, 1 Mar 2013 23:03:17 +0000 (23:03 +0000)]
[analyzer] Special-case bitfields when finding sub-region bindings.
Previously we were assuming that we'd never ask for the sub-region bindings
of a bitfield, since a bitfield cannot have subregions. However,
unification of code paths has made that assumption invalid. While we could
take advantage of this by just checking for the single possible binding,
it's probably better to do the right thing, so that if/when we someday
support unions we'll do the right thing there, too.
This fixes a handful of false positives in analyzing LLVM.
comment parsing. Keep the original command format
in AST for source fidelity and use it in diagnostics
to refer to the original format. // rdar://13066276
Jordan Rose [Fri, 1 Mar 2013 19:45:10 +0000 (19:45 +0000)]
[analyzer] Suppress paths involving a reference whose rvalue is null.
Most map types have an operator[] that inserts a new element if the key
isn't found, then returns a reference to the value slot so that you can
assign into it. However, if the value type is a pointer, it will be
initialized to null. This is usually no problem.
However, if the user /knows/ the map contains a value for a particular key,
they may just use it immediately:
// From ClangSACheckersEmitter.cpp
recordGroupMap[group]->Checkers
In this case the analyzer reports a null dereference on the path where the
key is not in the map, even though the user knows that path is impossible
here. They could silence the warning by adding an assertion, but that means
splitting up the expression and introducing a local variable. (Note that
the analyzer has no way of knowing that recordGroupMap[group] will return
the same reference if called twice in a row!)
We already have logic that says a null dereference has a high chance of
being a false positive if the null came from an inlined function. This
patch simply extends that to references whose rvalues are null as well,
silencing several false positives in LLVM.
Daniel Jasper [Fri, 1 Mar 2013 16:48:32 +0000 (16:48 +0000)]
Normal indent for last element of builder-type call.
In builder type call, we indent to the laster function calls.
However, for the last element of such a call, we don't need to do
so, as that normally just wastes space and does not increase
readability.
John McCall [Fri, 1 Mar 2013 09:20:14 +0000 (09:20 +0000)]
Perform the receiver-expression transformations regardless of
whether we already have a method. Fixes a bug where we were
failing to properly contextually convert a message receiver
during template instantiation.
As a side-effect, we now actually perform correct method lookup
after adjusting a message-send to integral or non-ObjC pointer
types (legal outside of ARC).
Anna Zaks [Fri, 1 Mar 2013 06:38:16 +0000 (06:38 +0000)]
[analyzer] Reword FAQ
Reword the FAQ to stress more that the assert should be used only in case
the developer is sure that the issue is a false positive.
[PCH] Enhance InputFile to also include whether the file is out-of-date.
Previously we would return null for an out-of-date file. This inhibited ASTReader::ReadSLocEntry
from creating a FileID to recover gracefully in such a case.
objective-C: clang, following gcc, warns on
use of stand-alone protocol as type and uses
id<proto>. Modify warning to say what compiler
is doing. // rdar//13158394
John McCall [Thu, 28 Feb 2013 19:01:20 +0000 (19:01 +0000)]
Use the actual ABI-determined C calling convention for runtime
calls and declarations.
LLVM has a default CC determined by the target triple. This is
not always the actual default CC for the ABI we've been asked to
target, and so we sometimes find ourselves annotating all user
functions with an explicit calling convention. Since these
calling conventions usually agree for the simple set of argument
types passed to most runtime functions, using the LLVM-default CC
in principle has no effect. However, the LLVM optimizer goes
into histrionics if it sees this kind of formal CC mismatch,
since it has no concept of CC compatibility. Therefore, if this
module happens to define the "runtime" function, or got LTO'ed
with such a definition, we can miscompile; so it's quite
important to get this right.
Defining runtime functions locally is quite common in embedded
applications.
objective-C code completion. Property accessors may not
have their own code completion comments. Use those in
their properties in this case.
// rdar://12791315
Manuel Klimek [Thu, 28 Feb 2013 13:21:39 +0000 (13:21 +0000)]
First step towards adding a parent map to the ASTContext.
This does not yet implement the LimitNode approach discussed.
The impact of this is an O(n) in the number of nodes in the AST
reduction of complexity for certain kinds of matchers (as otherwise the
parent map gets recreated for every new MatchFinder).
See FIXMEs in the comments for the direction of future work.
Daniel Jasper [Thu, 28 Feb 2013 11:05:57 +0000 (11:05 +0000)]
Improve formatting of #defines.
Two improvements:
1) Always leave at least one space before "\". Otherwise is can look bad
and there is a risk of unwillingly joining to characters to a different
token.
2) Use the full column limit for single-line #defines.
Fixes llvm.org/PR15148
Jordan Rose [Wed, 27 Feb 2013 18:49:57 +0000 (18:49 +0000)]
[analyzer] Teach FindLastStoreBRVisitor to understand stores of the same value.
Consider this case:
int *p = 0;
p = getPointerThatMayBeNull();
*p = 1;
If we inline 'getPointerThatMayBeNull', we might know that the value of 'p'
is NULL, and thus emit a null pointer dereference report. However, we
usually want to suppress such warnings as error paths, and we do so by using
FindLastStoreBRVisitor to see where the NULL came from. In this case, though,
because 'p' was NULL both before and after the assignment, the visitor
would decide that the "last store" was the initialization, not the
re-assignment.
This commit changes FindLastStoreBRVisitor to consider all PostStore nodes
that assign to this region. This still won't catches changes made directly
by checkers if they re-assign the same value, but it does handle the common
case in user-written code and will trigger ReturnVisitor's suppression
machinery as expected.
Jordan Rose [Wed, 27 Feb 2013 18:49:43 +0000 (18:49 +0000)]
[analyzer] Turn on C++ constructor inlining by default.
This enables constructor inlining for types with non-trivial destructors.
The plan is to enable destructor inlining within the next month, but that
needs further verification.
Chad Rosier [Wed, 27 Feb 2013 18:46:04 +0000 (18:46 +0000)]
[driver] The failure of any phase (e.g., preprocess, compile, assemble) for a
single translation unit should prevent later phases from executing. Otherwise,
this generates lots of noise in build systems. This a fallout from r173825.
Patch by Matthew Curtis <mcurtis@codeaurora.org>.
rdar://13298009
Simon Atanasyan [Wed, 27 Feb 2013 14:55:49 +0000 (14:55 +0000)]
[Mips] Add two new aliases for MIPS ABI names 32 (means o32 abi) and 64
(means n64 abi) to improve compatibility with GNU tools.
Patch by Jia Liu <proljc@gmail.com>.
Alexey Samsonov [Wed, 27 Feb 2013 11:14:55 +0000 (11:14 +0000)]
[Sanitizer] Change driver behavior when linking with -fsanitize=thread and -fsanitize=memory. TSan/MSan also provide their versions of new/delete and should use the same strategy as ASan. Share the code that sets linker flags for all sanitizers.
Daniel Jasper [Wed, 27 Feb 2013 09:47:53 +0000 (09:47 +0000)]
Enable bin-packing in Google style.
After some discussions, it seems that this is the better path in
the long run. Does not change Chromium style, as there, bin packing
is forbidden by the style guide.
Also fix two minor bugs wrt. formatting:
1. If a call parameter is a function call itself and is split before
the "." or "->", split before the next parameter.
2. If a call parameter is string literal that has to be split onto
two lines, split before the next parameter.
Rafael Espindola [Wed, 27 Feb 2013 04:15:01 +0000 (04:15 +0000)]
Don't cache the visibility of types.
Since r175326 an implicitly hidden template argument can cause a template
installation to become hidden, even if the template itself has an explicit
default visibility. This requires that we keep track of "late" additions
of the visibility attribute.
This is hopefully the last followup change. It just removes the caching of
visibilities from types so that we can see new attributes even after a type has
been used.
Lang Hames [Wed, 27 Feb 2013 04:14:49 +0000 (04:14 +0000)]
Use the correct alignment for POD-member memcpys where the first field is a
bitfield. CGBitField::StorageAlignment holds the alignment in chars, but
emitMemcpy had been treating it as if it were held in bits, leading to
underaligned memcpys.
Ted Kremenek [Wed, 27 Feb 2013 01:26:58 +0000 (01:26 +0000)]
[analyzer] Add stop-gap patch to prevent assertion failure when analyzing LLVM codebase.
This potentially reduces a performance optimization of throwing away
PreStmtPurgeDeadSymbols nodes. I'll investigate the performance impact
soon and see if we need something better.
comment parsing: Properties are considered like methods, and people
think of them as having return values that may be computed. Don't
warn when using @return in their comment. // rdar://13189938
Jordan Rose [Wed, 27 Feb 2013 00:05:29 +0000 (00:05 +0000)]
[analyzer] If a struct has a partial lazy binding, its fields aren't Undef.
This is essentially the same problem as r174031: a lazy binding for the first
field of a struct may stomp on an existing default binding for the
entire struct. Because of the way RegionStore is set up, we can't help
but lose the top-level binding, but then we need to make sure that accessing
one of the other fields doesn't come back as Undefined.
In this case, RegionStore is now correctly detecting that the lazy binding
we have isn't the right type, but then failing to follow through on the
implications of that: we don't know anything about the other fields in the
aggregate. This fix adds a test when searching for other kinds of default
values to see if there's a lazy binding we rejected, and if so returns
a symbolic value instead of Undefined.
The long-term fix for this is probably a new Store model; see
<rdar://problem/12701038>.
Ted Kremenek [Wed, 27 Feb 2013 00:00:26 +0000 (00:00 +0000)]
Refine SourceManager's isBeforeInTranslationUnit() cache to have more entries.
isBeforeInTranslationUnit() uses a cache to reduce the expensive work
to compute a common ancestor for two FileIDs. This work is very
expensive, so even caching the latest used FileIDs was a big win.
A closer analysis of the cache before, however, shows that the cache
access pattern would oscillate between a working set of FileIDs, and
thus caching more pairs would be profitable.
This patch adds a side table for extending caching. This side table
is bounded in size (experimentally determined in this case from
a simple Objective-C project), and when the table gets too large
we fall back to the single entry caching before as before.
On Sketch (a small example Objective-C project), this optimization
reduces -fsyntax-only time on SKTGraphicView.m by 5%. This is
for a project that is already using PCH.
Along the way, shorten path diagnostics that say "Variable 'x'" to just
be "'x'". By the context, it is obvious that we have a variable,
and so this just consumes text space.
Rafael Espindola [Tue, 26 Feb 2013 19:33:14 +0000 (19:33 +0000)]
Use the most recent decl in getExplicitVisibility.
Now that implicitly hidden template arguments can make an instantiation hidden,
it is important to look at more than just the canonical decl of the argument
in order to see if an attribute is available in a more recent decl.
This has the disadvantage of exposing when getExplicitVisibility is called,
but lets us handle cases like
template <typename T>
struct __attribute__((visibility("default"))) barT {
static void zed() {}
};
class foo;
class __attribute__((visibility("default"))) foo;
template struct barT<foo>;
Daniel Jasper [Tue, 26 Feb 2013 13:10:34 +0000 (13:10 +0000)]
Only keep empty lines in unwrapped lines if they preceed a line comment.
Empty lines followed by line comments are often used to highlight the
comment. Empty lines somewhere else are usually left over from manual or
automatic formatting and should probably be removed.
projects / clang / log