diff --git a/Changelog b/Changelog --- a/Changelog +++ b/Changelog @@ -124,4 +124,5 @@ * v1.7 (, 2020) - New Checks: - overloaded signal + - invalid JNI signatures - qstring-arg warns when using QLatin1String::arg(int), as it casts to QChar diff --git a/CheckSources.cmake b/CheckSources.cmake --- a/CheckSources.cmake +++ b/CheckSources.cmake @@ -19,6 +19,7 @@ ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/thread-with-slots.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/tr-non-literal.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/unneeded-cast.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/checks/manuallevel/jnisignatures.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/level0/connect-by-name.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/level0/connect-non-signal.cpp ${CMAKE_CURRENT_LIST_DIR}/src/checks/level0/connect-not-normalized.cpp diff --git a/README.md b/README.md --- a/README.md +++ b/README.md @@ -234,6 +234,7 @@ - [thread-with-slots](docs/checks/README-thread-with-slots.md) - [tr-non-literal](docs/checks/README-tr-non-literal.md) - [unneeded-cast](docs/checks/README-unneeded-cast.md) + - [jni-sigantures](docs/checks/README-jni-signatures.md) - Checks from Level 0: - [connect-by-name](docs/checks/README-connect-by-name.md) diff --git a/checks.json b/checks.json --- a/checks.json +++ b/checks.json @@ -616,6 +616,12 @@ "level" : -1, "categories" : ["containers"], "visits_stmts" : true + }, + { + "name" : "jni-signatures", + "level" : -1, + "categories" : ["bug"], + "visits_stmts" : true } ] } diff --git a/clazy.cmake b/clazy.cmake --- a/clazy.cmake +++ b/clazy.cmake @@ -107,6 +107,18 @@ ExtraClangOptions="-Xclang -plugin-arg-clazy -Xclang visit-implicit-code" fi +if [[ $CLAZY_CHECKS == *"jni-signatures"* ]] +then + + if [[ -z $ANDROID_NDK ]] + then + echo "To test JNI signatures you need to set ANDROID_NDK to your Android NDK installation." + exit + fi + + ExtraClangOptions=$ExtraClangOptions" -idirafter"$ANDROID_NDK"/sysroot/usr/include" +fi + ClazyPluginLib=ClazyPlugin@CMAKE_SHARED_LIBRARY_SUFFIX@ if ( test -f "$libdir/$ClazyPluginLib" ) diff --git a/docs/checks/README-jni-signatures.md b/docs/checks/README-jni-signatures.md new file mode 100644 --- /dev/null +++ b/docs/checks/README-jni-signatures.md @@ -0,0 +1,11 @@ +# jni-signatures + +Tries to find invalid JNI signatures in QAndroidJniObject usage. Those will lead to runtime errors. + +#### Example +``` + QAndroidJniObject::callStaticMethod("toString", "()Ljava/lang/String"); // Should be '()Ljava/lang/String;' +``` + +#### Pitfalls +For this check to work you need to set the ANDROID_NDK environment variable to your Android NDK installation. diff --git a/src/Checks.h b/src/Checks.h --- a/src/Checks.h +++ b/src/Checks.h @@ -47,6 +47,7 @@ #include "checks/manuallevel/thread-with-slots.h" #include "checks/manuallevel/tr-non-literal.h" #include "checks/manuallevel/unneeded-cast.h" +#include "checks/manuallevel/jnisignatures.h" #include "checks/level0/connect-by-name.h" #include "checks/level0/connect-non-signal.h" #include "checks/level0/connect-not-normalized.h" @@ -143,6 +144,7 @@ registerCheck(check("thread-with-slots", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts | RegisteredCheck::Option_VisitsDecls)); registerCheck(check("tr-non-literal", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); registerCheck(check("unneeded-cast", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); + registerCheck(check("jni-signatures", ManualCheckLevel, RegisteredCheck::Option_VisitsStmts)); registerCheck(check("connect-by-name", CheckLevel0, RegisteredCheck::Option_VisitsDecls)); registerCheck(check("connect-non-signal", CheckLevel0, RegisteredCheck::Option_Qt4Incompatible | RegisteredCheck::Option_VisitsStmts)); registerCheck(check("connect-not-normalized", CheckLevel0, RegisteredCheck::Option_VisitsStmts)); diff --git a/src/checks/manuallevel/jnisignatures.h b/src/checks/manuallevel/jnisignatures.h new file mode 100644 --- /dev/null +++ b/src/checks/manuallevel/jnisignatures.h @@ -0,0 +1,52 @@ +/* + This file is part of the clazy static checker. + + Copyright (C) 2020 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + Author: Nicolas Fella + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Library General Public + License as published by the Free Software Foundation; either + version 2 of the License, or (at your option) any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Library General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#ifndef JNISIGNATURES_H +#define JNISIGNATURES_H + +#include "checkbase.h" + +#include + +class ClazyContext; +namespace clang { +class Stmt; +class CXXMemberCallExpr; +class FunctionDecl; +} // namespace clang + +class JniSignatures + : public CheckBase +{ +public: + JniSignatures(const std::string &name, ClazyContext *context); + void VisitStmt(clang::Stmt *) override; +private: + template + void checkArgAt(T *call, unsigned int index); + void checkConstructorCall(clang::Stmt *stm); + void checkFunctionCall(clang::Stmt *stm); + bool functionShouldBeChecked(clang::FunctionDecl *funDecl); +}; + +#endif + diff --git a/src/checks/manuallevel/jnisignatures.cpp b/src/checks/manuallevel/jnisignatures.cpp new file mode 100644 --- /dev/null +++ b/src/checks/manuallevel/jnisignatures.cpp @@ -0,0 +1,138 @@ +/* + This file is part of the clazy static checker. + + Copyright (C) 2020 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com + Author: Nicolas Fella + + This library is free software; you can redistribute it and/or + modify it under the terms of the GNU Library General Public + License as published by the Free Software Foundation; either + version 2 of the License, or (at your option) any later version. + + This library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Library General Public License for more details. + + You should have received a copy of the GNU Library General Public License + along with this library; see the file COPYING.LIB. If not, write to + the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + Boston, MA 02110-1301, USA. +*/ + +#include "jnisignatures.h" +#include "MacroUtils.h" +#include "FunctionUtils.h" +#include "StringUtils.h" +#include "SourceCompatibilityHelpers.h" +#include "clazy_stl.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#include + +class ClazyContext; + +using namespace clang; +using namespace std; + +JniSignatures::JniSignatures(const std::string &name, ClazyContext *context) + : CheckBase(name, context, Option_CanIgnoreIncludes) +{ +} + +bool checkSignature(std::string signature) +{ + static regex rx("\\((\\[?([ZBCSIJFD]|L([a-zA-Z]+\\/)*[a-zA-Z]+;))*\\)\\[?([ZBCSIJFD]|L([a-zA-Z]+\\/)*[a-zA-Z]+;|V)"); + + smatch match; + return regex_match(signature, match, rx); +} + +template +void JniSignatures::checkArgAt(T *call, unsigned int index) +{ + if (call->getNumArgs() < index + 1) + return; + + StringLiteral *stringLiteral = clazy::getFirstChildOfType2(call->getArg(index)); + + if (!stringLiteral) + return; + + if (stringLiteral->getCharByteWidth() != 1) + return; + + const std::string signature = stringLiteral->getString().str(); + + const bool valid = checkSignature(signature); + + if (!valid) { + emitWarning(call, "Invalid method signature: '" + signature + "'"); + } +} + +bool JniSignatures::functionShouldBeChecked(FunctionDecl *funDecl) +{ + const std::string qualifiedName = funDecl->getQualifiedNameAsString(); + if (!clazy::startsWith(qualifiedName, "QAndroidJniObject::")) { + return false; + } + + const std::string name = clazy::name(funDecl); + return name == "callObjectMethod" + || name == "callMethod" + || name == "callStaticObjectMethod" + || name == "callStaticMethod"; +} + +void JniSignatures::checkFunctionCall(Stmt *stm) +{ + auto callExpr = dyn_cast(stm); + if (!callExpr) + return; + auto funDecl = callExpr->getDirectCallee(); + if (!funDecl) { + return; + } + + if (!functionShouldBeChecked(funDecl)) { + return; + } + + checkArgAt(callExpr, 1); +} + +void JniSignatures::checkConstructorCall(Stmt *stm) +{ + auto constructExpr = dyn_cast(stm); + if (!constructExpr) { + return; + } + auto funDecl = constructExpr->getConstructor(); + + const std::string qualifiedName = funDecl->getQualifiedNameAsString(); + if (qualifiedName != "QAndroidJniObject::QAndroidJniObject") { + return; + } + + checkArgAt(constructExpr, 1); +} + +void JniSignatures::VisitStmt(Stmt *stm) +{ + checkConstructorCall(stm); + checkFunctionCall(stm); +} diff --git a/tests/jni-signatures/config.json b/tests/jni-signatures/config.json new file mode 100644 --- /dev/null +++ b/tests/jni-signatures/config.json @@ -0,0 +1,8 @@ +{ + "tests" : [ + { + "filename" : "main.cpp" + } + ] +} + diff --git a/tests/jni-signatures/main.cpp b/tests/jni-signatures/main.cpp new file mode 100644 --- /dev/null +++ b/tests/jni-signatures/main.cpp @@ -0,0 +1,35 @@ + +// Crude approximation of #include so we don't need QtAndroidExtras installed +class QAndroidJniObject +{ + public: + QAndroidJniObject(const char *className, const char *signature); + + template + void callObjectMethod(const char *methodName, const char *signature); + + template + void callMethod(const char *methodName, const char *signature); + + template + static void callStaticMethod(const char *methodName, const char *signature); + + template + static void callStaticObjectMethod(const char *methodName, const char *signature); +}; + +int main() { + + QAndroidJniObject obj("java/lang/String", "("); + obj.callObjectMethod("someMethod", "()V"); + obj.callObjectMethod("someMethod", "(III)V"); + obj.callObjectMethod("someMethod", "(III"); + + obj.callMethod("someMethod", "()V"); + obj.callMethod("someMethod", "(III)V"); + obj.callMethod("someMethod", "()Ljava/lang/String"); + + QAndroidJniObject::callStaticMethod("someMethod", "()Ljava/lang/String"); + + QAndroidJniObject::callStaticObjectMethod("someOtherMethod", "(II)"); +} diff --git a/tests/jni-signatures/main.cpp.expected b/tests/jni-signatures/main.cpp.expected new file mode 100644 --- /dev/null +++ b/tests/jni-signatures/main.cpp.expected @@ -0,0 +1,5 @@ +jni-signatures/main.cpp:23:23: warning: Invalid method signature: '(' [-Wclazy-jni-signatures] +jni-signatures/main.cpp:26:5: warning: Invalid method signature: '(III' [-Wclazy-jni-signatures] +jni-signatures/main.cpp:30:5: warning: Invalid method signature: '()Ljava/lang/String' [-Wclazy-jni-signatures] +jni-signatures/main.cpp:32:5: warning: Invalid method signature: '()Ljava/lang/String' [-Wclazy-jni-signatures] +jni-signatures/main.cpp:34:5: warning: Invalid method signature: '(II)' [-Wclazy-jni-signatures]