-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[analyzer] Teach -analyze-function about USRs, extend documentation #161666
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
This flag is really convinient in most cases. It's easy to figure out what value to pass for most cases. However, it can sometimes match too many times, like for template functions that has non-decuded (aka. explicitly specified) template parameters - because they don't appear in the parameter list, thus they are not accounted for in the current logic. It would be nice to improve `getFunctionName` but I'd say to just settle on using USRs. So this PR enables passing USRs to the flag, while keeping previous behavior.
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-static-analyzer-1 Author: Balazs Benics (steakhal) ChangesThis flag is really convinient in most cases. It would be nice to improve Full diff: https://github.com/llvm/llvm-project/pull/161666.diff 5 Files Affected:
diff --git a/clang/docs/analyzer/developer-docs/DebugChecks.rst b/clang/docs/analyzer/developer-docs/DebugChecks.rst
index 767ef6565d335..b3b908941317e 100644
--- a/clang/docs/analyzer/developer-docs/DebugChecks.rst
+++ b/clang/docs/analyzer/developer-docs/DebugChecks.rst
@@ -9,6 +9,22 @@ The analyzer contains a number of checkers which can aid in debugging. Enable
them by using the "-analyzer-checker=" flag, followed by the name of the
checker.
+These checkers are especially useful when analyzing a specific function, using
+the `-analyze-function` flag. The flag accepts the function name for C code,
+like `-analyze-function=myfunction`.
+For C++ code, due to overloading, the function name must include the
+parameter list, like `-analyze-function="myfunction(int, _Bool)"`.
+
+Note that `bool` must be spelled as `_Bool` in the parameter list.
+Refer to the output of `-analyzer-display-progress` to find the fully qualified
+function name.
+
+There are cases when this name can still collide. For example with template
+function instances with non-deducible (aka. explicit) template parameters.
+In such cases, prefer passing a USR instead of a function name can resolve this
+ambiguity, like this: `-analyze-function="c:@S@Window@F@overloaded#I#"`.
+
+Use the `clang-extdef-mapping` tool to find the USR for different functions.
General Analysis Dumpers
========================
diff --git a/clang/include/clang/CrossTU/CrossTranslationUnit.h b/clang/include/clang/CrossTU/CrossTranslationUnit.h
index e6b608a10e61b..9e0721edfc323 100644
--- a/clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ b/clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -180,8 +180,8 @@ class CrossTranslationUnitContext {
llvm::Expected<const VarDecl *> importDefinition(const VarDecl *VD,
ASTUnit *Unit);
- /// Get a name to identify a named decl.
- static std::optional<std::string> getLookupName(const NamedDecl *ND);
+ /// Get a name to identify a decl.
+ static std::optional<std::string> getLookupName(const Decl *D);
/// Emit diagnostics for the user for potential configuration errors.
void emitCrossTUDiagnostics(const IndexError &IE);
diff --git a/clang/lib/CrossTU/CrossTranslationUnit.cpp b/clang/lib/CrossTU/CrossTranslationUnit.cpp
index 847913d4b03fd..0287845a741ed 100644
--- a/clang/lib/CrossTU/CrossTranslationUnit.cpp
+++ b/clang/lib/CrossTU/CrossTranslationUnit.cpp
@@ -252,9 +252,9 @@ CrossTranslationUnitContext::CrossTranslationUnitContext(CompilerInstance &CI)
CrossTranslationUnitContext::~CrossTranslationUnitContext() {}
std::optional<std::string>
-CrossTranslationUnitContext::getLookupName(const NamedDecl *ND) {
+CrossTranslationUnitContext::getLookupName(const Decl *D) {
SmallString<128> DeclUSR;
- bool Ret = index::generateUSRForDecl(ND, DeclUSR);
+ bool Ret = index::generateUSRForDecl(D, DeclUSR);
if (Ret)
return {};
return std::string(DeclUSR);
diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 53466e7a75b0f..e9ba374851726 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -659,8 +659,11 @@ void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) {
AnalysisConsumer::AnalysisMode
AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
if (!Opts.AnalyzeSpecificFunction.empty() &&
- AnalysisDeclContext::getFunctionName(D) != Opts.AnalyzeSpecificFunction)
+ AnalysisDeclContext::getFunctionName(D) != Opts.AnalyzeSpecificFunction &&
+ cross_tu::CrossTranslationUnitContext::getLookupName(D).value_or("") !=
+ Opts.AnalyzeSpecificFunction) {
return AM_None;
+ }
// Unless -analyze-all is specified, treat decls differently depending on
// where they came from:
diff --git a/clang/test/Analysis/analyzeOneFunction.cpp b/clang/test/Analysis/analyzeOneFunction.cpp
new file mode 100644
index 0000000000000..3a362dfd9a08c
--- /dev/null
+++ b/clang/test/Analysis/analyzeOneFunction.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s \
+// RUN: -analyze-function="Window::overloaded(int)"
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s \
+// RUN: -analyze-function="c:@S@Window@F@overloaded#I#"
+
+// RUN: %clang_extdef_map %s | FileCheck %s
+// CHECK: 27:c:@S@Window@F@overloaded#I#
+// CHECK-NEXT: 27:c:@S@Window@F@overloaded#C#
+// CHECK-NEXT: 27:c:@S@Window@F@overloaded#d#
+
+void clang_analyzer_warnIfReached();
+
+struct Window {
+ void overloaded(double) { clang_analyzer_warnIfReached(); } // not analyzed, thus not reachable
+ void overloaded(char) { clang_analyzer_warnIfReached(); } // not analyzed, thus not reachable
+ void overloaded(int) { clang_analyzer_warnIfReached(); } // expected-warning {{REACHABLE}}
+};
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. also works well with #161663
This flag is really convinient in most cases.
It's easy to figure out what value to pass for most cases. However, it can sometimes match too many times, like for template functions that has non-decuded (aka. explicitly specified) template parameters - because they don't appear in the parameter list, thus they are not accounted for in the current logic.
It would be nice to improve
getFunctionName
but I'd say to just settle on using USRs. So this PR enables passing USRs to the flag, while keeping previous behavior.