[analyzer] Diagnose stack leaks via block captures
This diff extends StackAddrEscapeChecker
to catch stack addresses leaks via block captures
if the block is executed asynchronously or
returned from a function.
Zachary Turner [Mon, 20 Nov 2017 21:41:36 +0000 (21:41 +0000)]
Resubmit "Refactor debuginfo-tests" again.
This was reverted due to the tests being run twice on some
build bots. Each run had a slightly different configuration
due to the way in which it was being invoked. This fixes
the problem (albeit in a somewhat hacky way). Hopefully in
the future we can get rid of the workflow of running
debuginfo-tests as part of clang, and then this hack can
go away.
Hans Wennborg [Mon, 20 Nov 2017 17:48:54 +0000 (17:48 +0000)]
Revert r318456 "Issue -Wempty-body warnings for else blocks"
This caused warnings also when the if or else comes from macros. There was an
attempt to fix this in r318556, but that introduced new problems and was
reverted. Reverting this too until the whole issue is sorted.
> This looks like it was just an oversight.
>
> Fixes http://llvm.org/pr35319
>
> git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@318456 91177308-0d34-0410-b5e6-96231b3b80d8
Hans Wennborg [Mon, 20 Nov 2017 17:38:16 +0000 (17:38 +0000)]
Revert r318556 "Loosen -Wempty-body warning"
It seems this somehow made -Wempty-body fire in some macro cases where
it didn't before, e.g.
../../third_party/ffmpeg/libavcodec/bitstream.c(169,5): error: if statement has empty body [-Werror,-Wempty-body]
ff_dlog(NULL, "new table index=%d size=%d\n", table_index, table_size);
^
../../third_party/ffmpeg\libavutil/internal.h(276,80): note: expanded from macro 'ff_dlog'
# define ff_dlog(ctx, ...) do { if (0) av_log(ctx, AV_LOG_DEBUG, __VA_ARGS__); } while (0)
^
../../third_party/ffmpeg/libavcodec/bitstream.c(169,5): note: put the
semicolon on a separate line to silence this warning
Reverting until this can be figured out.
> Do not show it when `if` or `else` come from macros.
> E.g.,
>
> #define USED(A) if (A); else
> #define SOME_IF(A) if (A)
>
> void test() {
> // No warnings are shown in those cases now.
> USED(0);
> SOME_IF(0);
> }
>
> Patch by Ilya Biryukov!
>
> Differential Revision: https://reviews.llvm.org/D40185
Peter Smith [Mon, 20 Nov 2017 13:43:55 +0000 (13:43 +0000)]
[ARM] For assembler files recognize -Xassembler or -Wa, -mthumb
The Unified Arm Assembler Language is designed so that the majority of
assembler files can be assembled for both Arm and Thumb with the choice
made as a compilation option.
The way this is done in gcc is to pass -mthumb to the assembler with either
-Wa,-mthumb or -Xassembler -mthumb. This change adds support for these
options to clang. There is no assembler equivalent of -mno-thumb, -marm or
-mno-arm so we don't need to recognize these.
Ideally we would do all of the processing in
CollectArgsForIntegratedAssembler(). Unfortunately we need to change the
triple and at that point it is too late. Instead we look for the option
earlier in ComputeLLVMTriple().
Jonas Hahnfeld [Sat, 18 Nov 2017 21:00:46 +0000 (21:00 +0000)]
[OpenMP] Show error if VLAs are not supported
Some target devices (e.g. Nvidia GPUs) don't support dynamic stack
allocation and hence no VLAs. Print errors with description instead
of failing in the backend or generating code that doesn't work.
This patch handles explicit uses of VLAs (local variable in target
or declare target region) or implicitly generated (private) VLAs
for reductions on VLAs or on array sections with non-constant size.
Sanjay Patel [Sat, 18 Nov 2017 19:31:57 +0000 (19:31 +0000)]
[CodeGen] change const-ness of complex calls
After clarification about the C standard, POSIX, and implementations:
The C standard allows errno-setting, and it's (unfortunately for optimization) even
more clearly stated in the newer additions to the standards.
We can leave these functions as always constant ('c') because they don't
actually do any math and therefore won't set errno:
cimag ( http://en.cppreference.com/w/c/numeric/complex/cimag )
creal ( http://en.cppreference.com/w/c/numeric/complex/creal )
cproj ( http://en.cppreference.com/w/c/numeric/complex/cproj )
conj (http://en.cppreference.com/w/c/numeric/complex/conj )
Michal Gorny [Fri, 17 Nov 2017 22:21:23 +0000 (22:21 +0000)]
[cmake] Use llvm-lit directory when provided for stand-alone build
After the recent lit test changes, clang attempts to run its tests
via llvm-lit by default. However, the llvm-lit binary is not present
when performing stand-alone build resulting in a failure out of the box.
To solve that, add the llvm-lit directory to CMake when performing
a stand-alone build and LLVM sources are provided. This includes
the CMake rules generating the llvm-lit binary and effectively makes
it possible for clang to use it.
Ahmed Bougacha [Fri, 17 Nov 2017 19:46:47 +0000 (19:46 +0000)]
[CodeGen] Compute the objc EH vtable address point using inbounds GEP.
The object is provided by the objc runtime and is never visible in the
module itself, but even so, the address point we compute points into it,
and "+16" is guaranteed not to overflow.
This matches the c++ vtable IRGen.
Note that I'm not entirely convinced the 'i8*' type is correct here: at
the IR level, we're accessing memory that's outside the global object.
But we don't control the allocation, so it's not obviously wrong either.
But either way, this is only in a global initializer, so I don't think
it's going to be mucked with. Filed PR35352 to discuss that.
Martin Probst [Fri, 17 Nov 2017 18:06:33 +0000 (18:06 +0000)]
clang-format: remove trailing lines in lamdas and arrow functions.
Summary:
clang-format already removes empty lines at the beginning & end of
blocks:
int x() {
foo(); // lines before and after will be removed.
}
However because lamdas and arrow functions are parsed as expressions,
the existing logic to remove empty lines in UnwrappedLineFormatter
doesn't handle them.
This change special cases arrow functions in ContinuationIndenter to
remove empty lines:
x = []() {
foo(); // lines before and after will now be removed.
Dave Lee [Fri, 17 Nov 2017 16:27:21 +0000 (16:27 +0000)]
Fix skipping of flags in getClangStripDependencyFileAdjuster
Summary:
The ArgumentsAdjuster returned from `getClangStripDependencyFileAdjuster` will
skip dependency flags, and also their associated values for those flags that
take an argument. This change corrects the handling of the `-MD` and `-MMD`
flags, which do not take an argument.
Manuel Klimek [Fri, 17 Nov 2017 11:17:15 +0000 (11:17 +0000)]
Implement more accurate penalty & trade-offs while breaking protruding tokens.
For each line that we break in a protruding token, compute whether the
penalty of breaking is actually larger than the penalty of the excess
characters. Only break if that is the case.
[PCH+Modules] Improve diagnosticts to help out users pass an extra header search path
When mixing PCH and Implicit Modules, missing a header search path
can lead to the implicit built PCM to complaint about not finding its
matching module map.
Instead of adding more magic to implicit modules engine, add a note to
help the user add the appropriate path.
Justin Lebar [Fri, 17 Nov 2017 01:15:43 +0000 (01:15 +0000)]
[CUDA] Remove implementations of nexttoward.
Summary:
__builtin_nexttoward lowers to a libcall, e.g. nexttowardf(), that CUDA
does not have.
Rather than try to implement it, we simply remove these functions --
nvcc doesn't support them either, and nextafter, which does work, does
essentially the same thing on GPUs, because GPUs don't have long double.
Zachary Turner [Fri, 17 Nov 2017 00:41:18 +0000 (00:41 +0000)]
Re-revert "Refactor debuginfo-tests"
This is still broken because it causes certain tests to be
run twice with slightly different configurations, which is
wrong in some cases.
You can observe this by running:
ninja -nv check-all | grep debuginfo-tests
And seeing that it passes clang/test and clang/test/debuginfo-tests
to lit, which causes it to run debuginfo-tests twice. The fix is
going to involve either:
a) figuring out that we're running in this "deprecated" configuration,
and then deleting the clang/test/debuginfo-tests path, which should
cause it to behave identically to before, or:
b) make lit smart enough that it doesn't descend into a sub-suite if
that sub-suite already has a lit.cfg file.
Richard Smith [Thu, 16 Nov 2017 23:54:56 +0000 (23:54 +0000)]
PR22763: if a defaulted (non-user-provided) special member function is
explicitly instantiated, still emit it with each use.
We don't emit a definition of the member with an explicit instantiation
definition (and indeed it appears that we're not allowed to, since an explicit
instantiation definition does not constitute an odr-use and only odr-use
permits definition for defaulted special members). So we still need to emit a
weak definition with each use.
This also makes defaulted-in-class declarations behave more like
implicitly-declared special members, which matches their design intent.
And it matches the way this problem was solved in GCC.
Ben Hamilton [Thu, 16 Nov 2017 19:34:08 +0000 (19:34 +0000)]
[VirtualFileSystem] Support creating directories then adding files inside
Summary:
In https://reviews.llvm.org/D39572 , I added support for specifying
`Type` when invoking `InMemoryFileSystem::addFile()`.
However, I didn't account for the fact that when `Type` is
`directory_file`, we need to construct an `InMemoryDirectory`, not an
`InMemoryFile`, or else clients cannot create files inside that
directory.
Reid Kleckner [Thu, 16 Nov 2017 19:09:36 +0000 (19:09 +0000)]
[MS] Apply adjustments after storing 'this'
Summary:
The MS ABI convention is that the 'this' pointer on entry is the address
of the vfptr that was used to make the virtual method call. In other
words, the pointer on entry always points to the base subobject that
introduced the virtual method. Consider this hierarchy:
struct A { virtual void f() = 0; };
struct B { virtual void g() = 0; };
struct C : A, B {
void f() override;
void g() override;
};
On entry to C::g, [ER]CX will contain the address of C's B subobject,
and C::g will have to subtract sizeof(A) to recover a pointer to C.
Before this change, we applied this adjustment in the prologue and
stored the new value into the "this" local variable alloca used for
debug info. However, MSVC does not do this, presumably because it is
often profitable to fold the adjustment into later field accesses. This
creates a problem, because the debugger expects the variable to be
unadjusted. Unfortunately, CodeView doesn't have anything like DWARF
expressions for computing variables that aren't in the program anymore,
so we have to declare 'this' to be the unadjusted value if we want the
debugger to see the right value.
This has the side benefit that, in optimized builds, the 'this' pointer
will usually be available on function entry because it doesn't require
any adjustment.
Zachary Turner [Thu, 16 Nov 2017 18:26:20 +0000 (18:26 +0000)]
Resubmit "Refactor debuginfo-tests"
This was reverted due to some failures on specific darwin buildbots,
the issue being that the new lit configuration was not setting the
SDKROOT environment variable. We've tested a fix locally and confirmed
that it works, so this patch resubmits everything with the fix
applied.
Ilya Biryukov [Thu, 16 Nov 2017 16:25:01 +0000 (16:25 +0000)]
Allow to store precompiled preambles in memory.
Summary:
These preambles are built by ASTUnit and clangd. Previously, preambles
were always stored on disk.
In-memory preambles are routed back to the compiler as virtual files in
a custom VFS.
Interface of ASTUnit does not allow to use in-memory preambles, as
ASTUnit::CodeComplete receives FileManager as a parameter, so we can't
change VFS used by the compiler inside the CodeComplete method.
A follow-up commit will update clangd in clang-tools-extra to use
in-memory preambles.
Erich Keane [Wed, 15 Nov 2017 22:25:39 +0000 (22:25 +0000)]
Split x86 "Processor" info into its own def file. [NFC]
A first step toward removing the repetition of
features/CPU info in the x86 target info, this
patch pulls all the processor information out into
its own .def file.
Hans Wennborg [Wed, 15 Nov 2017 17:11:53 +0000 (17:11 +0000)]
BuiltinOperatorOverloadBuilder: Don't consider types that are unavailable on the target (PR35174)
In the PR, Clang ended up in a situation where it tried to mangle the
__float128 type, which isn't supported when targetingt MSVC, because
Clang instantiated a variable template with that type when searching for
a conversion to use in an arithmetic expression.
David Blaikie [Wed, 15 Nov 2017 16:52:12 +0000 (16:52 +0000)]
ASTMatchers.h: Fix ODR violations by avoiding internal linkage variables in headers
Internal linkage variables ODR referenced from inline functions create
ODR violations (the same inline function ends up having different
definitions in each TU, since it references different variables - rather
than one definition).
This also happens to break modular code generation - so this is the last
fix to allow clang to compile with modular code generation.
Alexey Bader [Wed, 15 Nov 2017 11:38:17 +0000 (11:38 +0000)]
[OpenCL] Fix code generation of function-scope constant samplers.
Summary:
Constant samplers are handled as static variables and clang's code generation
library, which leads to llvm::unreachable. We bypass emitting sampler variable
as static since it's translated to a function call later.
Richard Smith [Wed, 15 Nov 2017 01:33:46 +0000 (01:33 +0000)]
[modules] Fix crash in complex class merging scenario.
When we merge together class definitions, we can end up with the canonical
declaration of a field not being the one that was lexically within the
canonical definition of the class. Additionally, when we merge class
definitions via update records (eg, for a template specialization whose
declaration is instantiated in one module and whose definition is instantiated
in multiple others), we can end up with the list of lexical contents for the
class not including a particular declaration of a field whose lexical parent is
that class definition. In the worst case, we have a field whose canonical
declaration's lexical parent has no fields, and in that case this attempt to
number the fields by walking the fields in the declaration of the class that
contained one of the canonical fields will fail.
Instead, when numbering fields in a class, do the obvious thing: walk the
fields in the definition.
I'm still trying to reduce a testcase; the setup that leads to the above
scenario seems to be quite fragile.
Vedant Kumar [Tue, 14 Nov 2017 23:56:53 +0000 (23:56 +0000)]
[PGO] Detect more structural changes with the stable hash
Lifting from Bob Wilson's notes: The hash value that we compute and
store in PGO profile data to detect out-of-date profiles does not
include enough information. This means that many significant changes to
the source will not cause compiler warnings about the profile being out
of date, and worse, we may continue to use the outdated profile data to
make bad optimization decisions. There is some tension here because
some source changes won't affect PGO and we don't want to invalidate the
profile unnecessarily.
This patch adds a new hashing scheme which is more sensitive to loop
nesting, conditions, and out-of-order control flow. Here are examples
which show snippets which get the same hash under the current scheme,
and different hashes under the new scheme:
Loop Nesting Example
--------------------
// Snippet 1
while (foo()) {
while (bar()) {}
}
// Snippet 2
while (foo()) {}
while (bar()) {}
Condition Example
-----------------
// Snippet 1
if (foo())
bar();
baz();
// Snippet 2
if (foo())
bar();
else
baz();
Out-of-order Control Flow Example
---------------------------------
// Snippet 1
while (foo()) {
if (bar()) {}
baz();
}
// Snippet 2
while (foo()) {
if (bar())
continue;
baz();
}
In each of these cases, it's useful to differentiate between the
snippets because swapping their profiles gives bad optimization hints.
The new hashing scheme considers some logical operators in an effort to
detect more changes in conditions. This isn't a perfect scheme. E.g, it
does not produce the same hash for these equivalent snippets:
// Snippet 1
bool c = !a || b;
if (d && e) {}
// Snippet 2
bool f = d && e;
bool c = !a || b;
if (f) {}
This would require an expensive data flow analysis. Short of that, the
new hashing scheme looks reasonably complete, based on a scan over the
statements we place counters on.
Profiles which use the old version of the PGO hash remain valid and can
be used without issue (there are tests in tree which check this).
Hans Wennborg [Tue, 14 Nov 2017 21:13:27 +0000 (21:13 +0000)]
Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes
This updates -mcount to use the new attribute names (LLVM r318195), and
switches over -finstrument-functions to also use these attributes rather
than inserting instrumentation in the frontend.
It also adds a new flag, -finstrument-functions-after-inlining, which
makes the cygprofile instrumentation get inserted after inlining rather
than before.
Dave Lee [Tue, 14 Nov 2017 14:17:26 +0000 (14:17 +0000)]
Make isDefinition matcher support ObjCMethodDecl
Summary:
Allow the `isDefinition()` matcher to apply to `ObjCMethodDecl` nodes, in
addition to those it already supports. For whatever reason, `ObjCMethodDecl`
does not inherit from `FunctionDecl` and so this is specialization is necessary.
Erik Verbruggen [Tue, 14 Nov 2017 09:34:39 +0000 (09:34 +0000)]
[libclang] Allow crash recovery with LIBCLANG_NOTHREADS
Enabled crash recovery for some libclang operations on a calling thread even
when LIBCLANG_NOTHREAD is specified.
Previously it would only run under crash recovery if LIBCLANG_NOTHREAD is not
set. Moved handling of LIBCLANG_NOTHREAD env variable into RunSafely from its
call sites.
Create more orthogonal pieces. The restructuring made it easy to try out
several alternatives to D33589, and while none of the alternatives
turned out to be the right solution, the underlying simplification of
the structure is helpful.
Craig Topper [Tue, 14 Nov 2017 08:48:28 +0000 (08:48 +0000)]
[NewPassManager] Pass the -fdebug-pass-manager flag setting into the Analysis managers to match what we do in opt
Summary: Currently the -fdebug-pass-manager flag for clang doesn't enable the debug logging in the analysis managers. This is different than what the switch does when passed to opt.
Chandler Carruth [Tue, 14 Nov 2017 01:59:18 +0000 (01:59 +0000)]
[PM] Wire up support for the bounds checking sanitizer with the new PM.
Not much interesting here. Mostly wiring things together.
One thing worth noting is that the approach is substantially different
from the old PM. Here, the -O0 case works fundamentally differently in
that we just directly build the pipeline without any callbacks or other
cruft. In some ways, this is nice and clean. However, I don't like that
it causes the sanitizers to be enabled with different changes at
different times. =/ Suggestions for a better way to do this are welcome.
Zachary Turner [Mon, 13 Nov 2017 23:33:29 +0000 (23:33 +0000)]
Revert "Update test_debuginfo.pl script to point to new tree location."
This reverts the aforementioned patch and 2 subsequent follow-ups,
as some buildbots are still failing 2 tests because of it.
Investigation is ongoing into the cause of the failures.