Vedant Kumar [Tue, 14 Mar 2017 01:56:34 +0000 (01:56 +0000)]
[ubsan] Add a nullability sanitizer
Teach UBSan to detect when a value with the _Nonnull type annotation
assumes a null value. Call expressions, initializers, assignments, and
return statements are all checked.
Because _Nonnull does not affect IRGen, the new checks are disabled by
default. The new driver flags are:
-fsanitize=nullability-arg (_Nonnull violation in call)
-fsanitize=nullability-assign (_Nonnull violation in assignment)
-fsanitize=nullability-return (_Nonnull violation in return stmt)
-fsanitize=nullability (all of the above)
This patch builds on top of UBSan's existing support for detecting
violations of the nonnull attributes ('nonnull' and 'returns_nonnull'),
and relies on the compiler-rt support for those checks. Eventually we
will need to update the diagnostic messages in compiler-rt (there are
FIXME's for this, which will be addressed in a follow-up).
One point of note is that the nullability-return check is only allowed
to kick in if all arguments to the function satisfy their nullability
preconditions. This makes it necessary to emit some null checks in the
function body itself.
Testing: check-clang and check-ubsan. I also built some Apple ObjC
frameworks with an asserts-enabled compiler, and verified that we get
valid reports.
Juergen Ributzka [Tue, 14 Mar 2017 00:14:40 +0000 (00:14 +0000)]
Reapply [VFS] Ignore broken symlinks in the directory iterator.
Modified the tests to accept any iteration order, to run only on Unix, and added
additional error reporting to investigate SystemZ bot issue.
The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts don't stat entries unless they have to
descend into the next directory, which allows to recover from this issue by
clearing the error code and skipping to the next entry.
This change adds similar behavior to the VFS iterators. There should be no
change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.
Reid Kleckner [Mon, 13 Mar 2017 22:33:04 +0000 (22:33 +0000)]
Widen AST bitfields too small to represent all enumerators
All of these were found by a new warning that I am prototyping,
-Wbitfield-enum-conversion.
Stmt::ExprBits::ObjectKind - This was not wide enough to represent
OK_ObjSubscript, so this was a real, true positive bug.
ObjCDeclSpec::objCDeclQualifier - On Windows, setting DQ_CSNullability
would cause the bitfield to become negative because enum types are
always implicitly 'int' there. This would probably never be noticed
because this is a flag-style enum, so we only ever test one bit at a
time. Switching to 'unsigned' also makes this type pack smaller on
Windows.
FunctionDecl::SClass - Technically, we only need two bits for all valid
function storage classes. Functions can never have automatic or register
storage class. This seems a bit too clever, and we have a bit to spare,
so widening the bitfield seems like the best way to pacify the warning.
You could classify this as a false positive, but widening the bitfield
defends us from invalid ASTs.
Matt Arsenault [Mon, 13 Mar 2017 19:47:53 +0000 (19:47 +0000)]
AMDGPU: Make 0 the private nullptr value
We can't actually pretend that 0 is valid for address space 0.
r295877 added a workaround to stop allocating user objects
there, so we can use 0 as the invalid pointer.
Some of the tests seemed to be using private as the non-0 null
test address space, so add copies using local to make sure
this is still stressed.
Change ASTFileSignature from a random 32-bit number to the hash of the
PCM content.
- Move definition ASTFileSignature to Basic/Module.h so Module and
ASTSourceDescriptor can use it.
- Change the signature from uint64_t to std::array<uint32_t,5>.
- Stop using (saving/reading) the size and modification time of PCM
files when there is a valid SIGNATURE.
- Add UNHASHED_CONTROL_BLOCK, and use it to store the SIGNATURE record
and other records that shouldn't affect the hash. Because implicit
modules reuses the same file for multiple levels of -Werror, this
includes DIAGNOSTIC_OPTIONS and DIAG_PRAGMA_MAPPINGS.
This helps to solve a PCH + implicit Modules dependency issue: PCH files
are handled by the external build system, whereas implicit modules are
handled by internal compiler build system. This prevents invalidating a
PCH when the compiler overwrites a PCM file with the same content
(modulo the diagnostic differences).
Craig Topper [Mon, 13 Mar 2017 17:16:50 +0000 (17:16 +0000)]
[X86] Add checking of the scale argument to scatter/gather builtins
The only valid values for scale immediate of scatter/gather builtins are 1, 2, 4, or 8. This patch enforces this in the frontend otherwise we generate invalid instruction encodings in the backend.
When diagnosing taking address of packed members skip __unaligned-qualified expressions
Given that we have already explicitly stated in the qualifier that the
expression is __unaligned, it makes little sense to diagnose that the address
of the packed member may not be aligned.
Alex Lorenz [Mon, 13 Mar 2017 11:17:41 +0000 (11:17 +0000)]
Add -iframeworkwithsysroot compiler option
This commit adds support for a new -iframeworkwithsysroot compiler option which
allows the user to specify a framework path that can be prefixed with the
sysroot. This option is similar to the -iwithsysroot option that exists to
supplement -isystem.
Martin Probst [Mon, 13 Mar 2017 09:39:23 +0000 (09:39 +0000)]
clang-format: [JS] do not wrap @see tags.
Summary:
@see is special among JSDoc tags in that it is commonly followed by URLs. The JSDoc spec suggests that users should wrap URLs in an additional {@link url...} tag (@see http://usejsdoc.org/tags-see.html), but this is very commonly violated, with @see being followed by a "naked" URL.
This change special cases all JSDoc lines that contain an @see not to be wrapped to account for that.
Martin Probst [Mon, 13 Mar 2017 09:14:23 +0000 (09:14 +0000)]
clang-format: [JS] allow breaking after non-null assertions.
Summary:
Previously clang-format would not break after any !. However in TypeScript, ! can be used as a post fix operator for non-nullability:
x.foo()!.bar()!;
With this change, clang-format will wrap after the ! if it is likely a post-fix non null operator.
Sanjay Patel [Sun, 12 Mar 2017 19:15:10 +0000 (19:15 +0000)]
[x86] these aren't the undefs you're looking for (PR32176)
x86 has undef SSE/AVX intrinsics that should represent a bogus register operand.
This is not the same as LLVM's undef value which can take on multiple bit patterns.
There are better solutions / follow-ups to this discussed here:
https://bugs.llvm.org/show_bug.cgi?id=32176
...but this should prevent miscompiles with a one-line code change.
Eric Fiselier [Sat, 11 Mar 2017 02:35:37 +0000 (02:35 +0000)]
[coroutines] Fix diagnostics depending on the first coroutine statement.
Summary:
Some coroutine diagnostics need to point to the location of the first coroutine keyword in the function, like when diagnosing a `return` inside a coroutine. Previously we did this by storing each *valid* coroutine statement in a list and select the first one to use in diagnostics. However if every coroutine statement is invalid we would have no location to point to.
This patch fixes the storage of the first coroutine statement location, ensuring that it gets stored even when the resulting AST node would be invalid.
This patch also removes the `CoroutineStmts` list in `FunctionScopeInfo` because it was unused.
Gor Nishanov [Sat, 11 Mar 2017 01:30:17 +0000 (01:30 +0000)]
[coroutines] Refactor SuspendExpr to create just one OpaqueValue (almost NFC)
Summary:
Create only one OpaqueValue for await_ready/await_suspend/await_resume.
Store OpaqueValue used in the CoroutineSuspendExpr node, so that CodeGen does not have to hunt looking for it.
Juergen Ributzka [Fri, 10 Mar 2017 22:49:04 +0000 (22:49 +0000)]
Reapply [VFS] Ignore broken symlinks in the directory iterator.
Modified the tests to accept any iteration order.
The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts allow to recover from this issue by
clearing the error code and skipping to the next entry.
This change adds the same functionality to the VFS iterators. There should be
no change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.
Juergen Ributzka [Fri, 10 Mar 2017 21:23:27 +0000 (21:23 +0000)]
[VFS] Ignore broken symlinks in the directory iterator.
The VFS directory iterator and recursive directory iterator behave differently
from the LLVM counterparts. Once the VFS iterators hit a broken symlink they
immediately abort. The LLVM counterparts allow to recover from this issue by
clearing the error code and skipping to the next entry.
This change adds the same functionality to the VFS iterators. There should be
no change in current behavior in the current CLANG source base, because all
clients have loop exit conditions that also check the error code.
[OpenCL] Fix type compatibility check and generic AS mangling.
1. Reimplemented conditional operator so that it checks
compatibility of unqualified pointees of the 2nd and
the 3rd operands (C99, OpenCL v2.0 6.5.15).
Define QualTypes compatibility for OpenCL as following:
- corresponding types are compatible (C99 6.7.3)
- CVR-qualifiers are equal (C99 6.7.3)
- address spaces are equal (implementation defined)
2. Added generic address space to Itanium mangling.
Alex Lorenz [Fri, 10 Mar 2017 15:04:58 +0000 (15:04 +0000)]
Print nested name specifiers for typedefs and type aliases
Printing typedefs or type aliases using clang_getTypeSpelling() is missing the
namespace they are defined in. This is in contrast to other types that always
yield the full typename including namespaces.
Anna Zaks [Fri, 10 Mar 2017 00:33:19 +0000 (00:33 +0000)]
[analyzer] Turn suppress-c++-stdlib on by default
We have several reports of false positives coming from libc++. For example,
there are reports of false positives in std::regex, std::wcout, and also
a bunch of issues are reported in https://reviews.llvm.org/D30593. In many
cases, the analyzer trips over the complex libc++ code invariants. Let's turn
off the reports coming from these headers until we can re-evalate the support.
We can turn this back on once we individually suppress all known false
positives and perform deeper evaluation on large codebases that use libc++.
We'd also need to commit to doing these evaluations regularly as libc++
headers change.
Vedant Kumar [Thu, 9 Mar 2017 16:06:27 +0000 (16:06 +0000)]
Retry: [ubsan] Detect UB loads from bitfields
It's possible to load out-of-range values from bitfields backed by a
boolean or an enum. Check for UB loads from bitfields.
This is the motivating example:
struct S {
BOOL b : 1; // Signed ObjC BOOL.
};
S s;
s.b = 1; // This is actually stored as -1.
if (s.b == 1) // Evaluates to false, -1 != 1.
...
Changes since the original commit:
- Single-bit bools are a special case (see CGF::EmitFromMemory), and we
can't avoid dealing with them when loading from a bitfield. Don't try to
insert a check in this case.
Eric Fiselier [Thu, 9 Mar 2017 05:01:31 +0000 (05:01 +0000)]
[coroutines] Fix assertion in DependentCoawaitExpr when the argument is non-dependent.
Summary:
A `co_await arg` expression has a dependent type whenever the promise type is still dependent, even if the argument to co_await is not. This is because we cannot attempt the `await_transform(<arg>)` until after we know the promise type.
This patch fixes an assertion in the constructor of `DependentCoawaitExpr` that asserted that `arg` must also be dependent.
Gor Nishanov [Thu, 9 Mar 2017 03:09:43 +0000 (03:09 +0000)]
[coroutines] Build and pass coroutine_handle to await_suspend
Summary:
This patch adds passing a coroutine_handle object to await_suspend calls.
It builds the coroutine_handle using coroutine_handle<PromiseType>::from_address(__builtin_coro_frame()).
(a revision of https://reviews.llvm.org/D26316 that for some reason refuses to apply via arc patch)
Anna Zaks [Thu, 9 Mar 2017 00:01:10 +0000 (00:01 +0000)]
[analyzer] Improve usability of ExprInspectionChecker
Some of the magic functions take arguments of arbitrary type. However,
for semantic correctness, the compiler still requires a declaration
of these functions with the correct type. Since C does not have
argument-type-overloaded function, this made those functions hard to
use in C code. Improve this situation by allowing arbitrary suffixes
in the affected magic functions' names, thus allowing the user to
create different declarations for different types.
Anna Zaks [Thu, 9 Mar 2017 00:01:07 +0000 (00:01 +0000)]
[analyzer] Add bug visitor for taint checker.
Add a bug visitor to the taint checker to make it easy to distinguish where
the tainted value originated. This is especially useful when the original
taint source is obscured by complex data flow.
Richard Smith [Wed, 8 Mar 2017 23:00:26 +0000 (23:00 +0000)]
Take into account C++17's noexcept function types during merging -- it should
be possible to merge a declaration with an unresolved function type against one
with a resolved function type.
Driver/ToolChains: Mips -> MipsLinux
- Mips is architecture, not a toolchain
- Might help eliminate the confusion in the future by not having header files with the same name
Honor __unaligned in codegen for declarations and expressions
This patch honors the unaligned type qualifier (currently available through he
keyword __unaligned and -fms-extensions) in CodeGen. In the current form the
patch affects declarations and expressions. It does not affect fields of
classes.
[clang-format] Look at NoLineBreak and NoLineBreakInOperand before breakProtrudingToken
Summary:
This patch makes ContinuationIndenter call breakProtrudingToken only if
NoLineBreak and NoLineBreakInOperand is false.
Previously, clang-format required two runs to converge on the following example with 24 columns:
Note that the second operand shouldn't be splitted according to NoLineBreakInOperand, but the
token breaker doesn't take that into account:
```
func(a, "long long long long", c);
```
After first run:
```
func(a, "long long "
"long long",
c);
```
After second run, where NoLineBreakInOperand is taken into account:
```
func(a,
"long long "
"long long",
c);
```
With the patch, clang-format now obtains in one run:
```
func(a,
"long long long"
"long",
c);
```
which is a better token split overall.
[clang-format] Enable comment reflowing in multiline comments containing pragmas
Summary:
This patch enables comment reflowing of lines not matching the comment pragma regex
in multiline comments containing comment pragma lines. Previously, these comments
were dumped without being reindented to the result.
David L. Jones [Wed, 8 Mar 2017 01:02:16 +0000 (01:02 +0000)]
[Driver] Consolidate tools and toolchains by target platform. (NFC)
Summary:
(This is a move-only refactoring patch. There are no functionality changes.)
This patch splits apart the Clang driver's tool and toolchain implementation
files. Each target platform toolchain is moved to its own file, along with the
closest-related tools. Each target platform toolchain has separate headers and
implementation files, so the hierarchy of classes is unchanged.
There are some remaining shared free functions, mostly from Tools.cpp. Several
of these move to their own architecture-specific files, similar to r296056. Some
of them are only used by a single target platform; since the tools and
toolchains are now together, some helpers now live in a platform-specific file.
The balance are helpers related to manipulating argument lists, so they are now
in a new file pair, CommonArgs.h and .cpp.
I've tried to cluster the code logically, which is fairly straightforward for
most of the target platforms and shared architectures. I think I've made
reasonable choices for these, as well as the various shared helpers; but of
course, I'm happy to hear feedback in the review.
There are some particular things I don't like about this patch, but haven't been
able to find a better overall solution. The first is the proliferation of files:
there are several files that are tiny because the toolchain is not very
different from its base (usually the Gnu tools/toolchain). I think this is
mostly a reflection of the true complexity, though, so it may not be "fixable"
in any reasonable sense. The second thing I don't like are the includes like
"../Something.h". I've avoided this largely by clustering into the current file
structure. However, a few of these includes remain, and in those cases it
doesn't make sense to me to sink an existing file any deeper.
Sanjay Patel [Tue, 7 Mar 2017 19:24:54 +0000 (19:24 +0000)]
fix test to not check optimized IR; NFCI
This test broke with an LLVM instcombine patch (r297166).
I changed the RUN line to only run -mem2reg (to save time checking this large chunk of tests)
and updated the checks using the script attached to D17999:
https://reviews.llvm.org/D17999
The goal is to make this test immune to optimizer changes. If there's something in these
tests that was checking for an IR optimization, that should be tested in LLVM, not Clang.
[clang-format] Support namespaces ending in semicolon
Summary:
This patch adds support for namespaces ending in semicolon to the namespace comment fixer.
source:
```
namespace A {
int i;
int j;
};
```
clang-format before:
```
namespace A {
int i;
int j;
} // namespace A;
```
clang-format after:
```
namespace A {
int i;
int j;
}; // namespace A
```
Martin Bohme [Tue, 7 Mar 2017 08:42:37 +0000 (08:42 +0000)]
[analyzer] Fix crash when building CFG with variable of incomplete type
Summary:
I've included a unit test with a function template containing a variable
of incomplete type. Clang compiles this without errors (the standard
does not require a diagnostic in this case). Without the fix, this case
triggers the crash.
Eric Fiselier [Mon, 6 Mar 2017 23:38:15 +0000 (23:38 +0000)]
[coroutines] Add DependentCoawaitExpr and fix re-building CoroutineBodyStmt.
Summary:
The changes contained in this patch are:
1. Defines a new AST node `CoawaitDependentExpr` for representing co_await expressions while the promise type is still dependent.
2. Correctly detect and transform the 'co_await' operand to `p.await_transform(<expr>)` when possible.
3. Change the initial/final suspend points to build during the initial parse, so they have the correct operator co_await lookup results.
4. Fix transformation of the CoroutineBodyStmt so that it doesn't re-build the final/initial suspends.
@rsmith: This change is a little big, but it's not trivial for me to split it up. Please let me know if you would prefer this submitted as multiple patches.
Eric Fiselier [Mon, 6 Mar 2017 22:52:28 +0000 (22:52 +0000)]
[coroutines] Improve diagnostics when building implicit constructs.
Previously when a coroutine was building the implicit setup/destroy
constructs it would emit diagostics about failures on the first co_await/co_return/co_yield
it encountered. This was confusing because that construct may not itself be ill-formed.
This patch moves the diagnostics to the function start instead.
Reid Kleckner [Mon, 6 Mar 2017 22:18:34 +0000 (22:18 +0000)]
Don't assume cleanup emission preserves dominance in expr evaluation
Summary:
Because of the existence branches out of GNU statement expressions, it
is possible that emitting cleanups for a full expression may cause the
new insertion point to not be dominated by the result of the inner
expression. Consider this example:
struct Foo { Foo(); ~Foo(); int x; };
int g(Foo, int);
int f(bool cond) {
int n = g(Foo(), ({ if (cond) return 0; 42; }));
return n;
}
Before this change, result of the call to 'g' did not dominate its use
in the store to 'n'. The early return exit from the statement expression
branches to a shared cleanup block, which ends in a switch between the
fallthrough destination (the assignment to 'n') or the function exit
block.
This change solves the problem by spilling and reloading expression
evaluation results when any of the active cleanups have branches.
I audited the other call sites of enterFullExpression, and they don't
appear to keep and Values live across the site of the cleanup, except in
ARC code. I wasn't able to create a test case for ARC that exhibits this
problem, though.
projects / clang / log