[analyzer] Track terminator conditions on which a tracked expression depends
This patch is a major part of my GSoC project, aimed to improve the bug
reports of the analyzer.
TL;DR: Help the analyzer understand that some conditions are important,
and should be explained better. If an CFGBlock is a control dependency
of a block where an expression value is tracked, explain the condition
expression better by tracking it.
if (A) // let's explain why we believe A to be true
10 / x; // division by zero
This is an experimental feature, and can be enabled by the
off-by-default analyzer configuration "track-conditions".
In detail:
This idea was inspired by the program slicing algorithm. Essentially,
two things are used to produce a program slice (a subset of the program
relevant to a (statement, variable) pair): data and control
dependencies. The bug path (the linear path in the ExplodedGraph that leads
from the beginning of the analysis to the error node) enables to
analyzer to argue about data dependencies with relative ease.
Control dependencies are a different slice of the cake entirely.
Just because we reached a branch during symbolic execution, it
doesn't mean that that particular branch has any effect on whether the
bug would've occured. This means that we can't simply rely on the bug
path to gather control dependencies.
In previous patches, LLVM's IDFCalculator, which works on a control flow
graph rather than the ExplodedGraph was generalized to solve this issue.
We use this information to heuristically guess that the value of a tracked
expression depends greatly on it's control dependencies, and start
tracking them as well.
After plenty of evaluations this was seen as great idea, but still
lacking refinements (we should have different descriptions about a
conditions value), hence it's off-by-default.
[analyzer][IDF] Add a control dependency calculator + a new debug checker
I intend to improve the analyzer's bug reports by tracking condition
expressions.
01 bool b = messyComputation();
02 int i = 0;
03 if (b) // control dependency of the bug site, let's explain why we assume val
04 // to be true
05 10 / i; // warn: division by zero
I'll detail this heuristic in the followup patch, strictly related to this one
however:
* Create the new ControlDependencyCalculator class that uses llvm::IDFCalculator
to (lazily) calculate control dependencies for Clang's CFG.
* A new debug checker debug.DumpControlDependencies is added for lit tests
* Add unittests
Make joined instances of JoinedOrSeparate flags point to the unaliased args, like all other arg types do
This fixes an 8-year-old regression. r105763 made it so that aliases
always refer to the unaliased option – but it missed the "joined" branch
of JoinedOrSeparate flags. (r162231 then made the Args classes
non-virtual, and r169344 moved them from clang to llvm.)
Back then, there was no JoinedOrSeparate flag that was an alias, so it
wasn't observable. Now /U in CLCompatOptions is a JoinedOrSeparate alias
in clang, and warn_slash_u_filename incorrectly used the aliased arg id
(using the unaliased one isn't really a regression since that warning
checks if the undefined macro contains slash or backslash and only then
emits the warning – and no valid use will pass "-Ufoo/bar" or similar).
Also, lld has many JoinedOrSeparate aliases, and due to this bug it had
to explicitly call `getUnaliasedOption()` in a bunch of places, even
though that shouldn't be necessary by design. After this fix in Option,
these calls really don't have an effect any more, so remove them.
No intended behavior change.
(I accidentally fixed this bug while working on PR29106 but then
wondered why the warn_slash_u_filename broke. When I figured it out, I
thought it would make sense to land this in a separate commit.)
Mikael Holmen [Fri, 5 Jul 2019 06:12:24 +0000 (06:12 +0000)]
Silence gcc warning "control reaches end of non-void function" [NFCI]
Without this fix gcc (7.4) complains with
/data/repo/master/clang/lib/CodeGen/CGObjCMac.cpp: In member function 'std::__cxx11::string {anonymous}::CGObjCCommonMac::GetSectionName(llvm::StringRef, llvm::StringRef)':
/data/repo/master/clang/lib/CodeGen/CGObjCMac.cpp:4944:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
All values in the ObjectFormatType enum are currently handled in the switch
but gcc complains anyway.
[NFC] Make some ObjectFormatType switches covering
Summary:
This patch removes the `default` case from some switches on
`llvm::Triple::ObjectFormatType`, and cases for the missing enumerators
are then added.
For `UnknownObjectFormat`, the action (`llvm_unreachable`) for the
`default` case is kept.
For the other unhandled cases, `report_fatal_error` is used instead.
Fangrui Song [Thu, 4 Jul 2019 04:44:42 +0000 (04:44 +0000)]
[PowerPC] Support constraint code "ww"
Summary:
"ww" and "ws" are both constraint codes for VSX vector registers that
hold scalar double data. "ww" is preferred for float while "ws" is
preferred for double.
However, even though the path of execution in B2 only depends on C's value,
CFGBlock::getCondition() would return the entire condition (A && B && C). For
B3, it would return A && B. I changed this the actual condition.
[analyzer][Dominator] Add post dominators to CFG + a new debug checker
Transform clang::DominatorTree to be able to also calculate post dominators.
* Tidy up the documentation
* Make it clang::DominatorTree template class (similarly to how
llvm::DominatorTreeBase works), rename it to clang::CFGDominatorTreeImpl
* Clang's dominator tree is now called clang::CFGDomTree
* Clang's brand new post dominator tree is called clang::CFGPostDomTree
* Add a lot of asserts to the dump() function
* Create a new checker to test the functionality
In Clang's CFG, we use nullpointers to represent unreachable nodes, for
example, in the included testfile, block B0 is unreachable from block
B1, resulting in a nullpointer dereference somewhere in
llvm::DominatorTreeBase<clang::CFGBlock, false>::recalculate.
This patch fixes this issue by specializing
llvm::DomTreeBuilder::SemiNCAInfo::ChildrenGetter::Get for
clang::CFG to not contain nullpointer successors.
[clang-tidy] Fix the YAML created for checks like modernize-pass-by-value
Currently this check generates the replacement with the newline in the end.
The proper way to export it to YAML is to have two \n\n instead of one.
Without this fix clients should reinterpret the replacement as
"#include <utility> " instead of "#include <utility>\n"
[clang][HeaderSearch] Shorten paths for includes in mainfile's directory
Summary:
Currently HeaderSearch only looks at SearchDir's passed into it, but in
addition to those paths headers can be relative to including file's directory.
[analyzer] exploded-graph-rewriter: Collapse very long statement pretty-prints.
When printing various statements that include braces (compound
statements, lambda expressions, statement-expressions, etc.),
replace the code between braces with '...'.
Erik Pilkington [Tue, 2 Jul 2019 18:28:13 +0000 (18:28 +0000)]
[C++2a] Add __builtin_bit_cast, used to implement std::bit_cast
This commit adds a new builtin, __builtin_bit_cast(T, v), which performs a
bit_cast from a value v to a type T. This expression can be evaluated at
compile time under specific circumstances.
The compile time evaluation currently doesn't support bit-fields, but I'm
planning on fixing this in a follow up (some of the logic for figuring this out
is in CodeGen). I'm also planning follow-ups for supporting some more esoteric
types that the constexpr evaluator supports, as well as extending
__builtin_memcpy constexpr evaluation to use the same infrastructure.
Sam McCall [Tue, 2 Jul 2019 15:53:14 +0000 (15:53 +0000)]
clang-format: Add new style option AlignConsecutiveMacros
This option behaves similarly to AlignConsecutiveDeclarations and
AlignConsecutiveAssignments, aligning the assignment of C/C++
preprocessor macros on consecutive lines.
I've worked in many projects (embedded, mostly) where header files full
of large, well-aligned "#define" blocks are a common pattern. We
normally avoid using clang-format on these files, since it ruins any
existing alignment in said blocks. This style option will align "simple"
PP macros (no parameters) and PP macros with parameter lists on
consecutive lines.
Related Bugzilla entry (thanks mcuddie):
https://llvm.org/bugs/show_bug.cgi?id=20637
[LibTooling] Extend `RewriteRule` with support for adding includes.
Summary:
This revision allows users to specify the insertion of an included directive (at
the top of the file being rewritten) as part of a rewrite rule. These
directives are bundled with `RewriteRule` cases, so that different cases can
potentially result in different include actions.
[analyzer] exploded-graph-rewriter: Improve program point dumps.
- Take advantage of the stmt_point_kind.
- Dump block IDs for BlockEntrance nodes.
- Don't dump huge compound statements on PurgeDeadSymbols nodes.
- Rename Edge to BlockEdge for consistency.
- Tweak colors.
Summary:
kmalloc is freed with kfree in the linux kernel. kmalloc support was
added in r204832, but kfree was not. Adding kfree fixes incorrectly
detected memory leaks.
[analyzer] Fix invalidation when returning into a ctor initializer.
Due to RVO the target region of a function that returns an object by
value isn't necessarily a temporary object region; it may be an
arbitrary memory region. In particular, it may be a field of a bigger
object.
Make sure we don't invalidate the bigger object when said function is
evaluated conservatively.
[analyzer] NonnullGlobalConstants: Don't be confused by a _Nonnull attribute.
The NonnullGlobalConstants checker models the rule "it doesn't make sense
to make a constant global pointer and initialize it to null"; it makes sure
that whatever it's initialized with is known to be non-null.
Ironically, annotating the type of the pointer as _Nonnull breaks the checker.
Fix handling of the _Nonnull annotation so that it was instead one more reason
to believe that the value is non-null.
[analyzer] CStringChecker: Modernize to use CallDescriptions.
This patch uses the new CDF_MaybeBuiltin flag to handle C library functions.
It's mostly an NFC/refactoring pass, but it does fix a bug in handling memset()
when it expands to __builtin___memset_chk() because the latter has
one more argument and memset() handling code was trying to match
the exact number of arguments. Now the code is deduplicated and there's
less room for mistakes.
[analyzer] NFC: CallDescription: Implement describing C library functions.
When matching C standard library functions in the checker, it's easy to forget
that they are often implemented as macros that are expanded to builtins.
Such builtins would have a different name, so matching the callee identifier
would fail, or may sometimes have more arguments than expected, so matching
the exact number of arguments would fail, but this is fine as long as we have
all the arguments that we need in their respective places.
This patch adds a set of flags to the CallDescription class so that to handle
various special matching rules, and adds the first flag into this set,
which enables a more fuzzy matching for functions that
may be implemented as compiler builtins.
D60974 added tests which incorrectly assume that llvm-readelf is available. This is a bad assumption, it should instead declare the dependency explicitly in the tests.
[OPENMP]Fix handling of lambda captures in target regions.
Previously, lambda captures were processed in the function called during
capturing the variables. It leads to the recursive functions calls and
may result in the compiler crash.
Summary:
Now we store the errors for the Decls in the "to" context too. For
that, however, we have to put these errors in a shared state (among all
the ASTImporter objects which handle the same "to" context but different
"from" contexts).
After a series of imports from different "from" TUs we have a "to" context
which may have erroneous nodes in it. (Remember, the AST is immutable so
there is no way to delete a node once we had created it and we realized
the error later.) All these erroneous nodes are marked in
ASTImporterSharedState::ImportErrors. Clients of the ASTImporter may
use this as an input. E.g. the static analyzer engine may not try to
analyze a function if that is marked as erroneous (it can be queried via
ASTImporterSharedState::getImportDeclErrorIfAny()).
Sam Elliott [Mon, 1 Jul 2019 14:53:56 +0000 (14:53 +0000)]
[RISCV] Avoid save-restore target feature warning
Summary:
LLVM issues a warning if passed unknown target features. Neither I nor
@asb noticed this until after https://reviews.llvm.org/D63498 landed.
This patch stops passing the (unknown) "save-restore" target feature to
the LLVM backend, but continues to emit a warning if a driver asks for
`-msave-restore`. The default of assuming `-mno-save-restore` (and
emitting no warnings) remains.
Summary:
During import of a specific Decl D, it may happen that some AST nodes
had already been created before we recognize an error. In this case we
signal back the error to the caller, but the "to" context remains
polluted with those nodes which had been created. Ideally, those nodes
should not had been created, but that time we did not know about the
error, the error happened later. Since the AST is immutable (most of
the cases we can't remove existing nodes) we choose to mark these nodes
as erroneous.
Here are the steps of the algorithm:
1) We keep track of the nodes which we visit during the import of D: See
ImportPathTy.
2) If a Decl is already imported and it is already on the import path
(we have a cycle) then we copy/store the relevant part of the import
path. We store these cycles for each Decl.
3) When we recognize an error during the import of D then we set up this
error to all Decls in the stored cycles for D and we clear the stored
cycles.
[ASTImporter] Propagate error from ImportDeclContext
Summary:
During analysis of one project we failed to import one
CXXDestructorDecl. But since we did not propagate the error in
importDeclContext we had a CXXRecordDecl without a destructor. Then the
analyzer engine had a CallEvent where the nonexistent dtor was requested
(crash).
Solution is to propagate the errors we have during importing a
DeclContext.
Mike Spertus [Sun, 30 Jun 2019 22:04:25 +0000 (22:04 +0000)]
Various tweaks to MSVC natvis visualizers
Make more consistent use of na format.
Improve visualization of deduction guides.
Add visualizer for explicit specifier (including conditionally explicit)
Fix some typos
Alexey Bataev [Fri, 28 Jun 2019 20:45:14 +0000 (20:45 +0000)]
[OPENMP]Improve analysis of implicit captures.
If the variable is used in the OpenMP region implicitly, we need to
check the data-sharing attributes for such variables and generate
implicit clauses for them. Patch improves analysis of such variables for
better handling of data-sharing rules.
Alexey Bataev [Fri, 28 Jun 2019 16:16:00 +0000 (16:16 +0000)]
[OPENMP]Fix top DSA for static members.
Fixed handling of the data-sharing attributes for static members when
requesting top most attribute. Previously, it might return the incorrect
attributes for static members if they were overriden in the outer
constructs.
Alexey Bataev [Fri, 28 Jun 2019 15:16:37 +0000 (15:16 +0000)]
[OPENMP]Fix DSA for loop iteration variables in simd loops.
According to the OpenMP 5.0 standard, the loop iteration variable in the associated
for-loop of a simd construct with just one associated for-loop may be
listed in a private, lastprivate, or linear clause with a linear-step
that is the increment of the associated for-loop. Also, the loop
teration variables in the associated for-loops of a simd construct with
multiple associated for-loops may be listed in a private or lastprivate
clause.
Alexey Bataev [Fri, 28 Jun 2019 14:59:25 +0000 (14:59 +0000)]
[OPENMP]Fix checks for DSA in simd constructs.
The errors for incorrectly specified data-sharing attributes for simd
constructs must be emitted only for the explicitly provided clauses, not
the predetermined ones.
Aaron Puchert [Wed, 26 Jun 2019 21:36:35 +0000 (21:36 +0000)]
[Clang] Remove unused -split-dwarf and obsolete -enable-split-dwarf
Summary:
The changes in D59673 made the choice redundant, since we can achieve
single-file split DWARF just by not setting an output file name.
Like llc we can also derive whether to enable Split DWARF from whether
-split-dwarf-file is set, so we don't need the flag at all anymore.
The test CodeGen/split-debug-filename.c distinguished between having set
or not set -enable-split-dwarf with -split-dwarf-file, but we can
probably just always emit the metadata into the IR.
Alex Lorenz [Wed, 26 Jun 2019 21:11:51 +0000 (21:11 +0000)]
[clang-scan-deps] Introduce the DependencyScanning library with the
thread worker code and better error handling
This commit extracts out the code that will powers the fast scanning
worker into a new file in a new DependencyScanning library. The error
and output handling is improved so that the clients can gather
errors/results from the worker directly.
JF Bastien [Wed, 26 Jun 2019 19:50:12 +0000 (19:50 +0000)]
BitStream reader: propagate errors
The bitstream reader handles errors poorly. This has two effects:
* Bugs in file handling (especially modules) manifest as an "unexpected end of
file" crash
* Users of clang as a library end up aborting because the code unconditionally
calls `report_fatal_error`
The bitstream reader should be more resilient and return Expected / Error as
soon as an error is encountered, not way late like it does now. This patch
starts doing so and adopting the error handling where I think it makes sense.
There's plenty more to do: this patch propagates errors to be minimally useful,
and follow-ups will propagate them further and improve diagnostics.
Xing Xue [Wed, 26 Jun 2019 19:27:16 +0000 (19:27 +0000)]
Print NULL as "(null)" in diagnostic message
Summary:
Passing a null pointer to the printf family for a %s format specifier leads to undefined behaviour. The tests currently expect (null). Explicitly test for a null pointer and provide the expected string.
Without an explicit declaration for placement new, clang would reject
uses of placement new with "'default new' is not supported in OpenCL
C++". This may mislead users into thinking that placement new is not
supported, see e.g. PR42060.
Clarify that placement new requires an explicit declaration.
Kadir Cetinkaya [Wed, 26 Jun 2019 07:39:03 +0000 (07:39 +0000)]
[clang][Tooling] Infer target and mode from argv[0] when using JSONCompilationDatabase
Summary:
Wraps JSON compilation database with a target and mode adding database
wrapper. So that driver can correctly figure out which toolchain to use.
Note that clients that wants to make use of this target discovery mechanism
needs to link in TargetsInfos and initialize them at startup.
Yaxun Liu [Wed, 26 Jun 2019 03:47:37 +0000 (03:47 +0000)]
[HIP] Support attribute hip_pinned_shadow
This patch introduces support of hip_pinned_shadow variable for HIP.
A hip_pinned_shadow variable is a global variable with attribute hip_pinned_shadow.
It has external linkage on device side and has no initializer. It has internal
linkage on host side and has initializer or static constructor. It can be accessed
in both device code and host code.
This allows HIP runtime to implement support of HIP texture reference.
projects / clang / log