diff --git a/build/clang-plugin/Makefile.in b/build/clang-plugin/Makefile.in index a3ef7f763686..0e39dfcc4b54 100644 --- a/build/clang-plugin/Makefile.in +++ b/build/clang-plugin/Makefile.in @@ -16,6 +16,7 @@ CPPSRCS := \ TESTSRCS := \ TestMustOverride.cpp \ + TestStackClass.cpp \ $(NULL) OBJS := $(patsubst %.cpp,%.o,$(CPPSRCS)) @@ -30,7 +31,7 @@ $(OBJS): %.o: %.cpp Makefile $(PLUGIN): $(OBJS) rm -f $@ - $(CXX) -shared -o $@ $(CXXFLAGS) $(LDFLAGS) $(OBJS) + $(CXX) -shared -o $@ $(CXXFLAGS) $(LDFLAGS) $(OBJS) -lclangASTMatchers TESTFLAGS := -fsyntax-only -Xclang -verify \ -Xclang -load -Xclang $(CURDIR)/$(PLUGIN) \ diff --git a/build/clang-plugin/clang-plugin.cpp b/build/clang-plugin/clang-plugin.cpp index d1340c0b69a7..1f4d13e7c7b2 100644 --- a/build/clang-plugin/clang-plugin.cpp +++ b/build/clang-plugin/clang-plugin.cpp @@ -4,9 +4,12 @@ #include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/Version.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendPluginRegistry.h" +#include "clang/Frontend/MultiplexConsumer.h" #include "clang/Sema/Sema.h" #define CLANG_VERSION_FULL (CLANG_VERSION_MAJOR * 100 + CLANG_VERSION_MINOR) @@ -15,11 +18,37 @@ using namespace llvm; using namespace clang; namespace { + +using namespace clang::ast_matchers; +class DiagnosticsMatcher { +public: + DiagnosticsMatcher(); + + ASTConsumer *makeASTConsumer() { + return astMatcher.newASTConsumer(); + } + +private: + class StackClassChecker : public MatchFinder::MatchCallback { + public: + virtual void run(const MatchFinder::MatchResult &Result); + }; + + StackClassChecker stackClassChecker; + MatchFinder astMatcher; +}; + class MozChecker : public ASTConsumer, public RecursiveASTVisitor { DiagnosticsEngine &Diag; const CompilerInstance &CI; + DiagnosticsMatcher matcher; public: MozChecker(const CompilerInstance &CI) : Diag(CI.getDiagnostics()), CI(CI) {} + + ASTConsumer *getOtherConsumer() { + return matcher.makeASTConsumer(); + } + virtual void HandleTranslationUnit(ASTContext &ctx) { TraverseDecl(ctx.getTranslationUnitDecl()); } @@ -58,6 +87,16 @@ public: if (hasCustomAnnotation(*M, "moz_must_override")) must_overrides.push_back(*M); } + + // While we are looping over parent classes, we'll also check to make sure + // that the subclass has the annotation if the superclass does. + if (hasCustomAnnotation(parent, "moz_stack_class") && + !hasCustomAnnotation(d, "moz_stack_class")) { + unsigned badInheritID = Diag.getDiagnosticIDs()->getCustomDiagID( + DiagnosticIDs::Error, "%0 inherits from a stack class %1"); + Diag.Report(d->getLocation(), badInheritID) + << d->getDeclName() << parent->getDeclName(); + } } for (OverridesVector::iterator it = must_overrides.begin(); @@ -84,11 +123,78 @@ public: return true; } }; +} + +namespace clang { +namespace ast_matchers { + +/// This matcher will match any class with the stack class assertion or an +/// array of such classes. +AST_MATCHER(QualType, stackClassAggregate) { + QualType t = Node; + while (const ArrayType *arrTy = t->getAsArrayTypeUnsafe()) + t = arrTy->getElementType(); + CXXRecordDecl *clazz = t->getAsCXXRecordDecl(); + return clazz && MozChecker::hasCustomAnnotation(clazz, "moz_stack_class"); +} +} +} + +namespace { + +DiagnosticsMatcher::DiagnosticsMatcher() { + // Stack class assertion: non-local variables of a stack class are forbidden + // (non-localness checked in the callback) + astMatcher.addMatcher(varDecl(hasType(stackClassAggregate())).bind("node"), + &stackClassChecker); + // Stack class assertion: new stack class is forbidden (unless placement new) + astMatcher.addMatcher(newExpr(hasType(pointerType( + pointee(stackClassAggregate()) + ))).bind("node"), &stackClassChecker); + // Stack class assertion: a stack-class field is permitted only if it's a + // member of a class with the annotation + astMatcher.addMatcher(fieldDecl(hasType(stackClassAggregate())).bind("field"), + &stackClassChecker); +} + +void DiagnosticsMatcher::StackClassChecker::run( + const MatchFinder::MatchResult &Result) { + DiagnosticsEngine &Diag = Result.Context->getDiagnostics(); + unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID( + DiagnosticIDs::Error, "variable of type %0 only valid on the stack"); + if (const VarDecl *d = Result.Nodes.getNodeAs("node")) { + // Ignore the match if it's a local variable. + if (d->hasLocalStorage()) + return; + Diag.Report(d->getLocation(), stackID) << d->getType(); + } else if (const CXXNewExpr *expr = + Result.Nodes.getNodeAs("node")) { + // If it's placement new, then this match doesn't count. + if (expr->getNumPlacementArgs() > 0) + return; + Diag.Report(expr->getStartLoc(), stackID) << expr->getAllocatedType(); + } else if (const FieldDecl *field = + Result.Nodes.getNodeAs("field")) { + // AST matchers don't let me get the class that contains a field... + const RecordDecl *parent = field->getParent(); + if (!MozChecker::hasCustomAnnotation(parent, "moz_stack_class")) { + // We use a more verbose error message here. + unsigned stackID = Diag.getDiagnosticIDs()->getCustomDiagID( + DiagnosticIDs::Error, + "member of type %0 in class %1 that is not a stack class"); + Diag.Report(field->getLocation(), stackID) << field->getType() << + parent->getDeclName(); + } + } +} class MozCheckAction : public PluginASTAction { public: ASTConsumer *CreateASTConsumer(CompilerInstance &CI, StringRef fileName) { - return new MozChecker(CI); + MozChecker *checker = new MozChecker(CI); + + ASTConsumer *consumers[] = { checker, checker->getOtherConsumer() }; + return new MultiplexConsumer(consumers); } bool ParseArgs(const CompilerInstance &CI, diff --git a/build/clang-plugin/tests/TestStackClass.cpp b/build/clang-plugin/tests/TestStackClass.cpp new file mode 100644 index 000000000000..4ad6398fe56e --- /dev/null +++ b/build/clang-plugin/tests/TestStackClass.cpp @@ -0,0 +1,47 @@ +#define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class"))) +#include + +struct MOZ_STACK_CLASS Stack { + int i; + void *operator new(size_t x) { return 0; } + void *operator new(size_t blah, char *buffer) { return buffer; } +}; + +template +struct MOZ_STACK_CLASS TemplateClass { + T i; +}; + +void gobble(void *) { } + +void misuseStackClass(int len) { + Stack valid; + Stack alsoValid[2]; + static Stack notValid; // expected-error {{variable of type 'Stack' only valid on the stack}} + static Stack alsoNotValid[2]; // expected-error {{variable of type 'Stack [2]' only valid on the stack}} + + gobble(&valid); + gobble(¬Valid); + gobble(&alsoValid[0]); + + gobble(new Stack); // expected-error {{variable of type 'Stack' only valid on the stack}} + gobble(new Stack[10]); // expected-error {{variable of type 'Stack' only valid on the stack}} + gobble(new TemplateClass); // expected-error {{variable of type 'TemplateClass' only valid on the stack}} + gobble(len <= 5 ? &valid : new Stack); // expected-error {{variable of type 'Stack' only valid on the stack}} + + char buffer[sizeof(Stack)]; + gobble(new (buffer) Stack); +} + +Stack notValid; // expected-error {{variable of type 'Stack' only valid on the stack}} +struct RandomClass { + Stack nonstaticMember; // expected-error {{member of type 'Stack' in class 'RandomClass' that is not a stack class}} + static Stack staticMember; // expected-error {{variable of type 'Stack' only valid on the stack}} +}; +struct MOZ_STACK_CLASS RandomStackClass { + Stack nonstaticMember; + static Stack staticMember; // expected-error {{variable of type 'Stack' only valid on the stack}} +}; + +struct BadInherit : Stack {}; // expected-error {{'BadInherit' inherits from a stack class 'Stack'}} +struct MOZ_STACK_CLASS GoodInherit : Stack {}; diff --git a/mfbt/Attributes.h b/mfbt/Attributes.h index 2f2d12e8e519..edbc8b23f014 100644 --- a/mfbt/Attributes.h +++ b/mfbt/Attributes.h @@ -370,11 +370,19 @@ * attribute is not limited to virtual methods, so if it is applied to a * nonvirtual method and the subclass does not provide an equivalent * definition, the compiler will emit an error. + * MOZ_STACK_CLASS: Applies to all classes. Any class with this annotation is + * expected to live on the stack, so it is a compile-time error to use it, or + * an array of such objects, as a global or static variable, or as the type of + * a new expression (unless placement new is being used). It may be a base or + * a member of another class only if both classes are marked with this + * annotation. */ #ifdef MOZ_CLANG_PLUGIN # define MOZ_MUST_OVERRIDE __attribute__((annotate("moz_must_override"))) +# define MOZ_STACK_CLASS __attribute__((annotate("moz_stack_class"))) #else # define MOZ_MUST_OVERRIDE /* nothing */ +# define MOZ_STACK_CLASS /* nothing */ #endif /* MOZ_CLANG_PLUGIN */ #endif /* __cplusplus */