Erich Keane [Tue, 4 Dec 2018 14:34:09 +0000 (14:34 +0000)]
PTH-- Remove feature entirely-
When debugging a boost build with a modified
version of Clang, I discovered that the PTH implementation
stores TokenKind in 8 bits. However, we currently have 368
TokenKinds.
The result is that the value gets truncated and the wrong token
gets picked up when including PTH files. It seems that this will
go wrong every time someone uses a token that uses the 9th bit.
Upon asking on IRC, it was brought up that this was a highly
experimental features that was considered a failure. I discovered
via googling that BoostBuild (mostly Boost.Math) is the only user of
this
feature, using the CC1 flag directly. I believe that this can be
transferred over to normal PCH with minimal effort:
https://github.com/boostorg/build/issues/367
Based on advice on IRC and research showing that this is a nearly
completely unused feature, this patch removes it entirely.
Note: I considered leaving the build-flags in place and making them
emit an error/warning, however since I've basically identified and
warned the only user, it seemed better to just remove them.
Ulrich Weigand [Tue, 4 Dec 2018 10:51:36 +0000 (10:51 +0000)]
[SystemZ] Do not support __float128
As of rev. 268898, clang supports __float128 on SystemZ. This seems to
have been in error. GCC has never supported __float128 on SystemZ,
since the "long double" type on the platform is already IEEE-128. (GCC
only supports __float128 on platforms where "long double" is some other
data type.)
For compatibility reasons this patch removes __float128 on SystemZ
again. The test case is updated accordingly.
Adam Balogh [Tue, 4 Dec 2018 10:27:27 +0000 (10:27 +0000)]
[Analyzer] Iterator Checker - Forbid decrements past the begin() and increments past the end() of containers
Previously, the iterator range checker only warned upon dereferencing of
iterators outside their valid range as well as increments and decrements of
out-of-range iterators where the result remains out-of-range. However, the C++
standard is more strict than this: decrementing begin() or incrementing end()
results in undefined behaviour even if the iterator is not dereferenced
afterwards. Coming back to the range once out-of-range is also undefined.
This patch corrects the behaviour of the iterator range checker: warnings are
given for any operation whose result is ahead of begin() or past the end()
(which is the past-end iterator itself, thus now we are speaking of past
past-the-end).
Adam Balogh [Tue, 4 Dec 2018 10:22:28 +0000 (10:22 +0000)]
[Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region
If an iterator is represented by a derived C++ class but its comparison operator
is for its base the iterator checkers cannot recognize the iterators compared.
This results in false positives in very straightforward cases (range error when
dereferencing an iterator after disclosing that it is equal to the past-the-end
iterator).
To overcome this problem we always use the region of the topmost base class for
iterators stored in a region. A new method called getMostDerivedObjectRegion()
was added to the MemRegion class to get this region.
Clement Courbet [Tue, 4 Dec 2018 07:59:57 +0000 (07:59 +0000)]
[WIP][Sema] Improve static_assert diagnostics for type traits.
Summary:
In our codebase, `static_assert(std::some_type_trait<Ts...>::value, "msg")`
(where `some_type_trait` is an std type_trait and `Ts...` is the
appropriate template parameters) account for 11.2% of the `static_assert`s.
In these cases, the `Ts` are typically not spelled out explicitly, e.g.
`static_assert(std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value, "message");`
The diagnostic when the assert fails is typically not very useful, e.g.
`static_assert failed due to requirement 'std::is_same<SomeT::TypeT, typename SomeDependentT::value_type>::value' "message"`
This change makes the diagnostic spell out the types explicitly , e.g.
`static_assert failed due to requirement 'std::is_same<int, float>::value' "message"`
See tests for more examples.
After this is submitted, I intend to handle
`static_assert(!std::some_type_trait<Ts...>::value, "msg")`,
which is another 6.6% of static_asserts.
Petr Hosek [Tue, 4 Dec 2018 03:25:25 +0000 (03:25 +0000)]
[Sema] Provide -fvisibility-global-new-delete-hidden option
When the global new and delete operators aren't declared, Clang
provides and implicit declaration, but this declaration currently
always uses the default visibility. This is a problem when the
C++ library itself is being built with non-default visibility because
the implicit declaration will force the new and delete operators to
have the default visibility unlike the rest of the library.
The existing workaround is to use assembly to enforce the visiblity:
https://fuchsia.googlesource.com/zircon/+/master/system/ulib/zxcpp/new.cpp#108
but that solution is not always available, e.g. in the case of of
libFuzzer which is using an internal version of libc++ that's also built
with -fvisibility=hidden where the existing behavior is causing issues.
This change introduces a new option -fvisibility-global-new-delete-hidden
which makes the implicit declaration of the global new and delete
operators hidden.
Richard Smith [Tue, 4 Dec 2018 02:45:28 +0000 (02:45 +0000)]
Fix -Wmismatched-tags to not warn on redeclarations of structs in system
headers.
Previously, we would only check whether the new declaration is in a
system header, but that requires the user to be able to correctly guess
whether a declaration in a system header is declared as a struct or a
class when specializing standard library traits templates.
We now entirely ignore declarations for which the warning was disabled
when determining whether to warn on a tag mismatch.
Also extend the diagnostic message to clarify that
a) code containing such a tag mismatch is in fact valid and correct,
and
b) the (non-coding-style) reason to emit such a warning is that the
Microsoft C++ ABI is broken and includes the tag kind in decorated
names,
as it seems a lot of users are confused by our diagnostic here (either
not understanding why we produce it, or believing that it represents an
actual language rule).
Artem Dergachev [Tue, 4 Dec 2018 02:00:29 +0000 (02:00 +0000)]
[analyzer] MoveChecker: Improve warning and note messages.
The warning piece traditionally describes the bug itself, i.e.
"The bug is a _____", eg. "Attempt to delete released memory",
"Resource leak", "Method call on a moved-from object".
Event pieces produced by the visitor are usually in a present tense, i.e.
"At this moment _____": "Memory is released", "File is closed",
"Object is moved".
Additionally, type information is added into the event pieces for STL objects
(in order to highlight that it is in fact an STL object), and the respective
event piece now mentions that the object is left in an unspecified state
after it was moved, which is a vital piece of information to understand the bug.
Erik Pilkington [Tue, 4 Dec 2018 00:31:31 +0000 (00:31 +0000)]
NFC: Make this test kinder on downstream forks
Downstream forks that have their own attributes often run into this
test failing when a new attribute is added to clang because the
number of supported attributes no longer match. This is redundant
information for this test, so we can get by without it.
Artem Dergachev [Mon, 3 Dec 2018 23:06:07 +0000 (23:06 +0000)]
[analyzer] MoveChecker: Restrict to locals and std:: objects.
In general case there use-after-move is not a bug. It depends on how the
move-constructor or move-assignment is implemented.
In STL, the convention that applies to most classes is that the move-constructor
(-assignment) leaves an object in a "valid but unspecified" state. Using such
object without resetting it to a known state first is likely a bug. Objects
Local value-type variables are special because due to their automatic lifetime
there is no intention to reuse space. If you want a fresh object, you might
as well make a new variable, no need to move from a variable and than re-use it.
Therefore, it is not always a bug, but it is obviously easy to suppress when it
isn't, and in most cases it indeed is - as there's no valid intention behind
the intentional use of a local after move.
This applies not only to local variables but also to parameter variables,
not only of value type but also of rvalue reference type (but not to lvalue
references).
Artem Dergachev [Mon, 3 Dec 2018 22:44:16 +0000 (22:44 +0000)]
[analyzer] MoveChecker: NFC: Remove the workaround for the "zombie symbols" bug.
The checker had extra code to clean up memory regions that were sticking around
in the checker without ever being cleaned up due to the bug that was fixed in
r347953. Because of that, if a region was moved from, then became dead,
and then reincarnated, there were false positives.
Why regions are even allowed to reincarnate is a separate story. Luckily, this
only happens for local regions that don't produce symbols when loaded from.
No functional change intended. The newly added test demonstrates that even
though no cleanup is necessary upon destructor calls, the early return
cannot be removed. It was not failing before the patch.
Artem Dergachev [Mon, 3 Dec 2018 22:23:21 +0000 (22:23 +0000)]
[analyzer] Dump stable identifiers for objects under construction.
This continues the work that was started in r342313, which now gets applied to
object-under-construction tracking in C++. Makes it possible to debug
temporaries by dumping exploded graphs again.
Artem Dergachev [Mon, 3 Dec 2018 22:15:34 +0000 (22:15 +0000)]
[AST] Generate unique identifiers for CXXCtorInitializer objects.
This continues the work started in r342309 and r342315 to provide identifiers
to AST objects that are shorter and easier to read and remember than pointers.
This patch looks at any common prefix between the compilation
directory and the (absolute) file path and strips the redundant
part. More importantly it leaves the compilation directory empty if
the two paths have no common prefix.
After this patch the above entry is (assuming a compilation dir of "/Volumes/Data/llvm/_build"):
Bruno Ricci [Mon, 3 Dec 2018 16:17:45 +0000 (16:17 +0000)]
[Serialization][NFC] Remove pointless "+ 0" in ASTReader
Remove the pointless "+ 0" which I added for some reason when
modifying these statement/expression classes since it looks
like this is a typo. Following the suggestion of aaron.ballman
in D54902. NFC.
Bruno Ricci [Mon, 3 Dec 2018 14:54:03 +0000 (14:54 +0000)]
[AST][Sema] Remove CallExpr::setNumArgs
CallExpr::setNumArgs is the only thing that prevents storing the arguments
in a trailing array. There is only 3 places in Sema where setNumArgs is called.
D54900 dealt with one of them.
This patch remove the other two calls to setNumArgs in ConvertArgumentsForCall.
To do this we do the following changes:
1.) Replace the first call to setNumArgs by an assertion since we are moving the
responsability to allocate enough space for the arguments from
Sema::ConvertArgumentsForCall to its callers
(which are Sema::BuildCallToMemberFunction, and Sema::BuildResolvedCallExpr).
2.) Add a new member function CallExpr::shrinkNumArgs, which can only be used
to drop arguments and then replace the second call to setNumArgs by
shrinkNumArgs.
3.) Add a new defaulted parameter MinNumArgs to CallExpr and its derived
classes which specifies a minimum number of argument slots to allocate.
The actual number of arguments slots allocated will be
max(number of args, MinNumArgs) with the extra args nulled. Note that
after the creation of the call expression all of the arguments will be
non-null. It is just during the creation of the call expression that some of
the last arguments can be temporarily null, until filled by default arguments.
4.) Update Sema::BuildCallToMemberFunction by passing the number of parameters
in the function prototype to the constructor of CXXMemberCallExpr. Here the
change is pretty straightforward.
5.) Update Sema::BuildResolvedCallExpr. Here the change is more complicated
since the type-checking for the function type was done after the creation of
the call expression. We need to move this before the creation of the call
expression, and then pass the number of parameters in the function prototype
(if any) to the constructor of the call expression.
6.) Update the deserialization of CallExpr and its derived classes.
Pablo Barrio [Mon, 3 Dec 2018 14:40:37 +0000 (14:40 +0000)]
[AArch64] Add command-line option for SSBS
Summary:
SSBS (Speculative Store Bypass Safe) is only mandatory from 8.5
onwards but is optional from Armv8.0-A. This patch adds testing for
the ssbs command line option, added to allow enabling the feature
in previous Armv8-A architectures to 8.5.
Bruno Ricci [Mon, 3 Dec 2018 13:23:56 +0000 (13:23 +0000)]
[Sema] Avoid CallExpr::setNumArgs in Sema::BuildCallToObjectOfClassType
CallExpr::setNumArgs is the only thing that prevents storing the arguments
of a call expression in a trailing array since it might resize the argument
array. setNumArgs is only called in 3 places in Sema, and for all of them it
is possible to avoid it.
This deals with the call to setNumArgs in BuildCallToObjectOfClassType.
Instead of constructing the CXXOperatorCallExpr first and later calling
setNumArgs if we have default arguments, we first construct a large
enough SmallVector, do the promotion/check of the arguments, and
then construct the CXXOperatorCallExpr.
Incidentally this also avoid reallocating the arguments when the
call operator has default arguments but this is not the primary goal.
Marco Antognini [Mon, 3 Dec 2018 10:58:56 +0000 (10:58 +0000)]
[OpenCL][Sema] Improve BuildResolvedCallExpr handling of builtins
Summary:
This is a follow-up on https://reviews.llvm.org/D52879, addressing a few issues.
This:
- adds a FIXME for later improvement for specific builtins: I previously have only checked OpenCL ones and ensured tests cover those.
- fixed the CallExpr type.
Stefan Granitz [Mon, 3 Dec 2018 10:34:25 +0000 (10:34 +0000)]
[CMake] Store path to vendor-specific headers in clang-headers target property
Summary:
LLDB.framework wants a copy these headers. With this change LLDB can easily glob for the list of files:
```
get_target_property(clang_include_dir clang-headers RUNTIME_OUTPUT_DIRECTORY)
file(GLOB_RECURSE clang_vendor_headers RELATIVE ${clang_include_dir} "${clang_include_dir}/*")
```
By default `RUNTIME_OUTPUT_DIRECTORY` is unset for custom targets like `clang-headers`.
Matt Arsenault [Sat, 1 Dec 2018 21:56:10 +0000 (21:56 +0000)]
OpenCL: Extend argument promotion rules to vector types
The spec is ambiguous on whether vector types are allowed to be
implicitly converted. The only legal context I think this can
be used for OpenCL is printf, where it seems necessary.
Fangrui Song [Sat, 1 Dec 2018 01:43:05 +0000 (01:43 +0000)]
[Basic] Move DiagnosticsEngine::dump from .h to .cpp
The two LLVM_DUMP_METHOD methods have a undefined reference on clang::DiagnosticsEngine::DiagStateMap::dump.
tools/clang/tools/extra/clangd/benchmarks/IndexBenchmark links in
clangDaemon but does not link in clangBasic explicitly, which causes a
linker error "undefined symbol" in !NDEBUG + -DBUILD_SHARED_LIBS=on builds.
Move LLVM_DUMP_METHOD methods to .cpp to fix IndexBenchmark. They should
be unconditionally defined as they are also used by non-dump-method #pragma clang __debug diag_mapping
Adrian Prantl [Sat, 1 Dec 2018 00:24:27 +0000 (00:24 +0000)]
Honor -fdebug-prefix-map when creating function names for the debug info.
This adds a callback to PrintingPolicy to allow CGDebugInfo to remap
file paths according to -fdebug-prefix-map. Otherwise the debug info
(particularly function names for C++ lambdas) may contain paths that
should have been remapped in the debug info.
Fangrui Song [Fri, 30 Nov 2018 21:26:09 +0000 (21:26 +0000)]
Revert r347417 "Re-Reinstate 347294 with a fix for the failures."
Kept the "indirect_builtin_constant_p" test case in test/SemaCXX/constant-expression-cxx1y.cpp
while we are investigating why the following snippet fails:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};
Fangrui Song [Fri, 30 Nov 2018 21:15:41 +0000 (21:15 +0000)]
[ExprConstant] Try fixing __builtin_constant_p after D54355 (rC347417)
Summary:
Reinstate the original behavior (Success(false, E)) before D54355 when this branch is
taken. This fixes spurious error of the following snippet:
extern char extern_var;
struct { int a; } a = {__builtin_constant_p(extern_var)};
Aaron Ballman [Fri, 30 Nov 2018 20:55:26 +0000 (20:55 +0000)]
Adding tests for -ast-dump; NFC.
This adds tests for struct and union declarations in C. It also points out a bug when dumping anonymous record types -- they are sometimes reported as being contained by something of the wrong tag type. e.g., an anonymous struct inside of a union named X reports the anonymous struct as being inside of 'struct X' rather than 'union X'.
Kristof Umann [Fri, 30 Nov 2018 20:44:00 +0000 (20:44 +0000)]
[analyzer] Evaluate all non-checker config options before analysis
In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.
Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.
However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.
Also, almost all options related class have a similar interface, so uniformity
is also a benefit.
The implementation for errors on invalid input will be commited shorty.
Aaron Ballman [Fri, 30 Nov 2018 18:43:02 +0000 (18:43 +0000)]
Move AST tests into their own test directory; NFC.
This moves everything primarily testing the functionality of -ast-dump and -ast-print into their own directory, rather than leaving the tests spread around the testing directory.
Kadir Cetinkaya [Fri, 30 Nov 2018 17:10:11 +0000 (17:10 +0000)]
[clang] Fill RealPathName for virtual files.
Summary:
Absolute path information for virtual files were missing even if we
have already stat'd the files. This patch puts that information for virtual
files that can succesffully be stat'd.
Adam Balogh [Fri, 30 Nov 2018 10:37:44 +0000 (10:37 +0000)]
lyzer] [HOTFIX!] SValBuilder crash when `aggressive-binary-operation-simplification` enabled
During the review of D41938 a condition check with an early exit accidentally
slipped into a branch, leaving the other branch unprotected. This may result in
an assertion later on. This hotfix moves this contition check outside of the
branch.
Don't generate a checker-tagged node unconditionally on the first
checkDeadSymbols callback when no pointers are tracked.
This is a tiny performance optimization; it may change the behavior slightly
by making Static Analyzer bail out on max-nodes one node later (which is good)
but any test would either break for no good reason or become useless
every time someone sneezes.
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.
projects / clang / log