Jordan Rose [Mon, 11 Mar 2013 21:31:46 +0000 (21:31 +0000)]
[analyzer] Look for calls along with lvalue nodes in trackNullOrUndefValue.
r176737 fixed bugreporter::trackNullOrUndefValue to find nodes for an lvalue
even if the rvalue node had already been collected. This commit extends that
to call statement nodes as well, so that if a call is contained within
implicit casts we can still track the return value.
No test case because node reclamation is extremely finicky (dependent on
how the AST and CFG are built, and then on our current reclamation rules,
and /then/ on how many nodes were generated by the analyzer core and the
current set of checkers). I consider this a low-risk change, though, and
it will only happen in cases of reclamation when the rvalue node isn't
available.
Ted Kremenek [Mon, 11 Mar 2013 20:51:52 +0000 (20:51 +0000)]
Disallow using -fmodules with -no-integrated-as.
Modules enables features such as auto-linking, and we simply do not want to
support a matrix of subtly enabled/disabled features depending on whether or
not a user is using the integrated assembler.
It isn't clear if this is the best place to do this check. For one thing,
these kind of errors are not caught by the serialized diagnostics.
Adrian Prantl [Mon, 11 Mar 2013 18:33:46 +0000 (18:33 +0000)]
Improve the caching of debuginfo Objective C interface types.
Generate forward declarations that are RAUW'd by finalize().
We thus avoid outputting the same type several times in multiple
stages of completion.
Richard Smith [Sat, 9 Mar 2013 23:30:15 +0000 (23:30 +0000)]
Handle _Pragma on a u8, u, or U string literal per the C11 specification. Also
handle raw string literals here. C++11 doesn't yet specify how they will
behave, but discussion on core suggests that we should just strip off
everything but the r-char-sequence.
Anna Zaks [Sat, 9 Mar 2013 03:23:19 +0000 (03:23 +0000)]
[analyzer] Make Suppress IDC checker aware that it might not start from the same node it was registered at
The visitor used to assume that the value it’s tracking is null in the first node it examines. This is not true.
If we are registering the Suppress Inlined Defensive checks visitor while traversing in another visitor
(such as FindlastStoreVisitor). When we restart with the IDC visitor, the invariance of the visitor does
not hold since the symbol we are tracking no longer exists at that point.
I had to pass the ErrorNode when creating the IDC visitor, because, in some cases, node N is
neither the error node nor will be visible along the path (we had not finalized the path at that point
and are dealing with ExplodedGraph.)
We should revisit the other visitors which might not be aware that they might get nodes, which are
later in path than the trigger point.
This suppresses a number of inline defensive checks in JavaScriptCore.
Jordan Rose [Sat, 9 Mar 2013 00:59:10 +0000 (00:59 +0000)]
[analyzer] Be more consistent about Objective-C methods that free memory.
Previously, MallocChecker's pointer escape check and its post-call state
update for Objective-C method calls had a fair amount duplicated logic
and not-entirely-consistent checks. This commit restructures all this to
be more consistent and possibly allow us to be more aggressive in warning
about double-frees.
New policy (applies to system header methods only):
(1) If this is a method we know about, model it as taking/holding ownership
of the passed-in buffer.
(1a) ...unless there's a "freeWhenDone:" parameter with a zero (NO) value.
(2) If there's a "freeWhenDone:" parameter (but it's not a method we know
about), treat the buffer as escaping if the value is non-zero (YES) and
non-escaping if it's zero (NO).
(3) If the first selector piece ends with "NoCopy" (but it's not a method we
know about and there's no "freeWhenDone:" parameter), treat the buffer
as escaping.
The reason that (2) and (3) don't explicitly model the ownership transfer is
because we can't be sure that they will actually free the memory using free(),
and we wouldn't want to emit a spurious "mismatched allocator" warning
(coming in Anton's upcoming patch). In the future, we may have an idea of a
"generic deallocation", i.e. we assume that the deallocator is correct but
still continue tracking the region so that we can warn about double-frees.
Patch by Anton Yartsev, with modifications from me.
John McCall [Sat, 9 Mar 2013 00:54:31 +0000 (00:54 +0000)]
Adjust the special non-C++ enum block return type inference
so that it looks through certain syntactic forms and applies
even if normal inference would have succeeded.
There is potential for source incompatibility from this
change, but overall we feel that it produces a much
cleaner and more defensible result, and the block
compatibility rules should curb a lot of the potential
for annoyance.
Jordan Rose [Fri, 8 Mar 2013 23:30:56 +0000 (23:30 +0000)]
[analyzer] Look for lvalue nodes when tracking a null pointer.
r176010 introduced the notion of "interesting" lvalue expressions, whose
nodes are guaranteed never to be reclaimed by the ExplodedGraph. This was
used in bugreporter::trackNullOrUndefValue to find the region that contains
the null or undef value being tracked.
However, the /rvalue/ nodes (i.e. the loads from these lvalues that produce
a null or undef value) /are/ still being reclaimed, and if we couldn't
find the node for the rvalue, we just give up. This patch changes that so
that we look for the node for either the rvalue or the lvalue -- preferring
the former, since it lets us fall back to value-only tracking in cases
where we can't get a region, but allowing the latter as well.
Jordan Rose [Fri, 8 Mar 2013 23:30:53 +0000 (23:30 +0000)]
[analyzer] Don't rely on finding the correct return statement for suppression.
Previously, ReturnVisitor waited to suppress a null return path until it
had found the inlined "return" statement. Now, it checks up front whether
the return value was NULL, and suppresses the warning right away if so.
We still have to wait until generating the path notes to invalidate the bug
report, or counter-suppression will never be triggered. (Counter-suppression
happens while generating path notes, but the generation won't happen for
reports already marked invalid.)
This isn't actually an issue today because we never reclaim nodes for
top-level statements (like return statements), but it could be an issue
some day in the future. (But, no expected behavioral change and no new
test case.)
Jan Wen Voung [Fri, 8 Mar 2013 22:42:02 +0000 (22:42 +0000)]
Move clang tests that depend on llvm/ADT/Statistic.h to a subdir.
The subdirectory has a lit.local.cfg that marks the tests unsupported
if llvm was built without Asserts. There will be a patch in LLVM
that disables statistics gathering when built without Asserts so
that full Release builds can be faster. Statistics can also
be enabled by building with -DLLVM_ENABLE_STATS.
Jordan Rose [Fri, 8 Mar 2013 22:25:36 +0000 (22:25 +0000)]
Sema: Preserve attributes on parameters in instantiated function templates.
This was causing correctness issues for ARC and the static analyzer when a
function template has "consumed" Objective-C object parameters (i.e.
parameters that will be released by the function before returning).
The fix is threefold:
(1) Actually copy over the attributes from old ParmVarDecls to new ones.
(2) Have Sema::BuildFunctionType only work for building FunctionProtoTypes,
which it was doing anyway. This allows us to pass an ExtProtoInfo
instead of a plain ExtInfo and several flags.
(3) Drop param attributes as part of StripImplicitInstantiation, which is
used when an implicit instantiation is followed by an explicit one.
Douglas Gregor [Fri, 8 Mar 2013 21:25:01 +0000 (21:25 +0000)]
<rdar://problem/13140795> Transform the scope type of a pseudo-destructor expression within the object scope.
We were transforming the scope type of a pseudo-destructor expression
(e.g., the first T in x->T::~T()) as a freestanding type, which meant
that dependent template specialization types here would stay dependent
even when no template parameters were named. This would eventually
mean that a dependent expression would end up in what should be
fully-instantiated ASTs, causing IRgen to assert.
HeaderDoc: Support more of HeaderDoc documentation
commands; top level tags such as @interface and
their 2nd level tags such as @coclass, etc.
// rdar://12379114
John McCall [Thu, 7 Mar 2013 21:37:17 +0000 (21:37 +0000)]
Promote atomic type sizes up to a power of two, capped by
MaxAtomicPromoteWidth. Fix a ton of terrible bugs with
_Atomic types and (non-intrinsic-mediated) loads and stores
thereto.
John McCall [Thu, 7 Mar 2013 21:37:08 +0000 (21:37 +0000)]
Change hasAggregateLLVMType, which conflates complex and
aggregate types in a profoundly wrong way that has to be
worked around in every call site, to getEvaluationKind,
which classifies and distinguishes between all of these
cases.
Also, normalize the API for loading and storing complexes.
I'm working on a larger patch and wanted to pull these
changes out, but it would have be annoying to detangle
them from each other.
Daniel Jasper [Thu, 7 Mar 2013 20:50:00 +0000 (20:50 +0000)]
Remove unncessary whitespace when triggered on empty line.
With the cursor located at "I", clang-format would not do anything to:
int a;
I
int b;
With this patch, it reduces the number of empty lines as necessary, and
removes unnecessary whitespace. It does not change/reformat "int a;" or
"int b;".
Anna Zaks [Thu, 7 Mar 2013 03:02:36 +0000 (03:02 +0000)]
[analyzer] Warn on passing a reference to null pointer as an argument in a call
Warn about null pointer dereference earlier when a reference to a null pointer is
passed in a call. The idea is that even though the standard might allow this, reporting
the issue earlier is better for diagnostics (the error is reported closer to the place where
the pointer was set to NULL). This also simplifies analyzer’s diagnostic logic, which has
to track “where the null came from”. As a consequence, some of our null pointer
warning suppression mechanisms started triggering more often.
TODO: Change the name of the file and class to reflect the new check.
Add a hasExternalStorageAsWritten helper. No functionality change.
It is possible that some of the current uses of
"getStorageClassAsWritten() == SC_Extern" should use this but I don't know
enough about SC_PrivateExtern to change and test them.
Jordan Rose [Thu, 7 Mar 2013 01:23:25 +0000 (01:23 +0000)]
[analyzer] Check for returning null references in ReturnUndefChecker.
Officially in the C++ standard, a null reference cannot exist. However,
it's still very easy to create one:
int &getNullRef() {
int *p = 0;
return *p;
}
We already check that binds to reference regions don't create null references.
This patch checks that we don't create null references by returning, either.
Anna Zaks [Wed, 6 Mar 2013 20:25:59 +0000 (20:25 +0000)]
[analyzer] IDC: Add config option; perform the idc check on first “null node” rather than last “non-null”.
The second modification does not lead to any visible result, but, theoretically, is what we should
have been looking at to begin with since we are checking if the node was assumed to be null in
an inlined function.
[PCH] When pre-validating the headers from the PCH, only validate non-system headers.
Stat'ing all the headers from the PCH to make sure they are up-to-date takes significant time.
In a particular source file (whose PCH file included Cocoa.h) from total -fsyntax-only time
12% was just stat calls. Change pre-validation to only check non-system headers.
There are some notable disadvantages:
-If a system header, that is not include-guarded, changes after the PCH was created, we will not
find it in the header info table and we will #import it, effectively #importing it twice, thus
we will emit some error due to a multiple definition and after that the "header was modified" error will likely
be emitted, for example something like:
NSDictionary.h:12:1: error: duplicate interface definition for class 'NSDictionary'
@interface NSDictionary : NSObject <NSCopying, NSMutableCopying, NSSecureCoding, NSFastEnumeration>
^
NSDictionary.h:12:12: note: previous definition is here
@interface NSDictionary : NSObject <NSCopying, NSMutableCopying, NSSecureCoding, NSFastEnumeration>
^
fatal error: file 'NSDictionary.h' has been modified since the precompiled header was built
Though we get the "header was modified" error, this is a bit confusing.
-Theoretically it is possible that such a system header will cause no errors but it will just cause an
unfortunate semantic change, though I find this rather unlikely.
The advantages:
-Reduces compilation time when using a huge PCH like the Cocoa ones
-System headers change very infrequent and when they do, users/build systems should be able to know that
re-building from scratch is needed.
[PCH] For HeaderFileInfoTrait, hash the key using size & time of the file.
Previously the hash would be the filename portion of the path, which could be
different for a filename with different case or a symbolic link with a different
name completely.
This did not actually create any issue so far because by validating all headers
in the PCH we created uniqued FileEntries based on inodes, so an #include of
a symbolic link (refering to a file from the PCH) would end up with a FileEntry
with filename same as the one recorded in the PCH.
Edwin Vane [Wed, 6 Mar 2013 17:02:57 +0000 (17:02 +0000)]
New ASTMatchers and enhancement to hasOverloadedOperatorName
Added two new narrowing matchers:
* hasMethod: aplies a matcher to a CXXRecordDecl's methods until a match is made
or there are no more methods.
* hasCanonicalType: applies a matcher to a QualType's canonicalType.
Enhanced hasOverloadedOperatorName to work on CXXMethodDecl as well as
CXXOperatorCallExpr.
Richard Smith [Wed, 6 Mar 2013 01:37:38 +0000 (01:37 +0000)]
PR15390: If a function returns a pointer to a function, that function type
can't have default arguments even though it's a parameter-declaration-clause in
a function declaration.
Jordan Rose [Tue, 5 Mar 2013 23:54:55 +0000 (23:54 +0000)]
Preprocessor: don't keep comments under -traditional-cpp.
This patch is designed for minimal intrusion into normal preprocessing
and compilation; under -E -traditional-cpp, the lexer will still
generate tok::comment nodes since it is preserving all whitespace, but
the output printer will then throw it away.
Jordan Rose [Tue, 5 Mar 2013 22:51:04 +0000 (22:51 +0000)]
Preprocessor: don't consider // to be a line comment in -E -std=c89 mode.
It's beneficial when compiling to treat // as the start of a line
comment even in -std=c89 mode, since it's not valid C code (with a few
rare exceptions) and is usually intended as such. We emit a pedantic
warning and then continue on as if line comments were enabled.
This has been our behavior for quite some time.
However, people use the preprocessor for things besides C source files.
In today's prompting example, the input contains (unquoted) URLs, which
contain // but should still be preserved.
This change instructs the lexer to treat // as a plain token if Clang is
in C90 mode and generating preprocessed output rather than actually compiling.
Lang Hames [Tue, 5 Mar 2013 20:27:24 +0000 (20:27 +0000)]
Use ASTContext::getDeclAlign(<Field Decl>) to get the alignment of the first
field to be memcpy'd, rather instead of ASTContext::getTypeAlign(<Field Type>).
For packed structs the alignment of a field may be less than the alignment of
the field's type.
projects / clang / log