Bug 1283395 - clang-plugin - add an error if we encounter in MOZ_ASSERT assignment instead of logical expression. r?mystor
MozReview-Commit-ID: AybStmi6MIH
--- a/build/clang-plugin/clang-plugin.cpp
+++ b/build/clang-plugin/clang-plugin.cpp
@@ -34,16 +34,42 @@ typedef ASTConsumer *ASTConsumerPtr;
// ones for compatibility with older versions.
#define cxxConstructExpr constructExpr
#define cxxConstructorDecl constructorDecl
#define cxxMethodDecl methodDecl
#define cxxNewExpr newExpr
#define cxxRecordDecl recordDecl
#endif
+// Check if the given expression contains an assignment expression.
+// This can either take the form of a Binary Operator or a
+// Overloaded Operator Call.
+bool HasSideEffectAssignment(const Expr *expr) {
+ if (auto opCallExpr = dyn_cast_or_null<CXXOperatorCallExpr>(expr)) {
+ auto binOp = opCallExpr->getOperator();
+ if (binOp == OO_Equal || (binOp >= OO_PlusEqual && binOp <= OO_PipeEqual)) {
+ return true;
+ }
+ } else if (auto binOpExpr = dyn_cast_or_null<BinaryOperator>(expr)) {
+ if (binOpExpr->isAssignmentOp()) {
+ return true;
+ }
+ }
+
+ // Recurse to children.
+ for (const Stmt *SubStmt : expr->children()) {
+ auto childExpr = dyn_cast_or_null<Expr>(SubStmt);
+ if (childExpr && HasSideEffectAssignment(childExpr)) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
namespace {
using namespace clang::ast_matchers;
class DiagnosticsMatcher {
public:
DiagnosticsMatcher();
ASTConsumerPtr makeASTConsumer() { return astMatcher.newASTConsumer(); }
@@ -119,31 +145,37 @@ private:
virtual void run(const MatchFinder::MatchResult &Result);
};
class RefCountedCopyConstructorChecker : public MatchFinder::MatchCallback {
public:
virtual void run(const MatchFinder::MatchResult &Result);
};
+ class AssertAssignmentChecker : public MatchFinder::MatchCallback {
+ public:
+ virtual void run(const MatchFinder::MatchResult &Result);
+ };
+
ScopeChecker scopeChecker;
ArithmeticArgChecker arithmeticArgChecker;
TrivialCtorDtorChecker trivialCtorDtorChecker;
NaNExprChecker nanExprChecker;
NoAddRefReleaseOnReturnChecker noAddRefReleaseOnReturnChecker;
RefCountedInsideLambdaChecker refCountedInsideLambdaChecker;
ExplicitOperatorBoolChecker explicitOperatorBoolChecker;
NoDuplicateRefCntMemberChecker noDuplicateRefCntMemberChecker;
NeedsNoVTableTypeChecker needsNoVTableTypeChecker;
NonMemMovableTemplateArgChecker nonMemMovableTemplateArgChecker;
NonMemMovableMemberChecker nonMemMovableMemberChecker;
ExplicitImplicitChecker explicitImplicitChecker;
NoAutoTypeChecker noAutoTypeChecker;
NoExplicitMoveConstructorChecker noExplicitMoveConstructorChecker;
RefCountedCopyConstructorChecker refCountedCopyConstructorChecker;
+ AssertAssignmentChecker assertAttributionChecker;
MatchFinder astMatcher;
};
namespace {
std::string getDeclarationNamespace(const Decl *decl) {
const DeclContext *DC =
decl->getDeclContext()->getEnclosingNamespaceContext();
@@ -757,16 +789,25 @@ AST_MATCHER(QualType, autoNonAutoableTyp
AST_MATCHER(CXXConstructorDecl, isExplicitMoveConstructor) {
return Node.isExplicit() && Node.isMoveConstructor();
}
AST_MATCHER(CXXConstructorDecl, isCompilerProvidedCopyConstructor) {
return !Node.isUserProvided() && Node.isCopyConstructor();
}
+
+AST_MATCHER(CallExpr, isAssertAssignmentTestFunc) {
+ static const std::string assertName = "MOZ_AssertAssignmentTest";
+ const FunctionDecl *method = Node.getDirectCallee();
+
+ return method
+ && method->getDeclName().isIdentifier()
+ && method->getName() == assertName;
+}
}
}
namespace {
void CustomTypeAnnotation::dumpAnnotationReason(DiagnosticsEngine &Diag,
QualType T,
SourceLocation Loc) {
@@ -1065,16 +1106,20 @@ DiagnosticsMatcher::DiagnosticsMatcher()
&noExplicitMoveConstructorChecker);
astMatcher.addMatcher(
cxxConstructExpr(
hasDeclaration(cxxConstructorDecl(isCompilerProvidedCopyConstructor(),
ofClass(hasRefCntMember()))))
.bind("node"),
&refCountedCopyConstructorChecker);
+
+ astMatcher.addMatcher(
+ callExpr(isAssertAssignmentTestFunc()).bind("funcCall"),
+ &assertAttributionChecker);
}
// These enum variants determine whether an allocation has occured in the code.
enum AllocationVariety {
AV_None,
AV_Global,
AV_Automatic,
AV_Temporary,
@@ -1540,16 +1585,28 @@ void DiagnosticsMatcher::RefCountedCopyC
// Everything we needed to know was checked in the matcher - we just report
// the error here
const CXXConstructExpr *E = Result.Nodes.getNodeAs<CXXConstructExpr>("node");
Diag.Report(E->getLocation(), ErrorID);
Diag.Report(E->getLocation(), NoteID);
}
+void DiagnosticsMatcher::AssertAssignmentChecker::run(
+ const MatchFinder::MatchResult &Result) {
+ DiagnosticsEngine &Diag = Result.Context->getDiagnostics();
+ unsigned assignInsteadOfComp = Diag.getDiagnosticIDs()->getCustomDiagID(
+ DiagnosticIDs::Error, "Forbidden assignment in assert expression");
+ const CallExpr *funcCall = Result.Nodes.getNodeAs<CallExpr>("funcCall");
+
+ if (funcCall && HasSideEffectAssignment(funcCall)) {
+ Diag.Report(funcCall->getLocStart(), assignInsteadOfComp);
+ }
+}
+
class MozCheckAction : public PluginASTAction {
public:
ASTConsumerPtr CreateASTConsumer(CompilerInstance &CI,
StringRef fileName) override {
#if CLANG_VERSION_FULL >= 306
std::unique_ptr<MozChecker> checker(llvm::make_unique<MozChecker>(CI));
ASTConsumerPtr other(checker->getOtherConsumer());
new file mode 100644
--- /dev/null
+++ b/build/clang-plugin/tests/TestAssertWithAssignment.cpp
@@ -0,0 +1,68 @@
+#include "mozilla/MacroArgs.h"
+
+static __attribute__((always_inline)) bool MOZ_AssertAssignmentTest(bool expr) {
+ return expr;
+}
+
+#define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
+#define MOZ_CRASH() do { } while(0)
+#define MOZ_CHECK_ASSERT_ASSIGNMENT(expr) MOZ_AssertAssignmentTest(!!(expr))
+
+#define MOZ_ASSERT_HELPER1(expr) \
+ do { \
+ if (MOZ_UNLIKELY(!MOZ_CHECK_ASSERT_ASSIGNMENT(expr))) { \
+ MOZ_CRASH();\
+ } \
+ } while(0) \
+
+/* Now the two-argument form. */
+#define MOZ_ASSERT_HELPER2(expr, explain) \
+ do { \
+ if (MOZ_UNLIKELY(!MOZ_CHECK_ASSERT_ASSIGNMENT(expr))) { \
+ MOZ_CRASH();\
+ } \
+ } while(0) \
+
+#define MOZ_RELEASE_ASSERT_GLUE(a, b) a b
+#define MOZ_RELEASE_ASSERT(...) \
+ MOZ_RELEASE_ASSERT_GLUE( \
+ MOZ_PASTE_PREFIX_AND_ARG_COUNT(MOZ_ASSERT_HELPER, __VA_ARGS__), \
+ (__VA_ARGS__))
+
+#define MOZ_ASSERT(...) MOZ_RELEASE_ASSERT(__VA_ARGS__)
+
+void FunctionTest(int p) {
+ MOZ_ASSERT(p = 1); // expected-error {{Forbidden assignment in assert expression}}
+}
+
+void FunctionTest2(int p) {
+ MOZ_ASSERT(((p = 1))); // expected-error {{Forbidden assignment in assert expression}}
+}
+
+void FunctionTest3(int p) {
+ MOZ_ASSERT(p != 3);
+}
+
+class TestOverloading {
+ int value;
+public:
+ explicit TestOverloading(int _val) : value(_val) {}
+ // different operators
+ explicit operator bool() const { return true; }
+ TestOverloading& operator=(const int _val) { value = _val; return *this; }
+
+ int& GetInt() {return value;}
+};
+
+void TestOverloadingFunc() {
+ TestOverloading p(2);
+ int f;
+
+ MOZ_ASSERT(p);
+ MOZ_ASSERT(p = 3); // expected-error {{Forbidden assignment in assert expression}}
+ MOZ_ASSERT(p, "p is not valid");
+ MOZ_ASSERT(p = 3, "p different than 3"); // expected-error {{Forbidden assignment in assert expression}}
+ MOZ_ASSERT(p.GetInt() = 2); // expected-error {{Forbidden assignment in assert expression}}
+ MOZ_ASSERT(p.GetInt() == 2);
+ MOZ_ASSERT(p.GetInt() == 2, f = 3);
+}
--- a/build/clang-plugin/tests/moz.build
+++ b/build/clang-plugin/tests/moz.build
@@ -3,16 +3,17 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
# dummy library name to avoid skipping building the sources here.
Library('clang-plugin-tests')
SOURCES += [
+ 'TestAssertWithAssignment.cpp',
'TestBadImplicitConversionCtor.cpp',
'TestCustomHeap.cpp',
'TestExplicitOperatorBool.cpp',
'TestGlobalClass.cpp',
'TestHeapClass.cpp',
'TestInheritTypeAnnotationsFromTemplateArgs.cpp',
'TestMultipleAnnotations.cpp',
'TestMustOverride.cpp',