Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -79,6 +79,7 @@ VforkChecker.cpp VLASizeChecker.cpp VirtualCallChecker.cpp + QtSignalSlotChecker.cpp DEPENDS ClangSACheckers Index: lib/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- lib/StaticAnalyzer/Checkers/Checkers.td +++ lib/StaticAnalyzer/Checkers/Checkers.td @@ -243,6 +243,10 @@ HelpText<"Check for memory leaks. Traces memory managed by new/delete.">, DescFile<"MallocChecker.cpp">; +def QtSignalSlotChecker : Checker<"QtSignalSlotChecker">, + HelpText<"Check for proper usage of Qt connect">, + DescFile<"QtSignalSlotChecker.cpp">; + } // end: "cplusplus" let ParentPackage = CplusplusAlpha in { Index: lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp =================================================================== --- /dev/null +++ lib/StaticAnalyzer/Checkers/QtSignalSlotChecker.cpp @@ -0,0 +1,545 @@ +//=== QtSignalSlotChecker.cpp - A Qt signal/slots usage checker ----*- C++ +//-*--// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// +// +// This file defines Qt signal/slot checker, which check that such call +// QObject::connect(obj1, SIGNAL(singnal(T1, T2)), obj2, SLOT(slot(U1, U2))); +// used in right way, signal,slots exists, they signatures matches etc. +//===----------------------------------------------------------------------===// + +#include "ClangSACheckers.h" +#include "clang/AST/ParentMap.h" +#include "clang/AST/StmtVisitor.h" +#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" +#include "clang/StaticAnalyzer/Core/Checker.h" +#include "clang/StaticAnalyzer/Core/CheckerManager.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h" +#include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h" + +using namespace clang; +using namespace ento; + +namespace clang { +namespace ento { +//make it extern for unit testing +extern std::string qtNormalizeSignature(const StringRef &); +} // end namespace ento +} // end namespace clang + +namespace { +class QtSignalSlotChecker final : public Checker { +public: + using DetailsReporterT = std::function; + + QtSignalSlotChecker() {} + void checkPostCall(const CallEvent &Call, CheckerContext &C) const; + void reportBug(CheckerContext &C, const char *Title, + const DetailsReporterT &DetailsReporter) const; + +private: + mutable std::unique_ptr BT; +}; +} // end anonymous namespace + +static bool findSuitableMethod( + const CXXRecordDecl *Class, + const std::function &MethodHandler) { + assert(!!MethodHandler); + + if (Class == nullptr) + return false; + + for (auto &&M : Class->methods()) + if (M != nullptr && MethodHandler(*M)) + return true; + + for (const CXXBaseSpecifier &BC : Class->bases()) { + QualType T = BC.getType(); + const CXXRecordDecl *CT = T.getTypePtr()->getAsCXXRecordDecl(); + if (CT != nullptr) { + if (findSuitableMethod(CT, MethodHandler)) + return true; + } + } + return false; +} + +static void printMethodNameWithPramaTypes(llvm::raw_svector_ostream &Out, + const CheckerContext &C, + const StringRef &MName, + const CXXMethodDecl &M, + bool SkipDefArgs) { + Out << MName << "("; + bool F = false; + for (auto It = M.param_begin(), End = M.param_end(); It != End; ++It) { + if (SkipDefArgs && (*It)->hasDefaultArg()) + continue; + QualType ParamType = (*It)->getType(); + if (F) + Out << ","; + F = true; + Out << ParamType.stream(C.getLangOpts()); + } + Out << ")"; +} + +static bool +isMethodWithSignatureExists(CheckerContext &C, const CXXRecordDecl *Class, + const StringRef &MethodName, + const std::string &MethodWithParamsNorm) { + return (Class == nullptr) + ? false + : findSuitableMethod( + Class, [&MethodName, &C, + &MethodWithParamsNorm](const CXXMethodDecl &M) { + const IdentifierInfo *II = M.getIdentifier(); + if (II == nullptr) + return false; + StringRef FName = II->getName(); + if (FName == MethodName) { + SmallString<128> buf; + { + llvm::raw_svector_ostream Out(buf); + printMethodNameWithPramaTypes(Out, C, FName, M, false); + const std::string NS = qtNormalizeSignature(Out.str()); + if (NS == MethodWithParamsNorm) + return true; + } + buf.clear(); + { + llvm::raw_svector_ostream Out(buf); + printMethodNameWithPramaTypes(Out, C, FName, M, true); + const std::string NS = qtNormalizeSignature(Out.str()); + if (NS == MethodWithParamsNorm) + return true; + } + } + return false; + }); +} + +static StringRef getStringLiteral(const Stmt *St) { + StringRef Res; + + if (St == nullptr) + return Res; + + if (const StringLiteral *S = dyn_cast(St)) { + Res = S->getString(); + return Res; + } + for (const Stmt *SubStmt : St->children()) { + if (const StringLiteral *S = dyn_cast(SubStmt)) { + Res = S->getString(); + break; + } else if (SubStmt != nullptr) { + Res = getStringLiteral(SubStmt); + if (!Res.empty()) + break; + } + } + return Res; +} + +void QtSignalSlotChecker::reportBug( + CheckerContext &C, const char *Title, + const DetailsReporterT &DetailsReporter) const { + assert(Title != nullptr); + assert(!!DetailsReporter); + if (ExplodedNode *N = C.generateNonFatalErrorNode(C.getState())) { + if (!BT) + BT.reset(new BuiltinBug(this, Title)); + + SmallString<256> buf; + llvm::raw_svector_ostream os(buf); + DetailsReporter(os); + auto Report = llvm::make_unique(*BT, os.str(), N); + C.emitReport(std::move(Report)); + } +} + +void QtSignalSlotChecker::checkPostCall(const CallEvent &Call, + CheckerContext &C) const { + const SimpleFunctionCall *SC = dyn_cast(&Call); + if (SC == nullptr) + return; + + const FunctionDecl *FD = SC->getDecl(); + if (FD == nullptr) + return; + + const IdentifierInfo *II = FD->getIdentifier(); + if (II == nullptr) + return; + + StringRef FName = II->getName(); + + if (!FName.endswith("connect") || + FD->getQualifiedNameAsString() != "QObject::connect") + return; + + const unsigned nargs = SC->getNumArgs(); + if (nargs != 5) + return; + + const Expr *Sender = SC->getArgExpr(0); + const Expr *Receiver = SC->getArgExpr(2); + + const Expr *Signal = SC->getArgExpr(1); + assert(Signal != nullptr); + const Expr *Func = SC->getArgExpr(3); + assert(Func != nullptr); + + auto isConstCharPtr = [](const Expr &Expr) { + QualType T = Expr.getType(); + if (!T.getTypePtr()->isPointerType()) + return false; + T = T.getTypePtr()->getPointeeType(); + return T.getTypePtr()->isCharType() && T.isConstQualified(); + }; + + if (!isConstCharPtr(*Signal) || !isConstCharPtr(*Func)) + return; + + auto ExtractSig = [](StringRef FM) { return FM.substr(0, FM.find('\0')); }; + + const StringRef SignalSign = ExtractSig(getStringLiteral(Signal)); + const StringRef FuncSign = ExtractSig(getStringLiteral(Func)); + + if (SignalSign.empty() || FuncSign.empty()) { + // some of connect argument are variables, not supported yet + return; + } + + const std::string NormSignalSig = qtNormalizeSignature(SignalSign.substr(1)); + const std::string NormFuncSig = qtNormalizeSignature(FuncSign.substr(1)); + + auto getFuncName = [](const StringRef &Str) { + StringRef Res; + size_t pos = Str.find('(', 1); + if (pos == StringRef::npos) + return Res; + Res = Str.substr(1, pos - 1); + return Res; + }; + + const StringRef SignalName = getFuncName(SignalSign); + const StringRef FuncName = getFuncName(FuncSign); + assert(!SignalName.empty()); + assert(!FuncName.empty()); + + auto SenderClass = Sender->getBestDynamicClassType(); + auto RecvClass = Receiver->getBestDynamicClassType(); + + const bool FindResSig = + isMethodWithSignatureExists(C, SenderClass, SignalName, NormSignalSig); + const bool FindResRecv = + isMethodWithSignatureExists(C, RecvClass, FuncName, NormFuncSig); + + const bool SignalSigWarn = NormSignalSig != SignalSign.substr(1); + const bool FuncSigWarn = NormFuncSig != FuncSign.substr(1); + + auto getParamPack = [](const std::string &NormSig) { + StringRef Res; + const size_t B = NormSig.find('(', 1); + assert(B != StringRef::npos); + const size_t E = NormSig.find(')', B); + assert(E != StringRef::npos); + if ((E - B) == 1) + return Res; + Res = NormSig; + Res = Res.substr(B + 1, E - B - 1); + return Res; + }; + + const StringRef SigParamPack = getParamPack(NormSignalSig); + const StringRef FuncParamPack = getParamPack(NormFuncSig); + + bool Match = false; + if (FuncParamPack.empty() || SigParamPack == FuncParamPack) { + Match = true; + } else if (SigParamPack.startswith(FuncParamPack) && + SigParamPack[FuncParamPack.size()] == ',') { + Match = true; + } + + const char *Title = "QObject::connect warnings"; + + // move more important warning to title + if (!Match) + Title = "QObject::connect signautre mismatch"; + else if (!FindResSig || !FindResRecv) + Title = "QObject::connect can not find signal or method"; + + if (SignalSigWarn || FuncSigWarn || !FindResSig || !FindResRecv || !Match) { + reportBug(C, Title, [SignalSigWarn, FuncSigWarn, FindResSig, FindResRecv, + Match, SenderClass, RecvClass, &NormSignalSig, + &SignalSign, &NormFuncSig, + &FuncSign](llvm::raw_svector_ostream &os) { + bool NeedSemi = false; + if (!FindResSig) { + NeedSemi = true; + os << "Can not find such signal '" << SignalSign.substr(1) + << "' in class " << SenderClass->getQualifiedNameAsString(); + } + if (!FindResRecv) { + os << (NeedSemi ? "; " : "") << "Can not find such signal '" + << FuncSign.substr(1) << "' in class " + << RecvClass->getQualifiedNameAsString(); + NeedSemi = true; + } + if (!Match) { + os << (NeedSemi ? "; " : "") << "signal/slot signature mismatch"; + NeedSemi = true; + } + if (SignalSigWarn) { + os << (NeedSemi ? "; " : "") << "You use for as signal signature: '" + << SignalSign.substr(1) << "', consider use normalized variant: '" + << NormSignalSig << "', this improve performance"; + NeedSemi = true; + } + if (FuncSigWarn) { + os << (NeedSemi ? "; " : "") << "You use for as signal signature: '" + << FuncSign.substr(1) << "', consider use normalized variant: '" + << NormFuncSig << "', this improve performance"; + } + }); + } +} + +static inline bool qtSigIsIdentChar(char C) { + return (C >= 'a' && C <= 'z') || (C >= 'A' && C <= 'Z') || + (C >= '0' && C <= '9') || C == '_'; +} + +static inline bool qtSigIsSpace(char C) { return C == ' ' || C == '\t'; } + +static inline bool isRestOfArrayContains(const char *s1, const char *s2, + size_t n) { + return std::strncmp(s1, s2, n) == 0; +} + +static std::string doQtNormalizeType(const char *t, const char *e, + bool adjustConst = true) { + static const char CONST[] = "const "; + int len = e - t; + /* + Convert 'char const *' into 'const char *'. Start at index 1, + not 0, because 'const char *' is already OK. + */ + SmallVector constbuf; + for (int i = 1; i < len; i++) { + if (t[i] == 'c' && isRestOfArrayContains("onst", t + i + 1, 4) && + (i + 5 >= len || !qtSigIsIdentChar(t[i + 5])) && + !qtSigIsIdentChar(t[i - 1])) { + constbuf.append(t, t + len); + if (qtSigIsSpace(t[i - 1])) + constbuf.erase(constbuf.begin() + i - 1, constbuf.begin() + i - 1 + 6); + else + constbuf.erase(constbuf.begin() + i, constbuf.begin() + i + 5); + constbuf.insert(constbuf.begin(), CONST, CONST + sizeof(CONST) - 1); + t = constbuf.data(); + e = constbuf.data() + constbuf.size(); + break; + } + /* + We musn't convert 'char * const *' into 'const char **' + and we must beware of 'Bar'. + */ + if (t[i] == '&' || t[i] == '*' || t[i] == '<') + break; + } + if (adjustConst && e > t + 6 && isRestOfArrayContains("const ", t, 6)) { + if (*(e - 1) == '&') { // treat const reference as value + t += 6; + --e; + } else if (qtSigIsIdentChar(*(e - 1)) || + *(e - 1) == '>') { // treat const value as value + t += 6; + } + } + std::string Result; + Result.reserve(len); + + // consume initial 'const ' + if (isRestOfArrayContains("const ", t, 6)) { + t += 6; + Result += "const "; + } + + // some type substitutions for 'unsigned x' + if (isRestOfArrayContains("unsigned", t, 8)) { + // make sure "unsigned" is an isolated word before making substitutions + if (!t[8] || !qtSigIsIdentChar(t[8])) { + if (isRestOfArrayContains(" int", t + 8, 4)) { + t += 8 + 4; + Result += "uint"; + } else if (isRestOfArrayContains(" long", t + 8, 5)) { + if ((strlen(t + 8 + 5) < 4 || + !isRestOfArrayContains(t + 8 + 5, " int", + 4)) // preserve '[unsigned] long int' + && (strlen(t + 8 + 5) < 5 || + !isRestOfArrayContains(t + 8 + 5, " long", + 5)) // preserve '[unsigned] long long' + ) { + t += 8 + 5; + Result += "ulong"; + } + } else if (!isRestOfArrayContains(" short", t + 8, + 6) // preserve unsigned short + && + !isRestOfArrayContains(" char", t + 8, + 5)) { // preserve unsigned char + // treat rest (unsigned) as uint + t += 8; + Result += "uint"; + } + } + } else { + // discard 'struct', 'class', and 'enum'; they are optional + // and we don't want them in the normalized signature + struct { + const char *keyword; + int len; + } optional[] = {{"struct ", 7}, {"class ", 6}, {"enum ", 5}, {0, 0}}; + int i = 0; + do { + if (isRestOfArrayContains(optional[i].keyword, t, optional[i].len)) { + t += optional[i].len; + break; + } + } while (optional[++i].keyword != 0); + } + + bool star = false; + while (t != e) { + char c = *t++; + star = star || c == '*'; + Result += c; + if (c == '<') { + // template recursion + const char *tt = t; + int templdepth = 1; + while (t != e) { + c = *t++; + if (c == '<') + ++templdepth; + if (c == '>') + --templdepth; + if (templdepth == 0 || (templdepth == 1 && c == ',')) { + Result += doQtNormalizeType(tt, t - 1, false); + Result += c; + if (templdepth == 0) { + if (*t == '>') + Result += ' '; // avoid >> + break; + } + tt = t; + } + } + } + + // cv qualifers can appear after the type as well + if (!qtSigIsIdentChar(c) && t != e && + (e - t >= 5 && isRestOfArrayContains("const", t, 5)) && + (e - t == 5 || !qtSigIsIdentChar(t[5]))) { + t += 5; + while (t != e && qtSigIsSpace(*t)) + ++t; + if (adjustConst && t != e && *t == '&') { + // treat const ref as value + ++t; + } else if (adjustConst && !star) { + // treat const as value + } else if (!star) { + // move const to the front (but not if const comes after a *) + Result.insert(0, "const "); + } else { + // keep const after a * + Result += "const"; + } + } + } + + return Result; +} + +template +static void qtTrimSignature(const StringRef &M, Container &Res) { + auto It = M.begin(); + auto E = M.end(); + char last = 0; + while (It != E && qtSigIsSpace(*It)) + ++It; + while (It != E) { + while (It != E && !qtSigIsSpace(*It)) { + Res.push_back(last = *It); + ++It; + } + while (It != E && qtSigIsSpace(*It)) + ++It; + if (It != E && ((qtSigIsIdentChar(*It) && qtSigIsIdentChar(last)) || + ((*It == ':') && (last == '<')))) { + Res.push_back(last = ' '); + } + } + Res.push_back('\0'); +} + +static char *qtNormalizeType(char *d, int &TemplateDepth, std::string &result) { + const char *t = d; + while (*d && (TemplateDepth != 0 || (*d != ',' && *d != ')'))) { + if (*d == '<') + ++TemplateDepth; + else if (*d == '>') + --TemplateDepth; + ++d; + } + if (!isRestOfArrayContains("void", t, d - t)) + result += doQtNormalizeType(t, d); + + return d; +} + +std::string ento::qtNormalizeSignature(const StringRef &Method) { + std::string Res; + if (Method.empty()) + return Res; + + SmallVector TrimmedM; + qtTrimSignature(Method, TrimmedM); + + Res.reserve(Method.size()); + + int BraceDepth = 0; + int TemplateDepth = 0; + char *d = &TrimmedM[0]; + while (*d) { + if (BraceDepth == 1) { + d = qtNormalizeType(d, TemplateDepth, Res); + if (!*d) // most likely an invalid signature. + break; + } + if (*d == '(') + ++BraceDepth; + if (*d == ')') + --BraceDepth; + Res += *d++; + } + + return Res; +} + +void ento::registerQtSignalSlotChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} Index: test/Analysis/qt_connect.cpp =================================================================== --- /dev/null +++ test/Analysis/qt_connect.cpp @@ -0,0 +1,82 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,cplusplus -analyzer-store=region -verify %s + +const char *qFlagLocation(const char *method); +#define Q_OBJECT +#define QTOSTRING_HELPER(s) #s +#define QTOSTRING(s) QTOSTRING_HELPER(s) +# define QLOCATION "\0" __FILE__ ":" QTOSTRING(__LINE__) +# define SLOT(a) qFlagLocation("1"#a QLOCATION) +# define SIGNAL(a) qFlagLocation("2"#a QLOCATION) + +# define slots +# define signals protected + +namespace Qt { + enum ConnectionType { + AutoConnection, + DirectConnection, + QueuedConnection, + AutoCompatConnection, + BlockingQueuedConnection, + UniqueConnection = 0x80 + }; +} +struct QString {}; +class QObject { +public: + QObject() {} + static bool connect(const QObject *sender, const char *signal, + const QObject *receiver, const char *member, Qt::ConnectionType = Qt::AutoConnection); +}; + +class QWidget : public QObject { +public: + QWidget() {} +}; + +class Sender : public QWidget { + Q_OBJECT +signals: + void empty(); + void looksEmpty(bool = false); + void stringRef(const QString&); + void stringMutRef(QString&); + void f2(int, double*); + void f3(int, double*,char); + void f4(int, double*,QString); +public: + Sender() {} +}; + +class Recv : public QWidget { + Q_OBJECT +public slots: + void onEmpty() {} + void onStringRef(const QString&) {} + void onStringMutRef(QString&) {} + void onf2(int, double*) {} + void onf3(int, double*,char) {} +public: + Recv() {} +}; + + +class TestCase : public QWidget { +public: + TestCase() { + connect(&send, SIGNAL(empty()), &recv, SLOT(onEmpty())); + connect(&send, SIGNAL(stringRef(const QString &)), &recv, SLOT(onStringRef(const QString &))); // expected-warning{{consider use normalized variant}} + connect(&send, SIGNAL(stringMutRef(QString &)), &recv, SLOT(onStringRef(const QString &))); // expected-warning{{consider use normalized variant}} + connect(&send, SIGNAL(stringMutRef(QString &)), &recv, SLOT(onStringMutRef(QString&))); // expected-warning{{consider use normalized variant}} + connect(&send, SIGNAL(f2(int, double*)), &recv, SLOT(onf2(int, double*))); // expected-warning{{consider use normalized variant}} + connect(&send, SIGNAL(f2(int, double*)), &recv, SLOT(bugaga(int, double*))); // expected-warning{{Can not find such signal}} + connect(&send, SIGNAL(f2(int, double*)), &recv, SLOT(onf3(int, double*,char))); // expected-warning{{signal/slot signature mismatch}} + connect(&send, SIGNAL(f3(int,double*,char)), &recv, SLOT(onf2(int,double*))); // no-warning + connect(&send, SIGNAL(bugaga()), &recv, SLOT(onEmpty()));// expected-warning{{Can not find such signal 'bugaga()'}} + connect(&send, SIGNAL(looksEmpty()), &recv, SLOT(onEmpty())); // no-warning + connect(&send, SIGNAL(looksEmpty(bool)), &recv, SLOT(onEmpty())); // no-warning + } +private: + Sender send; + Recv recv; +}; Index: unittests/StaticAnalyzer/CMakeLists.txt =================================================================== --- unittests/StaticAnalyzer/CMakeLists.txt +++ unittests/StaticAnalyzer/CMakeLists.txt @@ -4,10 +4,12 @@ add_clang_unittest(StaticAnalysisTests AnalyzerOptionsTest.cpp + QtSignalSlotCheckerTest.cpp ) target_link_libraries(StaticAnalysisTests clangBasic clangAnalysis - clangStaticAnalyzerCore + clangStaticAnalyzerCore + clangStaticAnalyzerCheckers ) Index: unittests/StaticAnalyzer/QtSignalSlotCheckerTest.cpp =================================================================== --- /dev/null +++ unittests/StaticAnalyzer/QtSignalSlotCheckerTest.cpp @@ -0,0 +1,37 @@ +#include "llvm/ADT/StringRef.h" +#include "gtest/gtest.h" + +namespace clang { +namespace ento { + +extern std::string qtNormalizeSignature(const llvm::StringRef &Method); + +TEST(QtSignalSlotChecker, QMetaObjectNormalizedSignature) { + const std::pair NormConv[] = { + { + "a(const int&, int const&, const int *, int const *, int * const, const int * const, int &, int *)", + "a(int,int,const int*,const int*,int*const,int*const,int&,int*)" + }, + { + "b(const unsigned&, const unsigned long&, unsigned char, std::vector&, std::vector const&, unsigned short, short)", + "b(uint,ulong,unsigned char,std::vector&,std::vector,unsigned short,short)" + }, + { + "c(const int * const * const)", + "c(int*const*const)" + }, + { + "d(class QString&, enum QScriptEngine::QObjectWrapOption, struct QString*)", + "d(QString&,QScriptEngine::QObjectWrapOption,QString*)" + }, + { + "a(int const a, const int& b)", + "a(int a,int&b)" + } + }; + for (auto&& InOut : NormConv) { + EXPECT_EQ(InOut.second, qtNormalizeSignature(InOut.first)); + } +} +} // end namespace ento +} // end namespace clang