Artem Dergachev [Fri, 30 Nov 2018 03:39:58 +0000 (03:39 +0000)]
[analyzer] Nullability: Don't detect post factum violation on concrete values.
The checker suppresses warnings on paths on which a nonnull value is assumed
to be nullable. This probably deserves a warning, but it's a separate story.
Now, because dead symbol collection fires in pretty random moments,
there sometimes was a situation when dead symbol collection fired after
computing a parameter but before actually evaluating call enter into the
function, which triggered the suppression when the argument was null
in the first place earlier than the obvious warning for null-to-nonnull
was emitted, causing false negatives.
Only trigger the suppression for symbols, not for concrete values.
It is impossible to constrain a concrete value post-factum because
it is impossible to constrain a concrete value at all.
This covers all the necessary cases because by the time we reach the call,
symbolic values should be either not constrained to null, or already collapsed
into concrete null values. Which in turn happens because they are passed through
the Store, and the respective collapse is implemented as part of getSVal(),
which is also weird.
Artem Dergachev [Fri, 30 Nov 2018 03:27:50 +0000 (03:27 +0000)]
[analyzer] Fix the "Zombie Symbols" bug.
It's an old bug that consists in stale references to symbols remaining in the
GDM if they disappear from other program state sections as a result of any
operation that isn't the actual dead symbol collection. The most common example
here is:
In this example the leak were not detected previously because the symbol
disappears from the public part of the program state due to evaluating
the assignment. For that reason the checker never receives a notification
that the symbol is dead, and never reports a leak.
This patch not only causes leak false negatives, but also a number of other
problems, including false positives on some checkers.
What's worse, even though the program state contains a finite number of symbols,
the set of symbols that dies is potentially infinite. This means that is
impossible to compute the set of all dead symbols to pass off to the checkers
for cleaning up their part of the GDM.
No longer compute the dead set at all. Disallow iterating over dead symbols.
Disallow querying if any symbols are dead. Remove the API for marking symbols
as dead, as it is no longer necessary. Update checkers accordingly.
George Karpenkov [Fri, 30 Nov 2018 02:18:37 +0000 (02:18 +0000)]
[attributes] Add a family of OS_CONSUMED, OS_RETURNS and OS_RETURNS_RETAINED attributes
The addition adds three attributes for communicating ownership,
analogous to existing NS_ and CF_ attributes.
The attributes are meant to be used for communicating ownership of all
objects in XNU (Darwin kernel) and all of the kernel modules.
The ownership model there is very similar, but still different from the
Foundation model, so we think that introducing a new family of
attributes is appropriate.
The addition required a sizeable refactoring of the existing code for
CF_ and NS_ ownership attributes, due to tight coupling and the fact
that differentiating between the types was previously done using a
boolean.
George Karpenkov [Fri, 30 Nov 2018 02:17:44 +0000 (02:17 +0000)]
[analyzer] Add the type of the leaked object to the diagnostic message
If the object is a temporary, and there is no variable it binds to,
let's at least print out the object name in order to help differentiate
it from other temporaries.
Adrian Prantl [Thu, 29 Nov 2018 22:33:09 +0000 (22:33 +0000)]
[-gmodules] Honor -fdebug-prefix-map in the debug info inside PCMs.
This patch passes -fdebug-prefix-map (a feature for renaming source
paths in the debug info) through to the per-module codegen options and
adds the debug prefix map to the module hash.
[OpenMP] Add a new version of the SPMD deinit kernel function
Summary: This patch adds a new runtime for the SPMD deinit kernel function which replaces the previous function. The new function takes as argument the flag which signals whether the runtime is required or not. This enables the compiler to optimize out the part of the deinit function which are not needed.
Zhizhou Yang [Thu, 29 Nov 2018 18:52:22 +0000 (18:52 +0000)]
set default max-page-size to 4KB in lld for Android Aarch64
Summary:
This patch passes an option '-z max-page-size=4096' to lld through clang driver.
This is for Android on Aarch64 target.
The lld default page size is too large for Aarch64, which produces larger .so files and images for arm64 device targets.
In this patch we set default page size to 4KB for Android Aarch64 targets instead.
Teresa Johnson [Thu, 29 Nov 2018 17:02:59 +0000 (17:02 +0000)]
[ThinLTO] Allow importing of multiple symbols with same GUID
Summary:
The is the clang side of the fix in D55047, to handle the case where
two different modules have local variables with the same GUID because
they had the same source file name at compilation time. Allow multiple
symbols with the same GUID to be imported, and test that this case works
with the distributed backend path.
Matt Arsenault [Thu, 29 Nov 2018 15:45:05 +0000 (15:45 +0000)]
Mark __builtin_shufflevector as using custom type checking
The custom handling seems to all be implemented already.
This avoids regressions in a future patch when float vectors
are ordinarily promoted to double vectors in variadic calls.
Raphael Isemann [Thu, 29 Nov 2018 13:50:30 +0000 (13:50 +0000)]
Set MustBuildLookupTable on PrimaryContext in ExternalASTMerger
Summary:
`MustBuildLookupTable` must always be called on a primary context as we otherwise
trigger an assert, but we don't ensure that this will always happen in our code right now.
This patch explicitly requests the primary context when doing this call as this shouldn't break
anything (as calling `getPrimaryContext` on a context which is its own primary context is a no-op)
but will catch these rare cases where we somehow operate on a declaration context that is
not its own primary context.
Kristina Brooks [Thu, 29 Nov 2018 03:49:14 +0000 (03:49 +0000)]
Add Hurd target to Clang driver (2/2)
This adds Hurd toolchain support to Clang's driver in addition
to handling translating the triple from Hurd-compatible form to
the actual triple registered in LLVM.
(Phabricator was stripping the empty files from the patch so I
manually created them)
Adrian Prantl [Wed, 28 Nov 2018 21:44:06 +0000 (21:44 +0000)]
Ensure sanitizer check function calls have a !dbg location
Function calls without a !dbg location inside a function that has a
DISubprogram make it impossible to construct inline information and
are rejected by the verifier. This patch ensures that sanitizer check
function calls have a !dbg location, by carrying forward the location
of the preceding instruction or by inserting an artificial location if
necessary.
This fixes a crash when compiling the attached testcase with -Os.
Note: This reapllies r344915, modified to reuse the IRBuilder's
DebugLoc if one exists instead of picking the one from CGDebugInfo
since the latter may get reset when emitting thunks such as block
helpers in the middle of emitting another function.
Erich Keane [Wed, 28 Nov 2018 20:58:43 +0000 (20:58 +0000)]
Correct 'target' default behavior on redecl, allow forward declaration.
Declarations without the attribute were disallowed because it would be
ambiguous which 'target' it was supposed to be on. For example:
void ___attribute__((target("v1"))) foo();
void foo(); // Redecl of above, or fwd decl of below?
void ___attribute__((target("v2"))) foo();
However, a first declaration doesn't have that problem, and erroring
prevents it from working in cases where the forward declaration is
useful.
Additionally, a forward declaration of target==default wouldn't properly
cause multiversioning, so this patch fixes that.
The patch was not split since the 'default' fix would require
implementing the same check for that case, followed by undoing the same
change for the fwd-decl implementation.
Vedant Kumar [Wed, 28 Nov 2018 20:48:07 +0000 (20:48 +0000)]
[Coverage] Do not visit artificial stmts in defaulted methods (PR39822)
There is no reason to emit coverage mappings for artificial statements
contained within defaulted methods, as these statements are not visible
to users.
Only emit a mapping for the body of the defaulted method (clang treats
the text of the "default" keyword as the body when reporting locations).
This allows users to see how often the default method is called, but
trims down the coverage mapping by skipping visitation of the children
of the method.
The immediate motivation for this change is that the lexer's
getPreciseTokenLocEnd API cannot return the correct location when given
an artificial statement (with a somewhat made-up location) as an input.
Summary: Left only the constructors that are actually required, and marked the move constructors as deleted. They are not used anymore and we were never sure they've actually worked correctly.
Richard Trieu [Wed, 28 Nov 2018 04:36:31 +0000 (04:36 +0000)]
Move LoopHint.h from Sema to Parse
struct LoopHint was only used within Parse and not in any of the Sema or
Codegen files. In the non-Parse files where it was included, it either wasn't
used or LoopHintAttr was used, so its inclusion did nothing.
Zola Bridges [Tue, 27 Nov 2018 19:56:46 +0000 (19:56 +0000)]
[clang][slh] add attribute for speculative load hardening
Summary:
Resubmit this with no changes because I think the build was broken
by a different diff.
-----
The prior diff had to be reverted because there were two tests
that failed. I updated the two tests in this diff
----- Summary from Previous Diff (Still Accurate) -----
LLVM IR already has an attribute for speculative_load_hardening. Before
this commit, when a user passed the -mspeculative-load-hardening flag to
Clang, every function would have this attribute added to it. This Clang
attribute will allow users to opt into SLH on a function by function basis.
This can be applied to functions and Objective C methods.
Richard Smith [Tue, 27 Nov 2018 19:33:49 +0000 (19:33 +0000)]
Don't speculatively emit VTTs for classes unless we are able to correctly emit references to all the functions they will (directly or indirectly) reference.
Summary:
This fixes a miscompile where we'd emit a VTT for a class that ends up
referencing an inline virtual member function that we can't actually
emit a body for (because we never instantiated it in the current TU),
which in a corner case of a corner case can lead to link errors.
Balazs Keri [Tue, 27 Nov 2018 18:36:31 +0000 (18:36 +0000)]
[ASTImporter] Added Import functions for transition to new API.
Summary:
These Import_New functions should be used in the ASTImporter,
and the old Import functions should not be used. Later the
Import_New should be renamed to Import again and the old Import
functions must be removed. But this can happen only after LLDB
was updated to use the new Import interface.
This commit is only about introducing the new Import_New
functions. These are not implemented now, only calling the old
Import ones.
Fix linker option for -fprofile-arcs -ftest-coverage
Summary:
Linux toolchain accidentally added "-u__llvm_runtime_variable" when "-fprofile-arcs -ftest-coverage", this is not added when "--coverage" option is used.
Using "-u__llvm_runtime_variable" generates an empty default.profraw file while an application built with "-fprofile-arcs -ftest-coverage" is running.
This reverts commit r347035 as it introduced assertion failures under
certain conditions. More information can be found here:
https://reviews.llvm.org/rL347035
Marco Antognini [Tue, 27 Nov 2018 14:54:58 +0000 (14:54 +0000)]
Derive builtin return type from its definition
Summary:
Prior to this patch, OpenCL code such as the following would attempt to create
a BranchInst with a non-bool argument:
if (enqueue_kernel(get_default_queue(), 0, nd, ^(void){})) /* ... */
This patch is a follow up on a similar issue with pipe builtin
operations. See commit r280800 and https://bugs.llvm.org/show_bug.cgi?id=30219.
This change, while being conservative on non-builtin functions,
should set the type of expressions invoking builtins to the
proper type, instead of defaulting to `bool` and requiring
manual overrides in Sema::CheckBuiltinFunctionCall.
In addition to tests for enqueue_kernel, the tests are extended to
check other OpenCL builtins.
Hans Wennborg [Tue, 27 Nov 2018 14:01:40 +0000 (14:01 +0000)]
Revert r347417 "Re-Reinstate 347294 with a fix for the failures."
This caused a miscompile in Chrome (see crbug.com/908372) that's
illustrated by this small reduction:
static bool f(int *a, int *b) {
return !__builtin_constant_p(b - a) || (!(b - a));
}
int arr[] = {1,2,3};
bool g() {
return f(arr, arr + 3);
}
$ clang -O2 -S -emit-llvm a.cc -o -
g() should return true, but after r347417 it became false for some reason.
This also reverts the follow-up commits.
r347417:
> Re-Reinstate 347294 with a fix for the failures.
>
> Don't try to emit a scalar expression for a non-scalar argument to
> __builtin_constant_p().
>
> Third time's a charm!
r347446:
> The result of is.constant() is unsigned.
r347480:
> A __builtin_constant_p() returns 0 with a function type.
r347512:
> isEvaluatable() implies a constant context.
>
> Assume that we're in a constant context if we're asking if the expression can
> be compiled into a constant initializer. This fixes the issue where a
> __builtin_constant_p() in a compound literal was diagnosed as not being
> constant, even though it's always possible to convert the builtin into a
> constant.
r347531:
> A "constexpr" is evaluated in a constant context. Make sure this is reflected
> if a __builtin_constant_p() is a part of a constexpr.
Gabor Marton [Tue, 27 Nov 2018 09:51:36 +0000 (09:51 +0000)]
[ASTImporter] Typedef import brings in the complete type
Summary:
When we already have an incomplete underlying type of a typedef in the
"To" context, and the "From" context has the same typedef, but the
underlying type is complete, then the imported type should be complete.
Fixes an assertion in CTU analysis of Xerces:
Assertion `DD && "queried property of class with no definition"' failed.
This assert is happening in the analyzer engine, because that attempts
to query an underlying type of a typedef, which happens to be
incomplete.
LLVM IR already has an attribute for speculative_load_hardening. Before
this commit, when a user passed the -mspeculative-load-hardening flag to
Clang, every function would have this attribute added to it. This Clang
attribute will allow users to opt into SLH on a function by function
basis.
This can be applied to functions and Objective C methods.
Reid Kleckner [Tue, 27 Nov 2018 02:21:51 +0000 (02:21 +0000)]
[MS] Push fewer DeclContexts for delayed template parsing
Only push the outermost record as a DeclContext when parsing a function
body. See the comments in Sema::getContainingDC about the way the parser
pushes contexts. This is intended to match the behavior the parser
normally displays where it parses all method bodies from all nested
classes at the end of the outermost class, when all nested classes are
complete.
----- Summary from Previous Diff (Still Accurate) -----
LLVM IR already has an attribute for speculative_load_hardening. Before
this commit, when a user passed the -mspeculative-load-hardening flag to
Clang, every function would have this attribute added to it. This Clang
attribute will allow users to opt into SLH on a function by function basis.
This can be applied to functions and Objective C methods.
Zola Bridges [Mon, 26 Nov 2018 19:41:14 +0000 (19:41 +0000)]
[clang][slh] add attribute for speculative load hardening
Summary:
LLVM IR already has an attribute for speculative_load_hardening. Before
this commit, when a user passed the -mspeculative-load-hardening flag to
Clang, every function would have this attribute added to it. This Clang
attribute will allow users to opt into SLH on a function by function basis.
This can be applied to functions and Objective C methods.
Alexey Bataev [Mon, 26 Nov 2018 18:37:09 +0000 (18:37 +0000)]
[OPENMP][NVPTX]Emit default locations with the correct Exec|Runtime
modes.
If the region is inside target|teams|distribute region, we can emit the
locations with the correct info for execution mode and runtime mode.
Patch adds this ability to the NVPTX codegen to help the optimizer to
produce better code.
Nemanja Ivanovic [Mon, 26 Nov 2018 14:35:38 +0000 (14:35 +0000)]
[PowerPC] Vector load/store builtins overstate alignment of pointers
A number of builtins in altivec.h load/store vectors from pointers to scalar
types. Currently they just cast the pointer to a vector pointer, but expressions
like that have the alignment of the target type. Of course, the input pointer
did not have that alignment so this triggers UBSan (and rightly so).
This resolves https://bugs.llvm.org/show_bug.cgi?id=39704
Sanjay Patel [Sun, 25 Nov 2018 17:53:16 +0000 (17:53 +0000)]
[CodeGen] translate MS rotate builtins to LLVM funnel-shift intrinsics
This was originally part of:
D50924
and should resolve PR37387:
https://bugs.llvm.org/show_bug.cgi?id=37387
...but it was reverted because some bots using a gcc host compiler
would crash for unknown reasons with this included in the patch.
Trying again now to see if that's still a problem.
Kristof Umann [Sat, 24 Nov 2018 12:24:27 +0000 (12:24 +0000)]
[analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker
This checker implements a solution to the "INT50-CPP. Do not cast to an
out-of-range enumeration value" rule [1].
It lands in alpha for now, and a number of followup patches are planned in order
to enable it by default.
Bill Wendling [Sat, 24 Nov 2018 10:45:55 +0000 (10:45 +0000)]
isEvaluatable() implies a constant context.
Assume that we're in a constant context if we're asking if the expression can
be compiled into a constant initializer. This fixes the issue where a
__builtin_constant_p() in a compound literal was diagnosed as not being
constant, even though it's always possible to convert the builtin into a
constant.
Fangrui Song [Sat, 24 Nov 2018 00:14:31 +0000 (00:14 +0000)]
[CodeComplete] Format SemaCodeComplete.cpp and improve code consistency
There are some mis-indented places and missing spaces here and there. Just format the whole file.
Also, newer code (from 2014 onwards) in this file prefers const auto *X = dyn_cast to not repeat the Decl type name. Make other occurrences consistent.
Remove two anonymous namespaces that are not very necessary: 1) a typedef 2) a local function (should use static)
[clang-format] Do not treat asm clobber [ as ObjCExpr, refined
Summary:
r346756 refined clang-format to not treat the `[` in `asm (...: [] ..)` as an
ObjCExpr. However that's not enough, as we might have a comma-separated list of
such clobbers as in the newly added test.
This updates the detection to instead look at the Line's first token being `asm`
and not mark `[`-s as ObjCExprs in this case.
Alexey Bataev [Wed, 21 Nov 2018 21:04:34 +0000 (21:04 +0000)]
[OPENMP][NVPTX]Emit default locations as constant with undefined mode.
For the NVPTX target default locations should be emitted as constants +
additional info must be emitted in the reserved_2 field of the ident_t
structure. The 1st bit controls the execution mode and the 2nd bit
controls use of the lightweight runtime. The combination of the bits for
Non-SPMD mode + lightweight runtime represents special undefined mode,
used outside of the target regions for orphaned directives or functions.
Should allow and additional optimization inside of the target regions.