[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. In https://reviews.llvm.org/D48027#1209844, @NoQ wrote: > So i believe that one of the important remaining problems with > `CallDescription` is to teach it to discriminate between global functions and > methods. We can do it in a number of ways: > > 1. Make a special sub-class for `CallDescription` depending on the sort of > the function (too verbose), > 2. Tag all items in the list as "record" vs. "namespace" (too many tags), > 3. Dunno, tons of other boring and verbose approaches, > 4. Change our `PreCall`/`PostCall` callbacks to callback templates that let > allow the user subscribe to specific sub-classes of `CallEvent` similarly to > how he can subscribe to different kinds of statements in > `PreStmt`/`PostStmt`, and then the user doesn't need to write > any code to check it dynamically. Personally, I prefer `4`. It is more checker developer friendly! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
NoQ added a comment.
In https://reviews.llvm.org/D48027#1207721, @rnkovacs wrote:
> I guess it is highly unlikely for someone to write namespaces like that, and
> if they do, I think they deserve to have them treated as a `std::string`.
Yup. On the other hand, if we specify our method as `{"basic_string", "c_str"}`
and they make a `namespace basic_string { const char *c_str(); }`, we are
pretty likely to //crash// when we try to obtain this-value for the call that
isn't a method. This is still not a big deal because it's still highly
unlikely, but we've seen crash reports of this sort, and the easier our spec
is, the more likely it becomes that somebody has a different thing with the
same name. For example, if we want to model iterators and we specify
`{"begin"}` without specifying the base class (so that all sorts of containers
were covered), we still want to specify that we're looking for a method call
and not for a global function call.
So i believe that one of the important remaining problems with
`CallDescription` is to teach it to discriminate between global functions and
methods. We can do it in a number of ways:
1. Make a special sub-class for CallDescription depending on the sort of the
function (too verbose),
2. Tag all items in the list as "record" vs. "namespace" (too many tags),
3. Dunno, tons of other boring and verbose approaches,
4. Change our PreCall/PostCall callbacks to callback templates that let allow
the user subscribe to specific sub-classes of `CallEvent` similarly to how he
can subscribe to different kinds of statements in
PreStmt/PostStmt, and then the user doesn't need to write any
code to check it dynamically.
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC340407: [analyzer] Improve `CallDescription` to handle c++
method. (authored by henrywong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D48027?vs=161217=161938#toc
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -80,24 +80,41 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
- StringRef FuncName;
+ // The list of the qualified names used to identify the specified CallEvent,
+ // e.g. "{a, b}" represent the qualified names, like "a::b".
+ std::vector QualifiedName;
unsigned RequiredArgs;
public:
const static unsigned NoArgRequirement = std::numeric_limits::max();
/// Constructs a CallDescription object.
///
+ /// @param QualifiedName The list of the qualified names of the function that
+ /// will be matched. It does not require the user to provide the full list of
+ /// the qualified name. The more details provided, the more accurate the
+ /// matching.
+ ///
+ /// @param RequiredArgs The number of arguments that is expected to match a
+ /// call. Omit this parameter to match every occurrence of call with a given
+ /// name regardless the number of arguments.
+ CallDescription(std::vector QualifiedName,
+ unsigned RequiredArgs = NoArgRequirement)
+ : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs) {}
+
+ /// Constructs a CallDescription object.
+ ///
/// @param FuncName The name of the function that will be matched.
///
/// @param RequiredArgs The number of arguments that is expected to match a
/// call. Omit this parameter to match every occurrence of call with a given
/// name regardless the number of arguments.
CallDescription(StringRef FuncName, unsigned RequiredArgs = NoArgRequirement)
- : FuncName(FuncName), RequiredArgs(RequiredArgs) {}
+ : CallDescription(std::vector({FuncName}), NoArgRequirement) {
+ }
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const { return QualifiedName.back(); }
};
template
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -359,11 +359,38 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const Decl *D = getDecl();
+ // If CallDescription provides prefix names, use them to improve matching
+ // accuracy.
+ if (CD.QualifiedName.size() > 1 && D) {
+const DeclContext *Ctx = D->getDeclContext();
+std::vector QualifiedName = CD.QualifiedName;
+QualifiedName.pop_back();
+for (; Ctx && isa(Ctx); Ctx = Ctx->getParent()) {
+ if (const auto *ND = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && ND->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+
+ if (const auto *RD = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && RD->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+}
+
+if (!QualifiedName.empty())
+ return false;
+ }
+
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -86,14 +86,20 @@
};
InnerPointerChecker()
- : AppendFn("append"), AssignFn("assign"), ClearFn("clear"),
-CStrFn("c_str"), DataFn("data"), EraseFn("erase"), InsertFn("insert"),
-PopBackFn("pop_back"), PushBackFn("push_back"), ReplaceFn("replace"),
-ReserveFn("reserve"), ResizeFn("resize"),
-ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
-
- /// Check if the object of this member function call is a `basic_string`.
- bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
+ : AppendFn({"std", "basic_string", "append"}),
+AssignFn({"std", "basic_string", "assign"}),
+
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. In https://reviews.llvm.org/D48027#1207645, @xazax.hun wrote: > Sorry for the delay, I think this is OK to commit. > As a possible improvement, I can imagine making it slightly stricter, e.g. > you could only skip QualifiedNames for inline namespaces and the beginning. > Maybe add support for staring with `""` or `::` to even disable skipping > namespaces at the beginning? > But these are just nice to have features, I am perfectly ok with not having > them or doing it in a followup patch. Thanks, Gábor! I will land it first and do the improvement according to the mismatch case in the followup patch! https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
rnkovacs accepted this revision.
rnkovacs added a comment.
In https://reviews.llvm.org/D48027#1203944, @MTC wrote:
> However this approach has limit. Given the code below, we cannot distinguish
> whether the `basic_string` is user-defined struct or namespace. That's means
> when the user provide {"std", "basic_string", "append"}, we can only know the
> qualified name of the call sequentially contains `std`, `basic_string`,
> `append`. We don't know if these names come from `RecordDecl` or
> `NamespaceDecl`.
>
> namespace std {
> namespace basic_string {
> struct A {
> void append() {}
> };
> }
> }
>
> void foo() {
> std::basic_string::A a;
> a.append(); // Match
> }
>
>
> @rnkovacs What do you think? Can this approach meet `InnerPointerChecker`'s
> needs?
I guess it is highly unlikely for someone to write namespaces like that, and if
they do, I think they deserve to have them treated as a `std::string`.
Thanks, Henry, this is great!
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. Oh, and thanks for working on this, this improvement was long overdue, but everybody was busy with something else :) https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun accepted this revision. xazax.hun added a comment. Sorry for the delay, I think this is OK to commit. As a possible improvement, I can imagine making it slightly stricter, e.g. you could only skip QualifiedNames for inline namespaces and the beginning. Maybe add support for staring with `""` or `::` to even disable skipping namespaces at the beginning? But these are just nice to have features, I am perfectly ok with not having them or doing it in a followup patch. https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. @xazax.hun What do you think? https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment.
In https://reviews.llvm.org/D48027#1201248, @xazax.hun wrote:
> Generally looks good, I only wonder if this works well with inline
> namespaces. Could you test? Probably it will.
Thank you for reminding me! Yea, this can handle inline namespaces.
However this approach has limit. Given the code below, we cannot distinguish
whether the `basic_string` is user-defined struct or namespace. That's means
when the user provide {"std", "basic_string", "append"}, we can only know the
qualified name of the call sequentially contains `std`, `basic_string`,
`append`. We don't know if these names come from `RecordDecl` or
`NamespaceDecl`.
namespace std {
namespace basic_string {
struct A {
void append() {}
};
}
}
void foo() {
std::basic_string::A a;
a.append(); // Match
}
@rnkovacs What do you think? Can this approach meet `InnerPointerChecker`'s
needs?
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC updated this revision to Diff 161217.
MTC added a comment.
- rebase
- Since we have enhanced the ability of `CallDescription`, remove the helper
method `isCalledOnStringObject()`.
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -359,11 +359,38 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const Decl *D = getDecl();
+ // If CallDescription provides prefix names, use them to improve matching
+ // accuracy.
+ if (CD.QualifiedName.size() > 1 && D) {
+const DeclContext *Ctx = D->getDeclContext();
+std::vector QualifiedName = CD.QualifiedName;
+QualifiedName.pop_back();
+for (; Ctx && isa(Ctx); Ctx = Ctx->getParent()) {
+ if (const auto *ND = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && ND->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+
+ if (const auto *RD = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && RD->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+}
+
+if (!QualifiedName.empty())
+ return false;
+ }
+
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -86,14 +86,20 @@
};
InnerPointerChecker()
- : AppendFn("append"), AssignFn("assign"), ClearFn("clear"),
-CStrFn("c_str"), DataFn("data"), EraseFn("erase"), InsertFn("insert"),
-PopBackFn("pop_back"), PushBackFn("push_back"), ReplaceFn("replace"),
-ReserveFn("reserve"), ResizeFn("resize"),
-ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
-
- /// Check if the object of this member function call is a `basic_string`.
- bool isCalledOnStringObject(const CXXInstanceCall *ICall) const;
+ : AppendFn({"std", "basic_string", "append"}),
+AssignFn({"std", "basic_string", "assign"}),
+ClearFn({"std", "basic_string", "clear"}),
+CStrFn({"std", "basic_string", "c_str"}),
+DataFn({"std", "basic_string", "data"}),
+EraseFn({"std", "basic_string", "erase"}),
+InsertFn({"std", "basic_string", "insert"}),
+PopBackFn({"std", "basic_string", "pop_back"}),
+PushBackFn({"std", "basic_string", "push_back"}),
+ReplaceFn({"std", "basic_string", "replace"}),
+ReserveFn({"std", "basic_string", "reserve"}),
+ResizeFn({"std", "basic_string", "resize"}),
+ShrinkToFitFn({"std", "basic_string", "shrink_to_fit"}),
+SwapFn({"std", "basic_string", "swap"}) {}
/// Check whether the called member function potentially invalidates
/// pointers referring to the container object's inner buffer.
@@ -122,21 +128,6 @@
} // end anonymous namespace
-bool InnerPointerChecker::isCalledOnStringObject(
-const CXXInstanceCall *ICall) const {
- const auto *ObjRegion =
-dyn_cast_or_null(ICall->getCXXThisVal().getAsRegion());
- if (!ObjRegion)
-return false;
-
- QualType ObjTy = ObjRegion->getValueType();
- if (ObjTy.isNull())
-return false;
-
- CXXRecordDecl *Decl = ObjTy->getAsCXXRecordDecl();
- return Decl && Decl->getName() == "basic_string";
-}
-
bool InnerPointerChecker::isInvalidatingMemberFunction(
const CallEvent ) const {
if (const auto *MemOpCall = dyn_cast()) {
@@ -220,33 +211,31 @@
ProgramStateRef State = C.getState();
if (const auto *ICall = dyn_cast()) {
-if (isCalledOnStringObject(ICall)) {
- const auto *ObjRegion = dyn_cast_or_null(
- ICall->getCXXThisVal().getAsRegion());
-
- if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
-SVal RawPtr = Call.getReturnValue();
-if (SymbolRef Sym = RawPtr.getAsSymbol(/*IncludeBaseRegions=*/true)) {
- // Start tracking this raw pointer by adding it to the set of symbols
- // associated with this container object in the program state map.
-
- PtrSet::Factory = State->getStateManager().get_context();
- const PtrSet *SetPtr = State->get(ObjRegion);
- PtrSet Set = SetPtr ? *SetPtr : F.getEmptySet();
- assert(C.wasInlined ||
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. Generally looks good, I only wonder if this works well with inline namespaces. Could you test? Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land. Herald added a subscriber: Szelethus. This looks great, thanks, this is exactly how i imagined it! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. kindly ping! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC updated this revision to Diff 157499.
MTC added a comment.
@xazax.hun Thanks for your tips! After some investigation, `MatchFinder::match`
just traverse one ASTNode, that means `match(namedDecl(HasNameMatcher()))` and
`match(namedDecl(matchesName()))` both not traverse children. So there are
three ways to match the specified AST node.
1. `match(namedDecl(HasNameMatcher()))` matches the full qualified name that
contains template parameter and that's not what we want to see.
2. `match(namedDecl(matchesName()))` uses llvm regex to match the full
qualified name.
3. Unwrap the declaration and match each layer, similar to
`HasNameMatcher::matchesNodeFullFast()`.
In this update, I use the third way to match the specified AST node. This is
simpler than I thought.
In addition, add a redundant constructor that is only used to construct a
`CallDescription` with one name, this avoids the modification of the existing
checker, like `BlockInCriticalSectionChecker.cpp`. Any suggestions?
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -256,11 +256,38 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const Decl *D = getDecl();
+ // If CallDescription provides prefix names, use them to improve matching
+ // accuracy.
+ if (CD.QualifiedName.size() > 1 && D) {
+const DeclContext *Ctx = D->getDeclContext();
+std::vector QualifiedName = CD.QualifiedName;
+QualifiedName.pop_back();
+for (; Ctx && isa(Ctx); Ctx = Ctx->getParent()) {
+ if (const auto *ND = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && ND->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+
+ if (const auto *RD = dyn_cast(Ctx)) {
+if (!QualifiedName.empty() && RD->getName() == QualifiedName.back())
+ QualifiedName.pop_back();
+continue;
+ }
+}
+
+if (!QualifiedName.empty())
+ return false;
+ }
+
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
+++ lib/StaticAnalyzer/Checkers/InnerPointerChecker.cpp
@@ -85,11 +85,20 @@
};
InnerPointerChecker()
- : AppendFn("append"), AssignFn("assign"), ClearFn("clear"),
-CStrFn("c_str"), DataFn("data"), EraseFn("erase"), InsertFn("insert"),
-PopBackFn("pop_back"), PushBackFn("push_back"), ReplaceFn("replace"),
-ReserveFn("reserve"), ResizeFn("resize"),
-ShrinkToFitFn("shrink_to_fit"), SwapFn("swap") {}
+ : AppendFn({"std", "basic_string", "append"}),
+AssignFn({"std", "basic_string", "assign"}),
+ClearFn({"std", "basic_string", "clear"}),
+CStrFn({"std", "basic_string", "c_str"}),
+DataFn({"std", "basic_string", "data"}),
+EraseFn({"std", "basic_string", "erase"}),
+InsertFn({"std", "basic_string", "insert"}),
+PopBackFn({"std", "basic_string", "pop_back"}),
+PushBackFn({"std", "basic_string", "push_back"}),
+ReplaceFn({"std", "basic_string", "replace"}),
+ReserveFn({"std", "basic_string", "reserve"}),
+ResizeFn({"std", "basic_string", "resize"}),
+ShrinkToFitFn({"std", "basic_string", "shrink_to_fit"}),
+SwapFn({"std", "basic_string", "swap"}) {}
/// Check whether the function called on the container object is a
/// member function that potentially invalidates pointers referring
@@ -148,10 +157,6 @@
if (!ObjRegion)
return;
- auto *TypeDecl = ObjRegion->getValueType()->getAsCXXRecordDecl();
- if (TypeDecl->getName() != "basic_string")
-return;
-
ProgramStateRef State = C.getState();
if (Call.isCalled(CStrFn) || Call.isCalled(DataFn)) {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -79,24 +79,41 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
- StringRef FuncName;
+ // The list of the qualified names used to identify the specified CallEvent,
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. In https://reviews.llvm.org/D48027#1142324, @MTC wrote: > - There is possible match the wrong AST node, e.g. for the NamedDecl `foo()`, > if the function body has the `a::b::x`, when we match `a::b::X()`, we will > match `foo()` . Because I'm not familiar with ASTMatcher, my bad, I can't > think of a perfect way to match only the specified nodes. Hmm, instead of using the match function which will traverse the ast, we could use the `HasNameMatcher` class which can be invoked on only one node. That class is in an internal namespace though, so I think the best would be to ask the maintainers whether it is ok to use that class externally, or were whe should put the functionality we need. Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment.
Thanks for your review, NoQ!
Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
: IILockGuard(nullptr), IIUniqueLock(nullptr),
- LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
- FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
- PthreadLockFn("pthread_mutex_lock"),
- PthreadTryLockFn("pthread_mutex_trylock"),
- PthreadUnlockFn("pthread_mutex_unlock"),
- MtxLock("mtx_lock"),
- MtxTimedLock("mtx_timedlock"),
- MtxTryLock("mtx_trylock"),
- MtxUnlock("mtx_unlock"),
+ LockFn({"lock"}), UnlockFn({"unlock"}), SleepFn({"sleep"}),
GetcFn({"getc"}),
+ FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),
NoQ wrote:
> I wish the old syntax for simple C functions was preserved.
>
> This could be accidentally achieved by using `ArrayRef` instead of
> `std::vector` for your constructor's argument.
`ArrayRef` can't achieve this goal. The only way I can figure out is
changing the declaration to the following form.
`CallDescription(StringRef FuncName, unsigned RequiredArgs = NoArgRequirement,
ArrayRef QualifiedName = None) {}`
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273-280
+std::string Regex = "^::(.*)?";
+Regex += llvm::join(CD.QualifiedName, "(.*)?::(.*)?") + "(.*)?$";
+
+auto Matches = match(namedDecl(matchesName(Regex)).bind("Regex"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
+
+if (Matches.empty())
NoQ wrote:
> Uhm, i think we don't need to flatten the class name to a string and then
> regex to do this.
>
> We can just unwrap the declaration and see if the newly unwrapped layer
> matches the expected name on every step, until all expected names have been
> found.
Is the way you mentioned above is similar `NamedDecl::printQualifiedName()`?
Get the full name through the `DeclContext` chain. See
https://github.com/llvm-mirror/clang/blob/master/lib/AST/Decl.cpp#L1511.
Or ignore namespace ,like `std`, just only consider the `CXXRecordDecl`? If so,
`dyn_cast<>` might be enough.
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp:68
: IILockGuard(nullptr), IIUniqueLock(nullptr),
- LockFn("lock"), UnlockFn("unlock"), SleepFn("sleep"), GetcFn("getc"),
- FgetsFn("fgets"), ReadFn("read"), RecvFn("recv"),
- PthreadLockFn("pthread_mutex_lock"),
- PthreadTryLockFn("pthread_mutex_trylock"),
- PthreadUnlockFn("pthread_mutex_unlock"),
- MtxLock("mtx_lock"),
- MtxTimedLock("mtx_timedlock"),
- MtxTryLock("mtx_trylock"),
- MtxUnlock("mtx_unlock"),
+ LockFn({"lock"}), UnlockFn({"unlock"}), SleepFn({"sleep"}),
GetcFn({"getc"}),
+ FgetsFn({"fgets"}), ReadFn({"read"}), RecvFn({"recv"}),
I wish the old syntax for simple C functions was preserved.
This could be accidentally achieved by using `ArrayRef` instead of
`std::vector` for your constructor's argument.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273-280
+std::string Regex = "^::(.*)?";
+Regex += llvm::join(CD.QualifiedName, "(.*)?::(.*)?") + "(.*)?$";
+
+auto Matches = match(namedDecl(matchesName(Regex)).bind("Regex"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
+
+if (Matches.empty())
Uhm, i think we don't need to flatten the class name to a string and then regex
to do this.
We can just unwrap the declaration and see if the newly unwrapped layer matches
the expected name on every step, until all expected names have been found.
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. kindly ping! Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC updated this revision to Diff 152702.
MTC added a comment.
Sorry for the long long delay, I was on the Dragon Boat Festival a few days ago.
This update has two parts:
- Use the `matchesName` to match the AST node with the specified name,
`matchesName` use regex to match the specified name.
- Use `std::vector` to record the list of qualified name, user can provide
information as much as possible to improve matching accuracy. But can also
provide only the function name.
There are two remain issues:
- There is possible match the wrong AST node, e.g. for the NamedDecl `foo()`,
if the function body has the `a::b::x`, when we match `a::b::X()`, we will
match `foo()` . Because I'm not familiar with ASTMatcher, my bad, I can't think
of a perfect way to match only the specified nodes.
- The `std::vector` to record the information provide by the users may be not
the best data structure.
- I'm not the regex expert, the regex used in this patch definitely has the
chance to improve.
And I am not good at English writing, if any comments are not very clear,
please correct me. Thanks for the help!
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/BlockInCriticalSectionChecker.cpp
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp
lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
lib/StaticAnalyzer/Checkers/ValistChecker.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -28,6 +28,7 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/ProgramPoint.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/CrossTU/CrossTranslationUnit.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
@@ -65,6 +66,7 @@
using namespace clang;
using namespace ento;
+using namespace clang::ast_matchers;
QualType CallEvent::getResultType() const {
ASTContext = getState()->getStateManager().getContext();
@@ -256,11 +258,28 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ if (CD.QualifiedName.size() > 1) {
+const NamedDecl *ND = dyn_cast_or_null(getDecl());
+if (!ND)
+ return false;
+
+std::string Regex = "^::(.*)?";
+Regex += llvm::join(CD.QualifiedName, "(.*)?::(.*)?") + "(.*)?$";
+
+auto Matches = match(namedDecl(matchesName(Regex)).bind("Regex"), *ND,
+ LCtx->getAnalysisDeclContext()->getASTContext());
+
+if (Matches.empty())
+ return false;
+ }
+
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -103,24 +103,24 @@
const SmallVector
ValistChecker::VAListAccepters = {
-{{"vfprintf", 3}, 2},
-{{"vfscanf", 3}, 2},
-{{"vprintf", 2}, 1},
-{{"vscanf", 2}, 1},
-{{"vsnprintf", 4}, 3},
-{{"vsprintf", 3}, 2},
-{{"vsscanf", 3}, 2},
-{{"vfwprintf", 3}, 2},
-{{"vfwscanf", 3}, 2},
-{{"vwprintf", 2}, 1},
-{{"vwscanf", 2}, 1},
-{{"vswprintf", 4}, 3},
+{{{"vfprintf"}, 3}, 2},
+{{{"vfscanf"}, 3}, 2},
+{{{"vprintf"}, 2}, 1},
+{{{"vscanf"}, 2}, 1},
+{{{"vsnprintf"}, 4}, 3},
+{{{"vsprintf"}, 3}, 2},
+{{{"vsscanf"}, 3}, 2},
+{{{"vfwprintf"}, 3}, 2},
+{{{"vfwscanf"}, 3}, 2},
+{{{"vwprintf"}, 2}, 1},
+{{{"vwscanf"}, 2}, 1},
+{{{"vswprintf"}, 4}, 3},
// vswprintf is the wide version of vsnprintf,
// vsprintf has no wide version
-{{"vswscanf", 3}, 2}};
-const CallDescription ValistChecker::VaStart("__builtin_va_start", 2),
-ValistChecker::VaCopy("__builtin_va_copy", 2),
-ValistChecker::VaEnd("__builtin_va_end", 1);
+{{{"vswscanf"}, 3}, 2}};
+const CallDescription ValistChecker::VaStart({"__builtin_va_start"}, 2),
+ValistChecker::VaCopy({"__builtin_va_copy"}, 2),
+ValistChecker::VaEnd({"__builtin_va_end"}, 1);
} // end anonymous namespace
void ValistChecker::checkPreCall(const CallEvent ,
Index: lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+{"std::basic_string::c_str"}, {"std::basic_string::c_str"},
NoQ wrote:
> xazax.hun wrote:
> > I am not sure if this is the right solution in case of this check. We
> > should track `c_str` calls regardless of what the template parameter is, so
> > supporting any instantiation of `basic_string` is desired. This might not
> > be the case, however, for other checks.
> >
> > If we think it is more likely we do not care about the template arguments,
> > maybe a separate API could be used, where we pass the qualified name of the
> > class separately without the template arguments.
> > Alternatively, we could use matches name so the users could use regexps.
> >
> > At this point I also wonder what isCalled API gives us compared to
> > matchers? Maybe it is more convenient to use than calling a `match`. Also,
> > isCalled API has an IdentifierInfo cached which could be used for
> > relatively efficient checks.
> >
> > @NoQ what do you think?
> >
> >
> I agree that it's better to match the chain of classes and namespaces (in as
> much detail as the user cares to provide) and discard template parameters.
>
> For example, i wish that a `CallDescription` that's defined as `{"std",
> "basic_string", "c_str"}` would be able to match both `std::string::c_str()`
> and `std::__1::string::c_str()`.
Yea, checker writers may can't provide all the template arguments, and it's not
convenient to use!
I will try to give a better solution!
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl();
if (TypeDecl->getName() != "basic_string")
return;
xazax.hun wrote:
> If we check for fully qualified names, this check might be redundant.
Right, thanks!
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273
+ auto Matches =
+ match(namedDecl(hasName(CD.FuncName)).bind("match_qualified_name"), *ND,
+LCtx->getAnalysisDeclContext()->getASTContext());
xazax.hun wrote:
> Doesn't match also traverse the subtree of the AST? We only need to check if
> that particular node matches the qualified name. I wonder if we could, during
> the traversal, find another node that is a match unintentionally.
Yea, this maybe a problem, I will test whether this is going to happen and give
a fine-grained match. Thanks!
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
NoQ added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+{"std::basic_string::c_str"}, {"std::basic_string::c_str"},
xazax.hun wrote:
> I am not sure if this is the right solution in case of this check. We should
> track `c_str` calls regardless of what the template parameter is, so
> supporting any instantiation of `basic_string` is desired. This might not be
> the case, however, for other checks.
>
> If we think it is more likely we do not care about the template arguments,
> maybe a separate API could be used, where we pass the qualified name of the
> class separately without the template arguments.
> Alternatively, we could use matches name so the users could use regexps.
>
> At this point I also wonder what isCalled API gives us compared to matchers?
> Maybe it is more convenient to use than calling a `match`. Also, isCalled API
> has an IdentifierInfo cached which could be used for relatively efficient
> checks.
>
> @NoQ what do you think?
>
>
I agree that it's better to match the chain of classes and namespaces (in as
much detail as the user cares to provide) and discard template parameters.
For example, i wish that a `CallDescription` that's defined as `{"std",
"basic_string", "c_str"}` would be able to match both `std::string::c_str()`
and `std::__1::string::c_str()`.
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32
check::PostCall> {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+{"std::basic_string::c_str"}, {"std::basic_string::c_str"},
I am not sure if this is the right solution in case of this check. We should
track `c_str` calls regardless of what the template parameter is, so supporting
any instantiation of `basic_string` is desired. This might not be the case,
however, for other checks.
If we think it is more likely we do not care about the template arguments,
maybe a separate API could be used, where we pass the qualified name of the
class separately without the template arguments.
Alternatively, we could use matches name so the users could use regexps.
At this point I also wonder what isCalled API gives us compared to matchers?
Maybe it is more convenient to use than calling a `match`. Also, isCalled API
has an IdentifierInfo cached which could be used for relatively efficient
checks.
@NoQ what do you think?
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:65
auto *TypeDecl = TypedR->getValueType()->getAsCXXRecordDecl();
if (TypeDecl->getName() != "basic_string")
return;
If we check for fully qualified names, this check might be redundant.
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:273
+ auto Matches =
+ match(namedDecl(hasName(CD.FuncName)).bind("match_qualified_name"), *ND,
+LCtx->getAnalysisDeclContext()->getASTContext());
Doesn't match also traverse the subtree of the AST? We only need to check if
that particular node matches the qualified name. I wonder if we could, during
the traversal, find another node that is a match unintentionally.
Repository:
rC Clang
https://reviews.llvm.org/D48027
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC updated this revision to Diff 151166.
MTC added a comment.
- Use `hasName` matcher to match the qualified name.
- Use the full name, like `std::basic_string::c_str` instead of `c_str`
to match the `c_str` method in `DanglingInternalBufferChecker.cpp`.
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -28,6 +28,7 @@
#include "clang/Analysis/AnalysisDeclContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/ProgramPoint.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/CrossTU/CrossTranslationUnit.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
@@ -65,6 +66,7 @@
using namespace clang;
using namespace ento;
+using namespace clang::ast_matchers;
QualType CallEvent::getResultType() const {
ASTContext = getState()->getStateManager().getContext();
@@ -256,11 +258,24 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const NamedDecl *ND = dyn_cast_or_null(getDecl());
+ if (!ND)
+return false;
+
+ auto Matches =
+ match(namedDecl(hasName(CD.FuncName)).bind("match_qualified_name"), *ND,
+LCtx->getAnalysisDeclContext()->getASTContext());
+
+ if (Matches.empty())
+return false;
+
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
+++ lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp
@@ -13,26 +13,28 @@
//
//===--===//
+#include "AllocationState.h"
#include "ClangSACheckers.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
-#include "AllocationState.h"
+#include "llvm/ADT/SmallVector.h"
using namespace clang;
using namespace ento;
namespace {
class DanglingInternalBufferChecker : public Checker {
- CallDescription CStrFn;
+ const llvm::SmallVector CStrFnFamily = {
+{"std::basic_string::c_str"}, {"std::basic_string::c_str"},
+{"std::basic_string::c_str"},
+{"std::basic_string::c_str"}};
public:
- DanglingInternalBufferChecker() : CStrFn("c_str") {}
-
/// Record the connection between the symbol returned by c_str() and the
/// corresponding string object region in the ProgramState. Mark the symbol
/// released if the string object is destroyed.
@@ -65,7 +67,15 @@
ProgramStateRef State = C.getState();
- if (Call.isCalled(CStrFn)) {
+ auto isCStrFnFamilyCall = [&](const CallEvent ) -> bool {
+for (auto CStrFn : CStrFnFamily) {
+ if (Call.isCalled(CStrFn))
+return true;
+}
+return false;
+ };
+
+ if (isCStrFnFamilyCall(Call)) {
SVal RawPtr = Call.getReturnValue();
if (!RawPtr.isUnknown()) {
State = State->set(TypedR, RawPtr.getAsSymbol());
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -79,6 +79,7 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
+ // Represent the function name or method name, like "X" or "a::b::X".
StringRef FuncName;
unsigned RequiredArgs;
@@ -96,7 +97,11 @@
: FuncName(FuncName), RequiredArgs(RequiredArgs) {}
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const {
+auto QualifierNamePair = FuncName.rsplit("::");
+return QualifierNamePair.second.empty() ? QualifierNamePair.first
+: QualifierNamePair.second;
+ }
};
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. In https://reviews.llvm.org/D48027#1128301, @xazax.hun wrote: > Having C++ support would be awesome! > Thanks for working on this! > > While I do agree matching is not trivial with qualified names, this problem > is already solved with AST matchers. > > I wonder if using matchers or reusing the `HasNameMatcher` class would make > this easier. That code path is already well tested and optimized. Thank you for pointing out the direction, xazax.hun! I will looking into the matchers and try to give a better solution. Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
xazax.hun added a comment. Having C++ support would be awesome! Thanks for working on this! While I do agree matching is not trivial with qualified names, this problem is already solved with AST matchers. I wonder if using matchers or reusing the `HasNameMatcher` class would make this easier. That code path is already well tested and optimized. Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC updated this revision to Diff 150758.
MTC added a comment.
Remove useless header files for testing.
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -256,11 +256,18 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II =
()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const auto *ND = dyn_cast_or_null(getDecl());
+ if (!ND)
+return false;
+ if (!CD.matchQualifiedName(ND->getQualifiedNameAsString()))
+return false;
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -79,6 +79,8 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
+ // Represent the function name or method name, like "c_str" or
+ // "std::string::c_str".
StringRef FuncName;
unsigned RequiredArgs;
@@ -96,7 +98,25 @@
: FuncName(FuncName), RequiredArgs(RequiredArgs) {}
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const {
+auto QualifierNamePair = FuncName.rsplit("::");
+return QualifierNamePair.second.empty() ? QualifierNamePair.first
+: QualifierNamePair.second;
+ }
+
+ bool matchQualifiedName(llvm::StringRef QualifiedName) const {
+llvm::SmallVector Names;
+// Split the function name with the separator "::".
+FuncName.split(Names, "::");
+
+// FIXME: Since there is no perfect way to get the qualified name without
+// template argument, we can only use a fuzzy matching approach.
+for (auto item : Names)
+ if (QualifiedName.find(item) == StringRef::npos)
+return false;
+
+return true;
+ }
};
template
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -256,11 +256,18 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const auto *ND = dyn_cast_or_null(getDecl());
+ if (!ND)
+return false;
+ if (!CD.matchQualifiedName(ND->getQualifiedNameAsString()))
+return false;
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -79,6 +79,8 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
+ // Represent the function name or method name, like "c_str" or
+ // "std::string::c_str".
StringRef FuncName;
unsigned RequiredArgs;
@@ -96,7 +98,25 @@
: FuncName(FuncName), RequiredArgs(RequiredArgs) {}
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const {
+auto QualifierNamePair = FuncName.rsplit("::");
+return QualifierNamePair.second.empty() ? QualifierNamePair.first
+: QualifierNamePair.second;
+ }
+
+ bool matchQualifiedName(llvm::StringRef QualifiedName) const {
+llvm::SmallVector Names;
+// Split the function name with the separator "::".
+FuncName.split(Names, "::");
+
+// FIXME: Since there is no perfect way to get the qualified name without
+// template argument, we can only use a fuzzy matching approach.
+for (auto item : Names)
+ if (QualifiedName.find(item) == StringRef::npos)
+return false;
+
+return true;
+ }
};
template
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC added a comment. The implementation is not complicated, the difficulty is that there is no good way to get the qualified name without template arguments. For 'std::basic_string::c_str()', its qualified name may be `std::__1::basic_string, std::__1::allocator >::c_str`, it is almost impossible for users to provide such a name. So one possible implementation is to use `std`, `basic_string` and `c_str` to match in the `std::__1::basic_string, std::__1::allocator >::c_str` sequentially. Repository: rC Clang https://reviews.llvm.org/D48027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D48027: [analyzer] Improve `CallDescription` to handle c++ method.
MTC created this revision.
MTC added reviewers: xazax.hun, NoQ, george.karpenkov.
Herald added subscribers: mikhail.ramalho, a.sidorin, rnkovacs, szepet.
`CallDecription` can only handle function for the time being. If we want to
match c++ method, we can only use method name to match and can't improve the
matching accuracy through the qualifiers.
Repository:
rC Clang
https://reviews.llvm.org/D48027
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
lib/StaticAnalyzer/Core/CallEvent.cpp
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -60,6 +60,7 @@
#include "llvm/Support/raw_ostream.h"
#include
#include
+#include
#define DEBUG_TYPE "static-analyzer-call-event"
@@ -256,11 +257,18 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II =
()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const auto *ND = dyn_cast_or_null(getDecl());
+ if (!ND)
+return false;
+ if (!CD.matchQualifiedName(ND->getQualifiedNameAsString()))
+return false;
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
namespace clang {
@@ -79,6 +80,8 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
+ // Represent the function name or method name, like "c_str" or
+ // "std::string::c_str".
StringRef FuncName;
unsigned RequiredArgs;
@@ -96,7 +99,25 @@
: FuncName(FuncName), RequiredArgs(RequiredArgs) {}
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const {
+auto QualifierNamePair = FuncName.rsplit("::");
+return QualifierNamePair.second.empty() ? QualifierNamePair.first
+: QualifierNamePair.second;
+ }
+
+ bool matchQualifiedName(llvm::StringRef QualifiedName) const {
+llvm::SmallVector Names;
+// Split the function name with the separator "::".
+FuncName.split(Names, "::");
+
+// FIXME: Since there is no perfect way to get the qualified name without
+// template argument, we can only use a fuzzy matching approach.
+for (auto item : Names)
+ if (QualifiedName.find(item) == StringRef::npos)
+return false;
+
+return true;
+ }
};
template
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -60,6 +60,7 @@
#include "llvm/Support/raw_ostream.h"
#include
#include
+#include
#define DEBUG_TYPE "static-analyzer-call-event"
@@ -256,11 +257,18 @@
return false;
if (!CD.IsLookupDone) {
CD.IsLookupDone = true;
-CD.II = ()->getStateManager().getContext().Idents.get(CD.FuncName);
+CD.II = ()->getStateManager().getContext().Idents.get(
+CD.getFunctionName());
}
const IdentifierInfo *II = getCalleeIdentifier();
if (!II || II != CD.II)
return false;
+
+ const auto *ND = dyn_cast_or_null(getDecl());
+ if (!ND)
+return false;
+ if (!CD.matchQualifiedName(ND->getQualifiedNameAsString()))
+return false;
return (CD.RequiredArgs == CallDescription::NoArgRequirement ||
CD.RequiredArgs == getNumArgs());
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -45,6 +45,7 @@
#include
#include
#include
+#include
namespace clang {
@@ -79,6 +80,8 @@
mutable IdentifierInfo *II = nullptr;
mutable bool IsLookupDone = false;
+ // Represent the function name or method name, like "c_str" or
+ // "std::string::c_str".
StringRef FuncName;
unsigned RequiredArgs;
@@ -96,7 +99,25 @@
: FuncName(FuncName), RequiredArgs(RequiredArgs) {}
/// Get the name of the function that this object matches.
- StringRef getFunctionName() const { return FuncName; }
+ StringRef getFunctionName() const {
+auto QualifierNamePair = FuncName.rsplit("::");
+return QualifierNamePair.second.empty() ? QualifierNamePair.first
+
