Bug 1339537 - Part 2: Update the MOZ_NON_PARAM analysis to implicitly apply to alignas(_) types, r=ehsan
authorMichael Layzell <michael@thelayzells.com>
Thu, 27 Apr 2017 12:44:49 -0400
changeset 355459 36ec718cab5a4a716e8d8816ba1b80b40a71073f
parent 355458 bf1c55598c82e26cce1eadbccd066d0510823aee
child 355460 05353a4fa2761c18284ada5470492bc134fa2461
push id31727
push usercbook@mozilla.com
push dateFri, 28 Apr 2017 08:36:40 +0000
treeherdermozilla-central@8f2b930cd028 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1339537
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1339537 - Part 2: Update the MOZ_NON_PARAM analysis to implicitly apply to alignas(_) types, r=ehsan MozReview-Commit-ID: 2VDJRxxkVjV
build/clang-plugin/CustomTypeAnnotation.cpp
build/clang-plugin/CustomTypeAnnotation.h
build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
build/clang-plugin/tests/TestNonParameterChecker.cpp
--- a/build/clang-plugin/CustomTypeAnnotation.cpp
+++ b/build/clang-plugin/CustomTypeAnnotation.cpp
@@ -10,18 +10,16 @@ CustomTypeAnnotation StackClass =
 CustomTypeAnnotation GlobalClass =
     CustomTypeAnnotation("moz_global_class", "global");
 CustomTypeAnnotation NonHeapClass =
     CustomTypeAnnotation("moz_nonheap_class", "non-heap");
 CustomTypeAnnotation HeapClass =
     CustomTypeAnnotation("moz_heap_class", "heap");
 CustomTypeAnnotation NonTemporaryClass =
     CustomTypeAnnotation("moz_non_temporary_class", "non-temporary");
-CustomTypeAnnotation NonParam =
-    CustomTypeAnnotation("moz_non_param", "non-param");
 
 void CustomTypeAnnotation::dumpAnnotationReason(BaseCheck &Check,
                                                 QualType T,
                                                 SourceLocation Loc) {
   const char* Inherits =
       "%1 is a %0 type because it inherits from a %0 type %2";
   const char* Member =
       "%1 is a %0 type because member %2 is a %0 type %3";
--- a/build/clang-plugin/CustomTypeAnnotation.h
+++ b/build/clang-plugin/CustomTypeAnnotation.h
@@ -62,11 +62,10 @@ protected:
   virtual bool hasFakeAnnotation(const TagDecl *D) const { return false; }
 };
 
 extern CustomTypeAnnotation StackClass;
 extern CustomTypeAnnotation GlobalClass;
 extern CustomTypeAnnotation NonHeapClass;
 extern CustomTypeAnnotation HeapClass;
 extern CustomTypeAnnotation NonTemporaryClass;
-extern CustomTypeAnnotation NonParam;
 
 #endif
--- a/build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
+++ b/build/clang-plugin/NonParamInsideFunctionDeclChecker.cpp
@@ -1,15 +1,54 @@
 /* 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/. */
 
 #include "NonParamInsideFunctionDeclChecker.h"
 #include "CustomMatchers.h"
 
+class NonParamAnnotation : public CustomTypeAnnotation
+{
+public:
+  NonParamAnnotation() : CustomTypeAnnotation("moz_non_param", "non-param") {};
+
+protected:
+  // Adding alignas(_) on a struct implicitly marks it as MOZ_NON_PARAM, due to
+  // MSVC limitations which prevent passing explcitly aligned types by value as
+  // parameters. This overload of hasFakeAnnotation injects fake MOZ_NON_PARAM
+  // annotations onto these types.
+  bool hasFakeAnnotation(const TagDecl *D) const override {
+    // Check if the decl itself has an AlignedAttr on it.
+    for (const Attr *A : D->attrs()) {
+      if (isa<AlignedAttr>(A)) {
+        D->dump();
+        return true;
+      }
+    }
+
+    // Check if any of the decl's fields have an AlignedAttr on them.
+    if (auto RD = dyn_cast<RecordDecl>(D)) {
+      for (auto F : RD->fields()) {
+        for (auto A : F->attrs()) {
+          if (isa<AlignedAttr>(A)) {
+            D->dump();
+
+            return true;
+          }
+        }
+      }
+    }
+
+    // We don't need to check the types of fields, as the CustomTypeAnnotation
+    // infrastructure will handle that for us.
+    return false;
+  }
+};
+NonParamAnnotation NonParam;
+
 void NonParamInsideFunctionDeclChecker::registerMatchers(MatchFinder* AstMatcher) {
   AstMatcher->addMatcher(
       functionDecl(anyOf(allOf(isDefinition(),
                                hasAncestor(classTemplateSpecializationDecl()
                                                .bind("spec"))),
                          isDefinition()))
           .bind("func"),
       this);
@@ -33,16 +72,27 @@ void NonParamInsideFunctionDeclChecker::
   if (!func) {
     return;
   }
 
   if (func->isDeleted()) {
     return;
   }
 
+  // We need to skip decls which have these types as parameters in system
+  // headers, because presumably those headers act like an assertion that the
+  // alignment will be preserved in that situation.
+  if (getDeclarationNamespace(func) == "std") {
+    return;
+  }
+
+  if (inThirdPartyPath(func)) {
+    return;
+  }
+
   // Don't report errors on the same declarations more than once.
   if (CheckedFunctionDecls.count(func)) {
     return;
   }
   CheckedFunctionDecls.insert(func);
 
   const ClassTemplateSpecializationDecl *Spec =
       Result.Nodes.getNodeAs<ClassTemplateSpecializationDecl>("spec");
--- a/build/clang-plugin/tests/TestNonParameterChecker.cpp
+++ b/build/clang-plugin/tests/TestNonParameterChecker.cpp
@@ -172,8 +172,18 @@ void testLambda()
     auto nonParamStructUnionLambda = [](HasNonParamStructUnion x) -> void {}; //expected-error {{Type 'HasNonParamStructUnion' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
 
     (void)[](Param x) -> void {};
     (void)[](NonParam x) -> void {}; //expected-error {{Type 'NonParam' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamStruct x) -> void {}; //expected-error {{Type 'HasNonParamStruct' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamUnion x) -> void {}; //expected-error {{Type 'HasNonParamUnion' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
     (void)[](HasNonParamStructUnion x) -> void {}; //expected-error {{Type 'HasNonParamStructUnion' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
 }
+
+// Check that alignas() implies the MOZ_NON_PARAM attribute.
+
+struct alignas(8) AlignasStruct { char a; };
+void takesAlignasStruct(AlignasStruct x) { } // expected-error {{Type 'AlignasStruct' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
+void takesAlignasStructByRef(const AlignasStruct& x) { }
+
+struct AlignasMember { alignas(8) char a; };
+void takesAlignasMember(AlignasMember x) { } // expected-error {{Type 'AlignasMember' must not be used as parameter}} expected-note {{Please consider passing a const reference instead}}
+void takesAlignasMemberByRef(const AlignasMember& x) { }