Commit Graph

3271 Commits

Author SHA1 Message Date
Yitzhak Mandelbaum 93fbaa46c8 Revert "Revert "[clang][dataflow] Add framework for testing analyses.""
This reverts commit 78ff12da11 and fixes the initial cause of the revert.
2021-12-11 23:16:59 +00:00
Nico Weber 78ff12da11 Revert "[clang][dataflow] Add framework for testing analyses."
Doesn't build on Windows.

This reverts commit 5a40df6381
and commit db494bd4e8.
2021-12-10 11:06:40 -05:00
Yitzhak Mandelbaum 5a40df6381 [clang][dataflow] Add framework for testing analyses.
Adds a general-purpose framework to support testing of dataflow analyses.

Differential Revision: https://reviews.llvm.org/D115341
2021-12-10 15:24:12 +00:00
Yitzhak Mandelbaum 28d3976819 Revert "[clang][dataflow] Add framework for testing analyses."
This reverts commit 47d526d67e.

The commit is failing to build on some platforms. Rolling back while we investigate.
2021-12-10 14:27:15 +00:00
Yitzhak Mandelbaum 47d526d67e [clang][dataflow] Add framework for testing analyses.
Adds a general-purpose framework to support testing of dataflow analyses.

Differential Revision: https://reviews.llvm.org/D115341
2021-12-10 14:00:36 +00:00
Stanislav Gatev 8dcaf3aa0b [clang][dataflow] Implement a basic algorithm for dataflow analysis
This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed By: xazax.hun, gribozavr2

Differential Revision: https://reviews.llvm.org/D115235
2021-12-10 11:44:49 +01:00
Logan Smith 715c72b4fb [NFC][analyzer] Return underlying strings directly instead of OS.str()
This avoids an unnecessary copy required by 'return OS.str()', allowing
instead for NRVO or implicit move. The .str() call (which flushes the
stream) is no longer required since 65b13610a5,
which made raw_string_ostream unbuffered by default.

Differential Revision: https://reviews.llvm.org/D115374
2021-12-09 16:05:46 -08:00
Benjamin Kramer 4c7de4fbda Thread safety analysis: Remove unused variable. NFC. 2021-12-09 18:57:43 +01:00
Bill Wendling c4582a689c [Analysis] Ignore casts and unary ops for uninitialized values
A series of unary operators and casts may obscure the variable we're
trying to analyze. Ignore them for the uninitialized value analysis.
Other checks determine if the unary operators result in a valid l-value.

Link: https://github.com/ClangBuiltLinux/linux/issues/1521

Reviewed By: nickdesaulniers

Differential Revision: https://reviews.llvm.org/D114848
2021-12-07 11:49:18 -08:00
Stanislav Gatev 6e2aecd759 [clang][dataflow] Add base types for building dataflow analyses
This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.

Reviewed By: ymandel, xazax.hun, gribozavr2

Differential Revision: https://reviews.llvm.org/D114234
2021-11-30 14:44:40 +00:00
Zarko Todorovski d8e5a0c42b [clang][NFC] Inclusive terms: replace some uses of sanity in clang
Rewording of comments to avoid using `sanity test, sanity check`.

Reviewed By: aaron.ballman, Quuxplusone

Differential Revision: https://reviews.llvm.org/D114025
2021-11-19 14:58:35 -05:00
Kazu Hirata eb1c7c1339 [AST, Analysis] Use llvm::reverse (NFC) 2021-11-07 09:53:14 -08:00
Nico Weber c7aaa2efef [clang] Add range accessor for ObjCAtTryStmt catch_stmts and use it
No behavior change.

Differential Revision: https://reviews.llvm.org/D112543
2021-10-27 08:57:05 -04:00
Nico Weber 04f30795f1 [clang] Implement CFG construction for @try and @catch
@finally is still not implemented.

With this, clang can emit -Wreturn-type warnings for functions containing
@try/@catch (but not yet @finally), and -Wunreachable-code also works for those
functions.

The implementation is similar to D36914.

Part of PR46693.

Differential Revision: https://reviews.llvm.org/D112287
2021-10-26 09:45:22 -04:00
Nico Weber 0b7c9addce [clang] Make loop in CFGBuilder::VisitCXXTryStmt() more canonical
No behavior change.
2021-10-26 09:45:22 -04:00
Nico Weber aa42785d01 [clang] Simplify CFG block printing code a bit
No behavior change.
2021-10-26 09:45:22 -04:00
Nico Weber d054b31d59 [clang] Use consistent punctuation at end of Block NULL comment
No behavior change.
2021-10-26 09:45:22 -04:00
Kazu Hirata 4bd46501c3 Use llvm::any_of and llvm::none_of (NFC) 2021-10-24 17:35:33 -07:00
Kazu Hirata 7cc8fa2dd2 Use llvm::is_contained (NFC) 2021-10-24 09:32:57 -07:00
Kazu Hirata dccfaddc6b [clang] Use StringRef::contains (NFC) 2021-10-21 08:58:19 -07:00
Nico Weber c74ab84ea2 [clang] Omit most AttributedStatements from the CFG
`[[clang::fallthrough]]` has meaning for the CFG, but all other
StmtAttrs we currently have don't. So omit them, as AttributedStatements
with children cause several issues and there's no benefit in including
them.

Fixes PR52103 and PR49454. See PR52103 for details.

Differential Revision: https://reviews.llvm.org/D111568
2021-10-12 09:15:45 -04:00
Nico Weber 5ab2a95edb [clang] Remove an else-after-return 2021-10-11 14:24:58 -04:00
Nico Weber 00ca004dda [clang] Convert a few loops to for-each 2021-10-11 14:24:32 -04:00
Nico Weber 144f851f6f [clang/CFG] Rewrap a line to 80 columns 2021-10-11 14:23:51 -04:00
Nico Weber 62abc1842b clang: Add range-based CFG::try_blocks()
..and use it. No behavior change.
2021-10-10 15:15:37 -04:00
Nico Weber 23d5fe6235 clang: Convert two loops to for-each
And rewrap a line at 80 columns while here. No behavior change.
2021-10-10 14:55:46 -04:00
Corentin Jabot 424733c12a Implement if consteval (P1938)
Modify the IfStmt node to suppoort constant evaluated expressions.

Add a new ExpressionEvaluationContext::ImmediateFunctionContext to
keep track of immediate function contexts.

This proved easier/better/probably more efficient than walking the AST
backward as it allows diagnosing nested if consteval statements.
2021-10-05 08:04:14 -04:00
Aaron Puchert 6de19ea4b6 Thread safety analysis: Drop special block handling
Previous changes like D101202 and D104261 have eliminated the special
status that break and continue once had, since now we're making
decisions purely based on the structure of the CFG without regard for
the underlying source code constructs.

This means we don't gain anything from defering handling for these
blocks. Dropping it moves some diagnostics, though arguably into a
better place. We're working around a "quirk" in the CFG that perhaps
wasn't visible before: while loops have an empty "transition block"
where continue statements and the regular loop exit meet, before
continuing to the loop entry. To get a source location for that, we
slightly extend our handling for empty blocks. The source location for
the transition ends up to be the loop entry then, but formally this
isn't a back edge. We pretend it is anyway. (This is safe: we can always
treat edges as back edges, it just means we allow less and don't modify
the lock set. The other way around it wouldn't be safe.)

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D106715
2021-09-20 15:20:15 +02:00
Aaron Puchert 9b889f826f Thread safety analysis: Warn when demoting locks on back edges
Previously in D104261 we warned about dropping locks from back edges,
this is the corresponding change for exclusive/shared joins. If we're
entering the loop with an exclusive change, which is then relaxed to a
shared lock before we loop back, we have already analyzed the loop body
with the stronger exclusive lock and thus might have false positives.

There is a minor non-observable change: we modify the exit lock set of a
function, but since that isn't used further it doesn't change anything.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D106713
2021-09-18 13:46:55 +02:00
Balazs Benics d3e14fafc6 [analyzer][NFC] Display the correct function name even in crash dumps
The `-analyzer-display-progress` displayed the function name of the
currently analyzed function. It differs in C and C++. In C++, it
prints the argument types as well in a comma-separated list.
While in C, only the function name is displayed, without the brackets.
E.g.:

  C++: foo(), foo(int, float)
  C:   foo

In crash traces, the analyzer dumps the location contexts, but the
string is not enough for `-analyze-function` in C++ mode.
This patch addresses the issue by dumping the proper function names
even in stack traces.

Reviewed By: NoQ

Differential Revision: https://reviews.llvm.org/D105708
2021-07-12 09:06:46 +02:00
David Blaikie 1def2579e1 PR51018: Remove explicit conversions from SmallString to StringRef to future-proof against C++23
C++23 will make these conversions ambiguous - so fix them to make the
codebase forward-compatible with C++23 (& a follow-up change I've made
will make this ambiguous/invalid even in <C++23 so we don't regress
this & it generally improves the code anyway)
2021-07-08 13:37:57 -07:00
Aaron Puchert e0b90771c3 Thread safety analysis: Rename parameters of ThreadSafetyAnalyzer::intersectAndWarn (NFC)
In D104261 we made the parameters' meaning slightly more specific, this
changes their names accordingly. In all uses we're building a new lock
set by intersecting existing locksets. The first (modifiable) argument
is the new lock set being built, the second (non-modifiable) argument is
the exit set of a preceding block.

Reviewed By: aaron.ballman, delesley

Differential Revision: https://reviews.llvm.org/D104649
2021-06-29 23:56:52 +02:00
Aaron Puchert f664e2ec37 Thread safety analysis: Always warn when dropping locks on back edges
We allow branches to join where one holds a managed lock but the other
doesn't, but we can't do so for back edges: because there we can't drop
them from the lockset, as we have already analyzed the loop with the
larger lockset. So we can't allow dropping managed locks on back edges.

We move the managed() check from handleRemovalFromIntersection up to
intersectAndWarn, where we additionally check if we're on a back edge if
we're removing from the first lock set (the entry set of the next block)
but not if we're removing from the second lock set (the exit set of the
previous block). Now that the order of arguments matters, I had to swap
them in one invocation, which also causes some minor differences in the
tests.

Reviewed By: delesley

Differential Revision: https://reviews.llvm.org/D104261
2021-06-29 23:56:52 +02:00
Martin Storsjö e5c7c171e5 [clang] Rename StringRef _lower() method calls to _insensitive()
This is mostly a mechanical change, but a testcase that contains
parts of the StringRef class (clang/test/Analysis/llvm-conventions.cpp)
isn't touched.
2021-06-25 00:22:01 +03:00
Simon Pilgrim 61cdaf66fe [ADT] Remove APInt/APSInt toString() std::string variants
<string> is currently the highest impact header in a clang+llvm build:

https://commondatastorage.googleapis.com/chromium-browser-clang/llvm-include-analysis.html

One of the most common places this is being included is the APInt.h header, which needs it for an old toString() implementation that returns std::string - an inefficient method compared to the SmallString versions that it actually wraps.

This patch replaces these APInt/APSInt methods with a pair of llvm::toString() helpers inside StringExtras.h, adjusts users accordingly and removes the <string> from APInt.h - I was hoping that more of these users could be converted to use the SmallString methods, but it appears that most end up creating a std::string anyhow. I avoided trying to use the raw_ostream << operators as well as I didn't want to lose having the integer radix explicit in the code.

Differential Revision: https://reviews.llvm.org/D103888
2021-06-11 13:19:15 +01:00
Matheus Izvekov aef5d8fdc7 [clang] NFC: Rename rvalue to prvalue
This renames the expression value categories from rvalue to prvalue,
keeping nomenclature consistent with C++11 onwards.

C++ has the most complicated taxonomy here, and every other language
only uses a subset of it, so it's less confusing to use the C++ names
consistently, and mentally remap to the C names when working on that
context (prvalue -> rvalue, no xvalues, etc).

Renames:
* VK_RValue -> VK_PRValue
* Expr::isRValue -> Expr::isPRValue
* SK_QualificationConversionRValue -> SK_QualificationConversionPRValue
* JSON AST Dumper Expression nodes value category: "rvalue" -> "prvalue"

Signed-off-by: Matheus Izvekov <mizvekov@gmail.com>

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D103720
2021-06-09 12:27:10 +02:00
Georgeta Igna 50f17e9d31 [analyzer] RetainCountChecker: Disable reference counting for OSMetaClass.
It is a reference-counted class but it uses different methods for that
and the checker doesn't understand them yet.

Differential Revision: https://reviews.llvm.org/D103081
2021-05-27 13:12:19 -07:00
Aaron Puchert cf0b337c1b Thread safety analysis: Allow exlusive/shared joins for managed and asserted capabilities
Similar to how we allow managed and asserted locks to be held and not
held in joining branches, we also allow them to be held shared and
exclusive. The scoped lock should restore the original state at the end
of the scope in any event, and asserted locks need not be released.

We should probably only allow asserted locks to be subsumed by managed,
not by (directly) acquired locks, but that's for another change.

Reviewed By: delesley

Differential Revision: https://reviews.llvm.org/D102026
2021-05-27 17:46:04 +02:00
Aaron Puchert 3d64677c28 Thread safety analysis: Factor out function for merging locks (NFC)
It's going to become a bit more complicated, so let's have it separate.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D102025
2021-05-27 17:44:48 +02:00
Tomasz Kamiński 058f384ae9 [analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr
Previously, information about `ConstructionContextLayer` was not
propagated thru causing the expression like:

  Var c = (createVar());

To produce unrelated temporary for the `createVar()` result and conjure
a new symbol for the value of `c` in C++17 mode.

Reviewed By: steakhal

Patch By: tomasz-kaminski-sonarsource!

Differential Revision: https://reviews.llvm.org/D102835
2021-05-24 10:16:52 +02:00
Pratyush Das 99d63ccff0 Add type information to integral template argument if required.
Non-comprehensive list of cases:
 * Dumping template arguments;
 * Corresponding parameter contains a deduced type;
 * Template arguments are for a DeclRefExpr that hadMultipleCandidates()

Type information is added in the form of prefixes (u8, u, U, L),
suffixes (U, L, UL, LL, ULL) or explicit casts to printed integral template
argument, if MSVC codeview mode is disabled.

Differential revision: https://reviews.llvm.org/D77598
2021-05-12 19:00:08 +00:00
Aaron Puchert d21e1b79ff Thread safety analysis: Eliminate parameter from intersectAndWarn (NFC)
We were modifying precisely when intersecting the lock sets of multiple
predecessors without back edge. That's no coincidence: we can't modify
on back edges, it doesn't make sense to modify at the end of a function,
and otherwise we always want to intersect on forward edges, because we
can build a new lock set for those.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D101755
2021-05-06 23:07:42 +02:00
Aaron Puchert daca6edb31 Thread safety analysis: Fix false negative on break
We weren't modifying the lock set when intersecting with one coming
from a break-terminated block. This is inconsistent, since break isn't a
back edge, and it leads to false negatives with scoped locks. We usually
don't warn for those when joining locksets aren't the same, we just
silently remove locks that are not in the intersection. But not warning
and not removing them isn't right.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D101202
2021-05-03 14:03:17 +02:00
Aaron Puchert 530e074faa Thread safety analysis: Replace flags in FactEntry by SourceKind (NFC)
The motivation here is to make it available in the base class whether a
fact is managed or not. That would have meant three flags on the base
class, so I had a look whether we really have 8 possible combinations.

It turns out we don't: asserted and declared are obviously mutually
exclusive. Managed facts are only created when we acquire a capability
through a scoped capability. Adopting an asserted or declared lock will
not (in fact can not, because Facts are immutable) make them managed.

We probably don't want to allow adopting an asserted lock (because then
the function should probably have a release attribute, and then the
assertion is pointless), but we might at some point decide to replace a
declared fact on adoption.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D100801
2021-05-03 14:03:17 +02:00
Aaron Puchert 572fe08776 Thread safety analysis: Simplify intersectAndWarn (NFC)
Instead of conditionally overwriting a nullptr and then branching on its
nullness, just branch directly on the original condition. Then we can
make both pointers (non-null) references instead.
2021-04-23 23:19:15 +02:00
Valeriy Savchenko 77f1e096e8 [-Wcompletion-handler] Don't recognize init methods as conventional
rdar://75704162

Differential Revision: https://reviews.llvm.org/D99601
2021-04-07 13:50:01 +03:00
Valeriy Savchenko 4821c15691 [analyzer] Fix body farm for Obj-C++ properties
When property is declared in a superclass (or in a protocol),
it still can be of CXXRecord type and Sema could've already
generated a body for us.  This patch joins two branches and
two ways of acquiring IVar in order to reuse the existing code.
And prevent us from generating l-value to r-value casts for
C++ types.

rdar://67416721

Differential Revision: https://reviews.llvm.org/D99194
2021-04-07 13:44:43 +03:00
Aaron Puchert dfec26b186 Thread safety analysis: Don't warn about managed locks on join points
We already did so for scoped locks acquired in the constructor, this
change extends the treatment to deferred locks and scoped unlocking, so
locks acquired outside of the constructor. Obviously this makes things
more consistent.

Originally I thought this was a bad idea, because obviously it
introduces false negatives when it comes to double locking, but these
are typically easily found in tests, and the primary goal of the Thread
safety analysis is not to find double locks but race conditions.
Since the scoped lock will release the mutex anyway when the scope ends,
the inconsistent state is just temporary and probably fine.

Reviewed By: delesley

Differential Revision: https://reviews.llvm.org/D98747
2021-04-06 22:29:48 +02:00
Simon Pilgrim 2901dc7575 Don't directly dereference getAs<> casts to avoid potential null dereferences. NFCI.
Replace with castAs<> which asserts the cast is valid.

Fixes a number of static analyzer warnings.
2021-04-06 12:24:19 +01:00
Aaron Puchert c61ae6e6d5 Deduplicate branches and adjust comment [NFC]
Currently we want to allow calling non-const methods even when only a
shared lock is held, because -Wthread-safety-reference is already quite
sensitive and not all code is const-correct. Even if it is, this might
require users to add std::as_const around the implicit object argument.

See D52395 for a discussion.

Fixes PR46963.
2021-03-27 23:08:43 +01:00