-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LifetimeSafety] Add statistics for missing origin #166163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-temporal-safety Author: None (DEBADRIBASAK) ChangesThis PR adds the implementation for PrintStats function in LifetimeSafetyAnalysis class. Purpose: This utility function is added to track the expression types with missing origin. While retrieving the origins from origin manager, some expressions show missing origins. Currently these are created on the fly using getOrCreate function. For analysing the coverage of the check, it will be necessary to see what kind of expressions have a missing origin. Approach: A static map from strings to counts has been added to the LifetimeSafetyAnalysis class. The OriginManager also internally tracks the count of missing origins using ExprTypeToMissingOriginCount map. After analysing each file, the missing origin stats are accumulated in the LifetimeSafetyAnalysis class, in the UpdateMissingOriginCount class. Example output: For the file llvm-project/llvm/lib/Demangle/Demangle.cpp: Full diff: https://github.com/llvm/llvm-project/pull/166163.diff 5 Files Affected:
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
index 91ffbb169f947..4952d84a80369 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h
@@ -23,7 +23,11 @@
#include "clang/Analysis/Analyses/LifetimeSafety/Facts.h"
#include "clang/Analysis/Analyses/LifetimeSafety/LiveOrigins.h"
#include "clang/Analysis/Analyses/LifetimeSafety/LoanPropagation.h"
+#include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
#include "clang/Analysis/AnalysisDeclContext.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/raw_ostream.h"
+#include <string>
namespace clang::lifetimes {
@@ -60,6 +64,9 @@ struct LifetimeFactory {
/// Running the lifetime safety analysis and querying its results. It
/// encapsulates the various dataflow analyses.
class LifetimeSafetyAnalysis {
+private:
+ static llvm::StringMap<int> MissingOriginCount;
+
public:
LifetimeSafetyAnalysis(AnalysisDeclContext &AC,
LifetimeSafetyReporter *Reporter);
@@ -73,6 +80,20 @@ class LifetimeSafetyAnalysis {
LiveOriginsAnalysis &getLiveOrigins() const { return *LiveOrigins; }
FactManager &getFactManager() { return FactMgr; }
+ static void PrintStats(llvm::raw_ostream &OS) {
+ llvm::errs() << "\n*** LifetimeSafety Missing Origin Stats "
+ "(expression_type : count) :\n";
+ for (const auto &[expr, count] : LifetimeSafetyAnalysis::count) {
+ OS << expr << " : " << count << '\n';
+ }
+ }
+
+ static void UpdateMissingOriginCount(const OriginManager &OM) {
+ for (const auto &[expr, missing_origin_count] : OM.getMissingOrigins()) {
+ LifetimeSafetyAnalysis::count[std::string(expr)] += missing_origin_count;
+ }
+ }
+
private:
AnalysisDeclContext &AC;
LifetimeSafetyReporter *Reporter;
diff --git a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h
index ba138b078b379..231cc60b7e097 100644
--- a/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h
+++ b/clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h
@@ -16,7 +16,10 @@
#include "clang/AST/Decl.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/TypeBase.h"
#include "clang/Analysis/Analyses/LifetimeSafety/Utils.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/raw_ostream.h"
namespace clang::lifetimes::internal {
@@ -76,6 +79,8 @@ class OriginManager {
void dump(OriginID OID, llvm::raw_ostream &OS) const;
+ const llvm::StringMap<int> getMissingOrigins() const;
+
private:
OriginID getNextOriginID() { return NextOriginID++; }
@@ -85,6 +90,7 @@ class OriginManager {
llvm::SmallVector<Origin> AllOrigins;
llvm::DenseMap<const clang::ValueDecl *, OriginID> DeclToOriginID;
llvm::DenseMap<const clang::Expr *, OriginID> ExprToOriginID;
+ llvm::StringMap<int> ExprTypeToMissingOriginCount;
};
} // namespace clang::lifetimes::internal
diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
index 00c7ed90503e7..a76fdd2535d97 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
@@ -23,14 +23,18 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "llvm/ADT/FoldingSet.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/raw_ostream.h"
#include <memory>
namespace clang::lifetimes {
namespace internal {
+llvm::StringMap<int> LifetimeSafetyAnalysis::MissingOriginCount;
+
LifetimeSafetyAnalysis::LifetimeSafetyAnalysis(AnalysisDeclContext &AC,
LifetimeSafetyReporter *Reporter)
: AC(AC), Reporter(Reporter) {}
@@ -66,6 +70,7 @@ void LifetimeSafetyAnalysis::run() {
LiveOrigins->dump(llvm::dbgs(), FactMgr.getTestPoints()));
runLifetimeChecker(*LoanPropagation, *LiveOrigins, FactMgr, AC, Reporter);
+ UpdateMissingOriginCount(FactMgr.getOriginMgr());
}
} // namespace internal
diff --git a/clang/lib/Analysis/LifetimeSafety/Origins.cpp b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
index ea51a75324e06..abe067a829cb7 100644
--- a/clang/lib/Analysis/LifetimeSafety/Origins.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/Origins.cpp
@@ -7,6 +7,8 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
+#include "clang/AST/TypeBase.h"
+#include "llvm/ADT/StringMap.h"
namespace clang::lifetimes::internal {
@@ -22,6 +24,10 @@ void OriginManager::dump(OriginID OID, llvm::raw_ostream &OS) const {
OS << ")";
}
+const llvm::StringMap<int> OriginManager::getMissingOrigins() const {
+ return ExprTypeToMissingOriginCount;
+}
+
Origin &OriginManager::addOrigin(OriginID ID, const clang::ValueDecl &D) {
AllOrigins.emplace_back(ID, &D);
return AllOrigins.back();
@@ -37,6 +43,16 @@ OriginID OriginManager::get(const Expr &E) {
auto It = ExprToOriginID.find(&E);
if (It != ExprToOriginID.end())
return It->second;
+
+ // if the expression has no specific origin, increment the missing origin
+ // counter.
+ const QualType ExprType = E.getType();
+ auto CountIt = ExprTypeToMissingOriginCount.find(ExprType.getAsString());
+ if (CountIt == ExprTypeToMissingOriginCount.end()) {
+ ExprTypeToMissingOriginCount[ExprType.getAsString()] = 1;
+ } else {
+ CountIt->second++;
+ }
// If the expression itself has no specific origin, and it's a reference
// to a declaration, its origin is that of the declaration it refers to.
// For pointer types, where we don't pre-emptively create an origin for the
diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 140b709dbb651..009994e189220 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -30,6 +30,7 @@
#include "clang/Analysis/Analyses/CalledOnceCheck.h"
#include "clang/Analysis/Analyses/Consumed.h"
#include "clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h"
+#include "clang/Analysis/Analyses/LifetimeSafety/Origins.h"
#include "clang/Analysis/Analyses/ReachableCode.h"
#include "clang/Analysis/Analyses/ThreadSafety.h"
#include "clang/Analysis/Analyses/UninitializedValues.h"
@@ -53,6 +54,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Debug.h"
+#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <deque>
#include <iterator>
@@ -3132,8 +3134,8 @@ void clang::sema::AnalysisBasedWarnings::IssueWarnings(
}
void clang::sema::AnalysisBasedWarnings::PrintStats() const {
+ clang::lifetimes::internal::LifetimeSafetyAnalysis::PrintStats(llvm::errs());
llvm::errs() << "\n*** Analysis Based Warnings Stats:\n";
-
unsigned NumCFGsBuilt = NumFunctionsAnalyzed - NumFunctionsWithBadCFGs;
unsigned AvgCFGBlocksPerFunction =
!NumCFGsBuilt ? 0 : NumCFGBlocks/NumCFGsBuilt;
|
…166226) Due to me not double-checking my PR, an overly eager AI auto-completion made it into my previous PR :/
…64281) This commit adds move constructor, move assignment and `swap` to `exception_ptr`. Adding those operators allows us to avoid unnecessary calls to `__cxa_{inc,dec}rement_refcount`. Performance results (from libc++'s CI): ``` Benchmark Baseline Candidate Difference % Difference ------------------------------------ ---------- ----------- ------------ -------------- bm_exception_ptr_copy_assign_nonnull 9.77 9.94 0.18 1.79% bm_exception_ptr_copy_assign_null 10.29 10.65 0.35 3.42% bm_exception_ptr_copy_ctor_nonnull 7.02 7.01 -0.01 -0.13% bm_exception_ptr_copy_ctor_null 10.54 10.60 0.06 0.56% bm_exception_ptr_move_assign_nonnull 16.92 13.76 -3.16 -18.70% bm_exception_ptr_move_assign_null 10.61 10.76 0.14 1.36% bm_exception_ptr_move_ctor_nonnull 13.31 10.25 -3.06 -23.02% bm_exception_ptr_move_ctor_null 10.28 7.30 -2.98 -28.95% bm_exception_ptr_swap_nonnull 19.22 0.63 -18.59 -96.74% bm_exception_ptr_swap_null 20.02 7.79 -12.23 -61.07% ``` As expected, the `bm_exception_ptr_copy_*` benchmarks are not influenced by this change. `bm_exception_ptr_move_*` benefits between 18% and 30%. The `bm_exception_ptr_swap_*` tests show the biggest improvements since multiple calls to the copy constructor are replaced by a simple pointer swap. While `bm_exception_ptr_move_assign_null` did not show a regression in the CI measurements, local measurements showed a regression from 3.98 to 4.71, i.e. by 18%. This is due to the additional `__tmp` inside `operator=`. The destructor of `__other` is a no-op after the move because `__other.__ptr` will be a nullptr. However, the compiler does not realize this, since the destructor is not inlined and is lacking a fast-path. As such, the swap-based implementation leads to an additional destructor call. `bm_exception_ptr_move_assign_nonnull` still benefits because the swap-based move constructor avoids unnecessary __cxa_{in,de}crement_refcount calls. As soon as we inline the destructor, this regression should disappear again. Works towards llvm#44892
…member function from a CallExpr. (llvm#166101) There's a bug illustrated by this example: ``` template <typename T> struct Holder { T value; T& operator*() { return value; } }; struct X { using Dispatch = float (X::*)() [[clang::nonblocking]]; void fails(Holder<Dispatch>& holder) [[clang::nonblocking]] { (this->*(*holder))(); <<< the expression is incorrectly determined not to be nonblocking } void succeeds(Holder<Dispatch>& holder) [[clang::nonblocking]] { auto func = *holder; (this->*func)(); } }; ``` In both cases we have a `CXXMemberCallExpr`. In `succeeds`, the expression refers to a `Decl` (`func`) and gets a useful PTMF type. In `fails`, the expression does not refer to a `Decl` and its type is special, printed as `bound member function`. `Expr` provides a method for extracting the true type so we can use that in this situation. --------- Co-authored-by: Doug Wyatt <dwyatt@apple.com> Co-authored-by: Sirraide <aeternalmail@gmail.com>
…name. NFC (llvm#165912) Note, X86 forces a frame pointer for stackmaps/patchpoint. So they use RBP where we use SP.
…#165293) The default behavior is to _not_ copy such swiftmodules into the dSYM, as perviously implemented in 96f95c9. This patch adds the option to override the behavior, so that such swiftmodules can be copied into the dSYM. This is useful when the dSYM will be used on a machine which has a different Xcode/SDK than where the swiftmodules were built. Without this, when LLDB is asked to "p/po" a Swift variable, the underlying Swift compiler code would rebuild the dependent `.swiftmodule` files of the Swift stdlibs, which takes ~1 minute in some cases. See PR for tests.
All builtin Clang headers need to be covered by the modulemap. This fixes llvm#166173
…ainerGlobals.cpp (llvm#166231) This PR fixes the appearance of the following warning message when building LLVM with clang (21.1.2) ``` [48/100] Building CXX object lib/Target/DirectX/CMakeFiles/LLVMDirectXCodeGen.dir/DXContainerGlobals.cpp.o In file included from /nix/store/ffrg0560kj0066s4k9pznjand907nlnz-gcc-14.3.0/include/c++/14.3.0/cassert:44, from /workspace/llvm-project/llvm/include/llvm/Support/Endian.h:19, from /workspace/llvm-project/llvm/include/llvm/Support/MD5.h:33, from /workspace/llvm-project/llvm/lib/Target/DirectX/DXContainerGlobals.cpp:28: /workspace/llvm-project/llvm/lib/Target/DirectX/DXContainerGlobals.cpp: In lambda function: /workspace/llvm-project/llvm/lib/Target/DirectX/DXContainerGlobals.cpp:198:78: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses] 198 | (uint64_t)Binding.LowerBound + Binding.Size - 1 <= UINT32_MAX && | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~ 199 | "Resource range is too large"); | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` I marked this PR as an NFC because it only modifies an assertion condition to remove a compiler warning.
This commit aggregates the following changes: 1. Fix the format (i.e., indentation) when printing stop hooks via `target stop-hook list`. 2. Add `IndentScope Stream::MakeIndentScope()` to make managing (and restoring!) of the indentation level on `Stream` instances more ergonomic and less error prone. 3. Simplify printing of stop hooks using the new `IndentScope`.
Previously, sign-extending a 1-bit boolean operand in `#DBG_VALUE` would convert `true` to -1 (i.e., 0xffffffffffffffff). However, DWARF treats booleans as unsigned values, so this resulted in the attribute `DW_AT_const_value(0xffffffffffffffff)` being emitted. As a result, the debugger would display the value as `255` instead of `true`. This change modifies the behavior to use zero-extension for 1-bit values instead, ensuring that `true` is represented as 1. Consequently, the DWARF attribute emitted is now `DW_AT_const_value(1)`, which allows the debugger to correctly display the boolean as `true`.
Support launching a supported DAP client using the lldb-dap binary. Currently, only the official LLDB-DAP Visual Studio Code extension is supported. It uses the VS Code launch URL format. Here's an example: ``` lldb-dap --client vscode -- /path/to/exe foo bar ``` This will open the following URL with `code --open-url`: ``` vscode://llvm-vs-code-extensions.lldb-dap/start?program=/path/to/exe&args=foo&arg=bar ``` Fixes llvm#125777
Makes it consistent with feedback given in the equivalent GISel code. llvm#165876
…implicit call to a destructor. (llvm#166110) This example is reduced from a discovery: resetting a shared pointer from a nonblocking function is not diagnosed. ``` void nb23() { struct X { int *ptr = nullptr; X() {} ~X() { delete ptr; } }; auto inner = []() [[clang::nonblocking]] { X(); }; } ``` `shared_ptr<T>::reset()` creates a temporary `shared_ptr` and swaps it with its current state. The temporary `shared_ptr` constructor is nonblocking but its destructor potentially deallocates memory and is unsafe. Analysis was ignoring the implicit call in the AST to destroy the temporary. --------- Co-authored-by: Doug Wyatt <dwyatt@apple.com>
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp,h -- clang/include/clang/Analysis/Analyses/LifetimeSafety/LifetimeSafety.h clang/include/clang/Analysis/Analyses/LifetimeSafety/Origins.h clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp clang/lib/Analysis/LifetimeSafety/Origins.cpp clang/lib/Sema/AnalysisBasedWarnings.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
index 828c08d1c..8c2825de8 100644
--- a/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/LifetimeSafety.cpp
@@ -40,18 +40,19 @@ LifetimeSafetyAnalysis::LifetimeSafetyAnalysis(AnalysisDeclContext &AC,
: AC(AC), Reporter(Reporter) {}
void LifetimeSafetyAnalysis::PrintStats(llvm::raw_ostream &OS) {
- llvm::errs() << "\n*** LifetimeSafety Missing Origin Stats "
- "(expression_type : count) :\n";
- for (const auto &[expr, count] : LifetimeSafetyAnalysis::MissingOriginCount) {
- OS << expr << " : " << count << '\n';
- }
+ llvm::errs() << "\n*** LifetimeSafety Missing Origin Stats "
+ "(expression_type : count) :\n";
+ for (const auto &[expr, count] : LifetimeSafetyAnalysis::MissingOriginCount) {
+ OS << expr << " : " << count << '\n';
}
+}
void LifetimeSafetyAnalysis::UpdateMissingOriginCount(const OriginManager &OM) {
- for (const auto &[expr, missing_origin_count] : OM.getMissingOrigins()) {
- LifetimeSafetyAnalysis::MissingOriginCount[std::string(expr)] += missing_origin_count;
- }
+ for (const auto &[expr, missing_origin_count] : OM.getMissingOrigins()) {
+ LifetimeSafetyAnalysis::MissingOriginCount[std::string(expr)] +=
+ missing_origin_count;
}
+}
void LifetimeSafetyAnalysis::run() {
llvm::TimeTraceScope TimeProfile("LifetimeSafetyAnalysis");
|
…s` (llvm#166001) This adds a virtual method that allows `InstrumentationRuntime` sub classes to match against all modules rather than just a library that matches a particular regex. When the implementation returns true `GetPatternForRuntimeLibrary()` is ignored and all modules are iterated over. The default implementation returns false which was the previous behavior which uses `GetPatternForRuntimeLibrary()` to only match a particular runtime library. The intended use case here is for implementing an `InstrumentationRuntime` where the runtime library of interest can have multiple implementations and whose name is not known ahead of time. The concrete use case here is for a `InstrumentationRuntime` plugin for implementations of the `-fbounds-safety` soft-trap runtime which can have multiple different implementations and so the module containing the runtime functions isn't known ahead of time. This plug-in will be upstreamed as part of the process of upstreaming `-fbounds-safety`. An alternative to this would be for the `GetPatternForRuntimeLibrary()` function to return a regex that matches everything. While that technically works this new API more clearly indicates in the intent. We probably also save a little perf by not executing the regex match for every loaded module but I have not measured this. rdar://163230807
…device/host (llvm#165503) Current implementation for -Xarch_device/host can work in any offloading model besides CUDA/HIP, so remove the specific offloading model in help text to align with implementation. --------- Signed-off-by: jinge90 <ge.jin@intel.com> Co-authored-by: Alexey Bader <alexey.bader@intel.com> Co-authored-by: Joseph Huber <huberjn@outlook.com>
|
This is deprecated in favor of #166568 |
Deprecated in favor of #166568
This PR adds the implementation for PrintStats function in LifetimeSafetyAnalysis class.
Purpose:
This utility function is added to track the expression types with missing origin. While retrieving the origins from origin manager, some expressions show missing origins. Currently these are created on the fly using getOrCreate function. For analysing the coverage of the check, it will be necessary to see what kind of expressions have a missing origin.
Approach:
A static map from strings to counts has been added to the LifetimeSafetyAnalysis class. The OriginManager also internally tracks the count of missing origins using ExprTypeToMissingOriginCount map. After analysing each file, the missing origin stats are accumulated in the LifetimeSafetyAnalysis class, in the UpdateMissingOriginCount class.
Example output:
For the file llvm-project/llvm/lib/Demangle/Demangle.cpp: