Revert deletion of no_randomize_Layout code

This reverts commits
	* f06d4cc.
	* 9949798.

Co-Authored-By: Cole Nixon <[email protected]>
Co-Authored-By: Jeff Takahashi <[email protected]>

Author: 3 people

clang/include/clang/AST/RecordFieldReorganizer.h

@@ -37,7 +37,7 @@ class RecordFieldReorganizer {
 };
 
 class Randstruct : public RecordFieldReorganizer {
-public:  
+public:
 /// Determines if the Record can be safely and easily randomized based on certain criteria (see implementation).
   static bool isTriviallyRandomizable(const RecordDecl *D);
 protected:

clang/include/clang/Basic/Attr.td

@@ -3208,3 +3208,10 @@ def RandomizeLayout : InheritableAttr {
   let Subjects = SubjectList<[Record]>;
   let Documentation = [ClangRandstructDocs];
 }
+
+def NoRandomizeLayout : InheritableAttr {
+  let Spellings = [GCC<"no_randomize_layout">, Declspec<"no_randomize_layout">,
+    Keyword<"no_randomize_layout">];
+  let Subjects = SubjectList<[Record]>;
+  let Documentation = [ClangRandstructDocs];
+}

clang/include/clang/Basic/AttrDocs.td

@@ -4099,10 +4099,24 @@ When compiled without ``-fobjc-arc``, this attribute is ignored.
 
 def ClangRandstructDocs : Documentation {
   let Category = DocCatVariable;
+  let Heading = "randomize_layout, no_randomize_layout";
   let Content = [{
-The attribute ``randomize_layout`` can be applied to the declaration of
-a record. ``randomize_layout`` instructs the compiler to randomize the memory layout 
+The attributes ``randomize_layout`` and ``no_randomize_layout`` can be applied
+to a record.
+
+``randomize_layout`` instructs the compiler to randomize the memory layout
 of the member variables of the record.
+
+Conversely, ``no_randomize_layout`` is used to indicate that if using the
+automatic strucuture selection feature of the Randstruct implementation, the
+compiler should not shuffle the members of the record.
+
+In the event that a record is labeled with both attributes, the compiler will
+emit a warning indicating that these two cannot be used on the same record.
+The default behavior in this case is to not randomize the struct, as the
+attribute ``no_randomize_layout`` takes precedence over ``randomize_layout``.
+This is implementation defined behavior.
+
 .. code-block:: c
 
   // Indicates that this struct should be randomized by Randstruct implementation.
@@ -4111,5 +4125,19 @@ of the member variables of the record.
     char *b;
     char *c;
   }__attribute__((randomize_layout));
+
+  // Indicates that this struct should NOT be randomized by Randstruct implementation.
+  struct s {
+    char *a;
+    char *b;
+    char *c;
+  }__attribute__((no_randomize_layout));
+
+  // Emits compiler warning. Struct is NOT randomized by Randstruct implementation.
+  struct s {
+    char *a;
+    char *b;
+    char *c;
+  }__attribute__((randomize_layout)) __attribute__((no_randomize_layout));
 }];
 }

clang/include/clang/Basic/DiagnosticASTKinds.td

@@ -345,4 +345,7 @@ def warn_unnecessary_packed : Warning<
   "packed attribute is unnecessary for %0">, InGroup<Packed>, DefaultIgnore;
 def warn_randomize_attr_union : Warning<
   "union declared with 'randomize_layout' attribute">, InGroup<DiagGroup<"randomize-layout">>;
+def warn_randomize_attr_conflict : Warning<
+  "struct declared with 'randomize_layout' and 'no_randomize_layout' attributes; "
+  "attribute 'no_randomize_layout' takes precedence">, InGroup<DiagGroup<"no-randomize-layout">>;
 }

clang/lib/AST/RecordFieldReorganizer.cpp

@@ -255,7 +255,7 @@ void Randstruct::reorganize(const ASTContext &C, const RecordDecl *D,
 }
 bool Randstruct::isTriviallyRandomizable(const RecordDecl *D) {
   for (auto f : D->fields()){
-    //If an element of the structure does not have a 
+    //If an element of the structure does not have a
     //function type is not a function pointer
     if(f->getFunctionType() == nullptr){ return false; }
   }

clang/lib/AST/RecordLayoutBuilder.cpp

@@ -2988,17 +2988,22 @@ ASTContext::getASTRecordLayout(const RecordDecl *D) const {
 
   const ASTRecordLayout *NewEntry = nullptr;
 
-  bool ShouldBeRandomized = (RandstructAutoSelect && Randstruct::isTriviallyRandomizable(D)) || D->getAttr<RandomizeLayoutAttr>() != nullptr;
-  if (ShouldBeRandomized) {
-    // There is no technical benefit to randomizing the fields of a union
-    // since they all share the same offset of zero.
-    if (D->isUnion()) {
+  bool ShouldBeRandomized = D->getAttr<RandomizeLayoutAttr>() != nullptr;
+  bool NotToBeRandomized = D->getAttr<NoRandomizeLayoutAttr>() != nullptr;
+  bool AutoSelectable = RandstructAutoSelect && Randstruct::isTriviallyRandomizable(D);
+
+  if (ShouldBeRandomized && NotToBeRandomized) {
+    getDiagnostics().Report(D->getLocation(), diag::warn_randomize_attr_conflict);
+  }
+
+  if (ShouldBeRandomized && D->isUnion()) {
       getDiagnostics().Report(D->getLocation(), diag::warn_randomize_attr_union);
-    }
-    else {
-      Randstruct randstruct;
-      randstruct.reorganizeFields(*this,D);
-    }
+      NotToBeRandomized = true;
+  }
+
+  if (!NotToBeRandomized && (ShouldBeRandomized || AutoSelectable)) {
+    Randstruct randstruct;
+    randstruct.reorganizeFields(*this,D);
   }
 
   if (isMsLayout(*this)) {

clang/lib/Sema/SemaDeclAttr.cpp

@@ -6856,6 +6856,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
   case ParsedAttr::AT_RandomizeLayout:
     handleSimpleAttribute<RandomizeLayoutAttr>(S, D, AL);
     break;
+  case ParsedAttr::AT_NoRandomizeLayout:
+    handleSimpleAttribute<NoRandomizeLayoutAttr>(S, D, AL);
+    break;
   case ParsedAttr::AT_CodeSeg:
     handleCodeSegAttr(S, D, AL);
     break;

clang/test/Misc/pragma-attribute-supported-attributes-list.test

@@ -80,6 +80,7 @@
 // CHECK-NEXT: NoInstrumentFunction (SubjectMatchRule_function)
 // CHECK-NEXT: NoMicroMips (SubjectMatchRule_function)
 // CHECK-NEXT: NoMips16 (SubjectMatchRule_function)
+// CHECK-NEXT: NoRandomizeLayout (SubjectMatchRule_record)
 // CHECK-NEXT: NoSanitize (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSanitizeSpecific (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: NoSpeculativeLoadHardening (SubjectMatchRule_function, SubjectMatchRule_objc_method)