Reid Kleckner [Wed, 3 May 2017 18:17:31 +0000 (18:17 +0000)]
[IR] Abstract away ArgNo+1 attribute indexing as much as possible
Summary:
Do three things to help with that:
- Add AttributeList::FirstArgIndex, which is an enumerator currently set
to 1. It allows us to change the indexing scheme with fewer changes.
- Add addParamAttr/removeParamAttr. This just shortens addAttribute call
sites that would otherwise need to spell out FirstArgIndex.
- Remove some attribute-specific getters and setters from Function that
take attribute list indices. Most of these were only used from
BuildLibCalls, and doesNotAlias was only used to test or set if the
return value is malloc-like.
I'm happy to split the patch, but I think they are probably easier to
review when taken together.
This patch should be NFC, but it sets the stage to change the indexing
scheme to this, which is more convenient when indexing into an array:
0: func attrs
1: retattrs
2...: arg attrs
Alex Lorenz [Wed, 3 May 2017 15:41:16 +0000 (15:41 +0000)]
DiagnosticsEngine should clear DelayedDiagID before reporting the
delayed diagnostic
This avoids an infinite loop that was uncovered in one of our internal tests
by r301992. The testcase is the most reduced version of that auto-generated
test.
Carlo Bertolli [Wed, 3 May 2017 15:28:48 +0000 (15:28 +0000)]
[OpenMP] Extended parse for 'always' map modifier
https://reviews.llvm.org/D32807
This patch allows the map modifier 'always' to be separated by the map type (to, from, tofrom) only by a whitespace, rather than strictly by a comma as in current trunk.
Oren Ben Simhon [Wed, 3 May 2017 14:05:00 +0000 (14:05 +0000)]
Reusing an existing attribute diagnostic
In a previous patch, a new generic error diagnostic for inconsistent attributes was added.
In this commit I reuse this diagnostic for ns_returns_retained attribute check.
It was written as "Memory Error" in most places and as "Memory error" in a few
other places, however it is the latter that is more consistent with
other categories (such as "Logic error").
Daniel Jasper [Wed, 3 May 2017 11:27:34 +0000 (11:27 +0000)]
Undo turning ExtBehavior into a bitfield.
This produces warnings that I can't explain in a GCC build:
In file included from ../tools/clang/include/clang/Lex/LexDiagnostic.h:13:0,
from /usr/local/google/home/djasper/llvm/tools/clang/lib/Lex/PTHLexer.cpp:19:
../tools/clang/include/clang/Basic/Diagnostic.h:219:34: warning: ‘clang::DiagnosticsEngine::DiagState::ExtBehavior’ is too small to hold all values of ‘enum class clang::diag::Severity’ [enabled by default]
diag::Severity ExtBehavior : 4; // Map extensions to warnings or errors?
^
While I don't think this warning makes sense, I'd like to get this back to being
warning-free. This only seems to trigger for "enum class".
Daniel Jasper [Wed, 3 May 2017 07:48:27 +0000 (07:48 +0000)]
Silences gcc's -Wnarrowing.
I think this is a false positive in GCC's warning, but nonetheless, we
should try to be warning-free. Smaller reproducer (reproduces with GCC
6.3):
https://godbolt.org/g/cJuO2z
Yuka Takahashi [Wed, 3 May 2017 04:58:39 +0000 (04:58 +0000)]
Fix a bug that -isysroot is completely ignored on Unix
-isysroot is the flag which set the system root directory.
This bug report https://bugs.llvm.org//show_bug.cgi?id=11503
shows that -isysroot is not handled at all on Unix, so fixed this bug.
After this diff, I could get this result https://pastebin.com/TeCmn9mj .
Richard Smith [Wed, 3 May 2017 00:28:49 +0000 (00:28 +0000)]
[modules] Round-trip -Werror flag through explicit module build.
The intent for an explicit module build is that the diagnostics produced within
the module are those that were configured when the module was built, not those
that are enabled within a user of the module. This includes diagnostics that
don't actually show up until the module is used (for instance, diagnostics
produced during template instantiation and weird cases like -Wpadded).
We serialized and restored the diagnostic state for individual warning groups,
but previously did not track the state for flags like -Werror and -Weverything,
which are implemented as separate bits rather than as part of the diagnostics
mapping information.
Vedant Kumar [Tue, 2 May 2017 23:46:56 +0000 (23:46 +0000)]
[ubsan] Skip overflow checks on safe arithmetic (fixes PR32874)
Currently, ubsan emits overflow checks for arithmetic that is known to
be safe at compile-time, e.g:
1 + 1 => CheckedAdd(1, 1)
This leads to breakage when using the __builtin_prefetch intrinsic. LLVM
expects the arguments to @llvm.prefetch to be constant integers, and
when ubsan inserts unnecessary checks on the operands to the intrinsic,
this contract is broken, leading to verifier failures (see PR32874).
Instead of special-casing __builtin_prefetch for ubsan, this patch fixes
the underlying problem, i.e that clang currently emits unnecessary
overflow checks.
Testing: I ran the check-clang and check-ubsan targets with a stage2,
ubsan-enabled build of clang. I added a regression test for PR32874, and
some extra checking to make sure we don't regress runtime checking for
unsafe arithmetic. The existing ubsan-promoted-arithmetic.cpp test also
provides coverage for this change.
Faisal Vali [Tue, 2 May 2017 21:02:46 +0000 (21:02 +0000)]
[NFC] Add original test that triggered crash post r301735
- this is added just for completeness sake (though the general case should be represented by the test added in the revision to that patch: https://reviews.llvm.org/rL301972 )
When computing the appropriate cv-qualifiers for the 'this' capture, we have to examine each enclosing lambda - but when using the FunctionScopeInfo stack we have to ensure that the lambda below (outer) is the decl-context of the closure-class of the current lambda.
https://bugs.llvm.org/show_bug.cgi?id=32831
This patch was initially committed here: https://reviews.llvm.org/rL301735
Then reverted here: https://reviews.llvm.org/rL301916
The issue with the original patch was a failure to check that the closure type has been created within the LambdaScopeInfo before querying its DeclContext - instead of just assuming it has (silly!). A reduced example such as this highlights the problem:
struct X {
int data;
auto foo() { return [] { return [] -> decltype(data) { return 0; }; }; }
};
When 'data' within decltype(data) tries to determine the type of 'this', none of the LambdaScopeInfo's have their closure types created at that point.
Reid Kleckner [Tue, 2 May 2017 20:10:03 +0000 (20:10 +0000)]
Simplify some va_start checking logic
Combine the logic doing the ms_abi/sysv_abi checks into one function so
that each check and its logical opposite are near each other. Now we
don't need two Sema entry points for MS va_start and regular va_start.
Refactor the code that checks if the va_start caller is a function,
block, or obj-c method. We do this in three places, and they are all
buggy for variadic lambdas (PR32737). After this change, I have one
place to apply the functional fix.
When discovering the initializer for A::a, the bounds of A::b aren't known yet.
It is unclear whether warning about errors should be deferred until the end of
the translation unit, possibly resolving errors that can be resolved. In
practice, the compiler can know the bounds of all arrays in this example.
Credits for reproducers and explanation go to Richard Smith. Richard, please
add more info in case my explanation is wrong.
Nick Lewycky [Tue, 2 May 2017 01:06:16 +0000 (01:06 +0000)]
Revert r301785 (and r301787) because they caused PR32864.
The fix is that ExprEvaluatorBase::VisitInitListExpr should handle transparent exprs instead of exprs with one element. Fixing that uncovers one testcase failure because the AST for "constexpr _Complex float test2 = {1};" is wrong (the _Complex prvalue should not be const-qualified), and a number of test failures in test/OpenMP where the captured stmt contains an InitListExpr that is in syntactic form.
Richard Smith [Mon, 1 May 2017 22:10:47 +0000 (22:10 +0000)]
Fix initial diagnostic state setup for an explicit module with no diagnostic pragmas.
If a file has no diagnostic pragmas, we build its diagnostic state lazily, but
in this case we never set up the root state to be the diagnostic state in which
the module was originally built, so the diagnostic flags for files in the
module with no diagnostic pragmas were incorrectly based on the user of the
module rather than the diagnostic state when the module was built.
Richard Smith [Mon, 1 May 2017 21:49:54 +0000 (21:49 +0000)]
Put back REQUIRES: system-darwin to fix asan bot.
These tests do not appear to be Darwin-specific, and this REQUIRES: appears to
be hiding a real bug; this change is just restoring the prior state to get the
buildbots happy again while we investigate. (The system-darwin requirement is
covered by PR32851.)
Richard Smith [Mon, 1 May 2017 18:49:04 +0000 (18:49 +0000)]
Improve handling of arrays of unknown bound in constant expressions.
Do not spuriously reject constexpr functions that access elements of an array
of unknown bound; this may later become valid once the bound is known. Permit
array-to-pointer decay on such arrays, but disallow pointer arithmetic (since
we do not know whether it will have defined behavior).
The standard is not clear on how this should work, but this seems to be a
decent answer.
Nick Lewycky [Sat, 29 Apr 2017 09:33:46 +0000 (09:33 +0000)]
Remove Sema::CheckForIntOverflow, and instead check all full-expressions.
CheckForIntOverflow used to implement a whitelist of top-level expressions to
send to the constant expression evaluator, which handled many more expressions
than the CheckForIntOverflow whitelist did.
When computing the appropriate cv-qualifiers for the 'this' capture, we have to examine each enclosing lambda - but when using the FunctionScopeInfo stack we have to ensure that the lambda below (outer) is the decl-context of the closure-class of the current lambda.
Richard Smith [Sat, 29 Apr 2017 00:34:47 +0000 (00:34 +0000)]
Add pragma to perform module import and use it in -E output.
Many of our supported configurations support modules but do not have any
first-class syntax to perform a module import. This leaves us with a problem:
there is no way to represent the expansion of a #include that imports a module
in the -E output for such languages. (We don't want to just leave it as a
#include because that requires the consumer of the preprocessed source to have
the same file system layout and include paths as the creator.)
This patch adds a new pragma:
#pragma clang module import MODULE.NAME.HERE
that imports a module, and changes -E and -frewrite-includes to use it when
rewriting a #include that maps to a module import. We don't make any attempt
to use a native language syntax import if one exists, to get more consistent
output. (If in the future, @import and #include have different semantics in
some way, the pragma will track the #include semantics.)
Nick Lewycky [Sat, 29 Apr 2017 00:07:27 +0000 (00:07 +0000)]
ObjCBoxedExpr can't be evaluated by the constant expression evaluator.
A boxed expression evaluates its subexpr and then calls an objc method to transform it into another value with pointer type. The objc method can never be constexpr and therefore this expression can never be evaluated. Fixes a miscompile boxing expressions with side-effects.
Also make ObjCBoxedExpr handling a normal part of the expression evaluator instead of being the only case besides full-expression where we check for integer overflow.
David Blaikie [Fri, 28 Apr 2017 20:50:25 +0000 (20:50 +0000)]
Enable -fno-split-dwarf-inlining even when -gsplit-dwarf isn't specified.
Since -gsplit-dwarf is specified on a backend compile (in ThinLTO
parlance) it isn't passed during the frontend compile (because no ELF
object/dwo file is produced then), yet the -fno-split-dwarf-inlining
value needs to be encoded in the LLVM DebugInfo metadata to have
effect...
So let it be specified & it'll be silently ignored if -gsplit-dwarf
isn't used in the end, otherwise it'll be used on a per-cu basis
depending on where it's specified in the frontend compile actions.
[CodeGen][ObjC] Don't retain captured Objective-C pointers at block
creation that are const-qualified.
When a block captures an ObjC object pointer, clang retains the pointer
to prevent prematurely destroying the object the pointer points to
before the block is called or copied.
When the captured object pointer is const-qualified, we can avoid
emitting the retain/release pair since the pointer variable cannot be
modified in the scope in which the block literal is introduced.
- I removed doxygen comments for the intrinsics that "alias" the other existing documented intrinsics and that only sligtly differ in spelling (single underscores vs. double underscores).
#define _tzcnt_u16(a) (__tzcnt_u16((a)))
It will be very hard to keep the documentation for these "aliases" in sync with the documentation for the intrinsics they alias to. Out of sync documentation will be more confusing than no documentation.
I got an OK from Eric Christopher to commit doxygen comments without prior code
review upstream.
[libclang] Expose some target information via the C API.
This allows users to query the target triple and target pointer width, which
would make me able to fix https://github.com/servo/rust-bindgen/issues/593 and
other related bugs in an elegant way (without having to manually parse the
target triple in the command line arguments).
Richard Smith [Fri, 28 Apr 2017 01:49:42 +0000 (01:49 +0000)]
Move functionality for handling module maps as inputs from the -emit-module
action to the general FrontendAction infrastructure.
This permits applying -E, -ast-dump, -fsyntax-only, and so on to a module map
compilation. (The -E form is not currently especially useful yet as there's no
good way to take the output and use it to actually build a module.)
In order to support this, -cc1 now accepts -x <lang>-module-map in all cases
where it accepts -x <lang> for a language we can parse (not ir/ast). And for
uniformity, we also accept -x <lang>-header for all such languages (we used
to reject for cuda and renderscript), and -x <lang>-cpp-output for all such
languages (we used to reject for c, cl, and renderscript).
(None of these new alternatives are accepted by the driver yet, so no
user-visible changes.)
[ARCMigrate] When applying changes from remap files, disable the 'adjustRemovals' functionality of EditedSource
'adjustRemovals' is used to avoid situation when removing a range inadvertently causes 2 separate identifiers to get joined into one.
But it is not useful when the edits are character precise, as is the case with the remap files.
[Modules] Improve diagnostics for incomplete umbrella
One of the -Wincomplete-umbrella warnings diagnoses when a header is present in
the directory but it's not present in the umbrella header. Currently, this
warning only happens on top level modules; any submodule using an umbrella
header does not get this warning. Fix that by also considering the submodules.
Headers: Make the type of SIZE_MAX the same as size_t
size_t is usually defined as unsigned long, but on 64-bit platforms,
stdint.h currently defines SIZE_MAX using "ull" (unsigned long long).
Although this is the same width, it doesn't necessarily have the same
alignment or calling convention. It also triggers printf warnings when
using the format flag "%zu" to print SIZE_MAX.
This changes SIZE_MAX to reuse the compiler-provided __SIZE_MAX__, and
provides similar fixes for the other integers:
... and fixes the typedefs for intptr_t and uintptr_t to use
__INTPTR_TYPE__ and __UINTPTR_TYPE__ instead of int32_t, effectively
reverting r89224, r89226, and r89237 (r89221 already having been
effectively reverted).
We can probably also kill __INTPTR_WIDTH__, __INTMAX_WIDTH__, and
__UINTMAX_WIDTH__ in a follow-up, but I was hesitant to delete all the
per-target CHECK lines in this commit since those might serve their own
purpose.
Preprocessor: Suppress -Wnonportable-include-path for header maps
If a file search involves a header map, suppress
-Wnonportable-include-path. It's firing lots of false positives for
framework authors internally, and it's not trivial to fix.
Consider a framework called "Foo" with a main (installed) framework header
"Foo/Foo.h". It's atypical for "Foo.h" to actually live inside a
directory called "Foo" in the source repository. Instead, the
build system generates a header map while building the framework.
If Foo.h lives at the top-level of the source repository (common), and
the git repo is called ssh://some.url/foo.git, then the header map will
have something like:
Foo/Foo.h -> /Users/myname/code/foo/Foo.h
where "/Users/myname/code/foo" is the clone of ssh://some.url/foo.git.
After #import <Foo/Foo.h>, the current implementation of
-Wnonportable-include-path will falsely assume that Foo.h was found in a
nonportable way, because of the name of the git clone (.../foo/Foo.h).
However, that directory name was not involved in the header search at
all.
This commit adds an extra parameter to Preprocessor::LookupFile and
HeaderSearch::LookupFile to track if the search used a header map,
making it easy to suppress the warning. Longer term, once we find a way
to avoid the false positive, we should turn the warning back on.
Clang warns that a profile is out-of-date if it can't find a profile
record for any function in a TU. This warning became noisy after llvm
started allowing dead-stripping of instrumented functions.
To fix this, this patch changes the existing profile out-of-date warning
(-Wprofile-instr-out-of-date) so that it only complains about mismatched
data. Further, it introduces a new, off-by-default warning about missing
function data (-Wprofile-instr-missing).
Remove leaking UnknownPragmaHandlers right after we are done with them.
The UnknownPragmaHandlers added by DoPrintPreprocessedInput conflict with the
real PragmaHandlers from clang::Parser because they try to handle the same
#pragma directives. This makes it impossible to use a Preprocessor (that was
previously passed to DoPrintPreprocessedInput), as an Preprocessor for a
clang::Parser instance which is what we currently do in cling.
This patch removes the added UnknownPragmaHandler to avoid conflicts these
conflicts and leave the PragmaHandlers of the Preprocessors in a the same state
as before calling DoPrintPreprocessedInput.
Martin Probst [Thu, 27 Apr 2017 13:07:24 +0000 (13:07 +0000)]
clang-format: [JS] parse async function declarations.
Summary:
Previously, clang-format would accidentally parse an async function
declaration as a function expression, and thus not insert an unwrapped
line for async functions, causing subsequent functions to run into the
function:
async function f() {
x();
} function g() { ...
With this change, async functions get parsed as top level function
declarations and get their own unwrapped line context.
[X86] Support of no_caller_saved_registers attribute
Implements the Clang part for no_caller_saved_registers attribute as appears here:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=5ed3cc7b66af4758f7849ed6f65f4365be8223be.
Alex Lorenz [Thu, 27 Apr 2017 10:43:48 +0000 (10:43 +0000)]
[ObjC] Disallow vector parameters and return values in Objective-C methods
for iOS < 9 and OS X < 10.11 X86 targets
This commit adds a new error that disallows methods that have parameters/return
values with a vector type for some older X86 targets. This diagnostic is
needed because objc_msgSend doesn't support SIMD vector registers/return values
on X86 in iOS < 9 and OS X < 10.11. Note that we don't necessarily know if the
vector argument/return value will use a SIMD register, so instead we chose to
be conservative and prohibit all vector types.
Nick Lewycky [Thu, 27 Apr 2017 07:27:36 +0000 (07:27 +0000)]
In the expression evaluator, visit the index of an ArraySubscriptExpr even if we can't evaluate the base, if the evaluation mode tells us to continue evaluation.
Nick Lewycky [Thu, 27 Apr 2017 07:11:09 +0000 (07:11 +0000)]
In the expression evaluator, descend into both the true and false expressions of a ConditionalOperator when the condition can't be evaluated and we're in an evaluation mode that says we should continue evaluating.
Richard Smith [Thu, 27 Apr 2017 01:17:05 +0000 (01:17 +0000)]
Improve diagnostics for bad -std= flag.
Don't list deprecated -std= values (c++0x etc). Only produce one line of output
per standard, even if we know it by multiple names.
In passing, add missing -std=gnu++03 alias (supported by GCC), and add new
spelling '-std=cl1.0' for OpenCL 1.0 for consistency with the other values,
with the same meaning as the preexisting '-std=cl'.
Richard Smith [Wed, 26 Apr 2017 23:44:33 +0000 (23:44 +0000)]
Don't accept -std= values that would switch us to a different source language.
We already prohibited this in most cases (in r130710), but had some bugs in our
enforcement of this rule. Specifically, this prevents the following
combinations:
* -x c -std=clN.M, which would previously effectively act as if -x cl were
used, despite the input being a C source file. (-x cl -std=cNN continues
to be disallowed.)
* -x c++ -std=cuda, which would previously select C++98 + CUDA, despite that
not being a C++ standard. (-x cuda -std=c++NN is still permitted, and
selects CUDA with the given C++ standard as its base language.
-x cuda -std=cuda is still supported with the meaning of CUDA + C++98.)
* -x renderscript -std=c++NN, which would previously form a hybrid "C++ with
RenderScript extensions" language. We could support such a thing, but
shouldn't do so by accident.