mubashar_ created this revision.
mubashar_ added a reviewer: lenary.
Herald added a subscriber: kristof.beyls.
mubashar_ requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A warning for when potential cases of
unaligned access with option -mno-unaligned-access 
enabled has been added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116221

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Sema/test-wunaligned-access.cpp

Index: clang/test/Sema/test-wunaligned-access.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/test-wunaligned-access.cpp
@@ -0,0 +1,298 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -Wunaligned-access
+
+// Packed-Unpacked Tests (No Pragma)
+
+struct T1
+{
+    char a;
+    int b;
+};
+
+struct __attribute__((packed)) U1 // Causes warning
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'U1' has an alignment greater than its parent this may be caused by 'U1' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct __attribute__((packed)) U2 // No warning
+{
+    char a;
+    T1 b __attribute__((aligned(4)));
+    int c;
+};
+
+struct __attribute__((packed)) U3 // No warning
+{
+    char a;
+    char b;
+    short c;
+    T1 d;
+};
+
+struct __attribute__((packed)) U4 // No warning
+{
+    T1 a;
+    int b;
+};
+
+struct __attribute__((aligned(4), packed)) U5 // Causes warning
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'U5' has an alignment greater than its parent this may be caused by 'U5' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct __attribute__((aligned(4), packed)) U6 // No warning
+{
+    char a;
+    char b;
+    short c;
+    T1 d;
+};
+
+// Packed-Unpacked Tests with Pragma
+
+#pragma pack(push, 1)
+
+struct __attribute__((packed)) U7 // Causes warning
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'U7' has an alignment greater than its parent this may be caused by 'U7' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct __attribute__((packed)) U8
+{
+    char a;
+    T1 b __attribute__((aligned(4))); // expected-warning {{field b within its parent 'U8' has an alignment greater than its parent this may be caused by 'U8' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct __attribute__((aligned(4))) U9
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'U9' has an alignment greater than its parent this may be caused by 'U9' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct U10
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'U10' has an alignment greater than its parent this may be caused by 'U10' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+#pragma pack(pop)
+
+// Packed-Packed Tests
+
+struct __attribute__((packed)) T2
+{
+    char a;
+    int b;
+};
+
+struct __attribute__((packed)) U11
+{
+    char a;
+    T2 b;
+    int c;
+};
+
+#pragma pack(push, 1)
+struct U12 // No warning
+{
+    char a;
+    T2 b;
+    int c;
+};
+#pragma pack(pop)
+
+// Unpacked-Packed Tests
+
+struct U13 // No warning
+{
+    char a;
+    T2 b;
+    int c;
+};
+
+struct U14 // No warning
+{
+    char a;
+    T2 b __attribute__((aligned(4)));
+    int c;
+};
+
+// Unpacked-Unpacked Test
+
+struct T3
+{
+    char a;
+    int b;
+};
+
+struct U15 // No warning
+{
+    char a;
+    T3 b;
+    int c;
+};
+
+// Packed-Packed-Unpacked Test (No pragma)
+
+struct __attribute__((packed)) A1
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'A1' has an alignment greater than its parent this may be caused by 'A1' being packed and can lead to unaligned accesses}}
+};
+
+struct __attribute__((packed)) U16 // No warning
+{
+    char a;
+    A1 b;
+    int c;
+};
+
+struct __attribute__((packed)) A2 // No warning
+{
+    char a;
+    T1 b __attribute__((aligned(4)));
+};
+
+struct __attribute__((packed)) U17 // Caused warning
+{
+    char a;
+    A2 b; // expected-warning {{field b within its parent 'U17' has an alignment greater than its parent this may be caused by 'U17' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+// Packed-Unpacked-Packed tests
+
+struct A3
+{
+    char a;
+    T2 b;
+};
+
+struct __attribute__((packed)) U18
+{
+    char a;
+    A3 b;
+    int c;
+};
+
+struct A4
+{
+    char a;
+    T2 b;
+    int c;
+};
+
+#pragma pack(push, 1)
+struct U19 // Caused warning
+{
+    char a;
+    A4 b; // expected-warning {{field b within its parent 'U19' has an alignment greater than its parent this may be caused by 'U19' being packed and can lead to unaligned accesses}}
+    int c;
+};
+#pragma pack(pop)
+
+// Packed-Unpacked-Unpacked tests
+
+struct A5
+{
+    char a;
+    T1 b;
+};
+
+struct __attribute__((packed)) U20 // Caused warning
+{
+    char a;
+    A5 b; // expected-warning {{field b within its parent 'U20' has an alignment greater than its parent this may be caused by 'U20' being packed and can lead to unaligned accesses}}
+    int c;
+};
+
+struct A6
+{
+    char a;
+    T1 b;
+};
+
+#pragma pack(push, 1)
+struct U21 // Caused warning
+{
+    char a;
+    A6 b; // expected-warning {{field b within its parent 'U21' has an alignment greater than its parent this may be caused by 'U21' being packed and can lead to unaligned accesses}}
+    int c;
+};
+#pragma pack(pop)
+
+// Unpacked-Packed-Packed test
+
+struct __attribute__((packed)) A7 // No warning
+{
+    char a;
+    T2 b;
+};
+
+struct U22 // No warning
+{
+    char a;
+    A7 b;
+    int c;
+};
+
+// Unpacked-Packed-Unpacked tests
+
+struct __attribute__((packed)) A8 // Should cause warning
+{
+    char a;
+    T1 b; // expected-warning {{field b within its parent 'A8' has an alignment greater than its parent this may be caused by 'A8' being packed and can lead to unaligned accesses}}
+};
+
+struct U23 // No warning
+{
+    char a;
+    A8 b;
+    int c;
+};
+
+struct __attribute__((packed)) A9 // No warning
+{
+    char a;
+    T1 b __attribute__((aligned(4)));
+};
+
+struct U24 // No warning
+{
+    char a;
+    A9 b;
+    int c;
+};
+
+struct U1 s1;
+struct U2 s2;
+struct U3 s3;
+struct U4 s4;
+struct U5 s5;
+struct U6 s6;
+struct U7 s7;
+struct U8 s8;
+struct U9 s9;
+struct U10 s10;
+struct U11 s11;
+struct U12 s12;
+struct U13 s13;
+struct U14 s14;
+struct U15 s15;
+struct U16 s16;
+struct U17 s17;
+struct U18 s18;
+struct U19 s19;
+struct U20 s20;
+struct U21 s21;
+struct U22 s22;
+struct U23 s23;
+struct U24 s24;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -346,7 +346,7 @@
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-    aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
+    aarch64::getAArch64TargetFeatures(D, Triple, Args, CmdArgs, Features, ForAS);
     break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -783,6 +783,7 @@
         D.Diag(diag::err_target_unsupported_unaligned) << "v8m.base";
     } else
       Features.push_back("+strict-align");
+      CmdArgs.push_back("-Wunaligned-access");
   } else {
     // Assume pre-ARMv6 doesn't support unaligned accesses.
     //
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===================================================================
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -22,6 +22,7 @@
 
 void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
                               const llvm::opt::ArgList &Args,
+                              llvm::opt::ArgStringList &CmdArgs,
                               std::vector<llvm::StringRef> &Features,
                               bool ForAS);
 
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -219,6 +219,7 @@
 void aarch64::getAArch64TargetFeatures(const Driver &D,
                                        const llvm::Triple &Triple,
                                        const ArgList &Args,
+                                       llvm::opt::ArgStringList &CmdArgs,
                                        std::vector<StringRef> &Features,
                                        bool ForAS) {
   Arg *A;
@@ -460,7 +461,10 @@
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
                                options::OPT_munaligned_access)) {
     if (A->getOption().matches(options::OPT_mno_unaligned_access))
+    {
       Features.push_back("+strict-align");
+      CmdArgs.push_back("-Wunaligned-access");
+    }
   } else if (Triple.isOSOpenBSD())
     Features.push_back("+strict-align");
 
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2021,6 +2021,7 @@
   CharUnits UnpackedFieldAlign =
       !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
   CharUnits UnpackedFieldOffset = FieldOffset;
+  CharUnits OriginalFieldAlign = UnpackedFieldAlign;
 
   if (FieldPacked) {
     FieldAlign = CharUnits::One();
@@ -2105,6 +2106,25 @@
   // Remember max struct/class ABI-specified alignment.
   UnadjustedAlignment = std::max(UnadjustedAlignment, FieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign, PreferredAlign);
+
+  // For checking the alignment of inner fields against
+  // the alignment of its parent record.
+  if (const RecordDecl* RD = D->getParent())
+  {
+    // Check if packed attribute or pragma pack is present.
+    if (RD->hasAttr<PackedAttr>() || !MaxFieldAlignment.isZero())
+      if (FieldAlign < OriginalFieldAlign)
+        if (D->getType()->isRecordType())
+        {
+          // If the offset is a multiple of the alignment of
+          // the type, raise the warning.
+          // TODO: Takes no account the alignment of the outer struct
+          if (FieldOffset % OriginalFieldAlign != 0)
+            Diag(D->getLocation(), diag::warn_unaligned_access)
+              << Context.getTypeDeclType(RD)
+              << D->getName();
+        }
+  }
 }
 
 void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -542,6 +542,7 @@
 def OrderedCompareFunctionPointers : DiagGroup<"ordered-compare-function-pointers">;
 def Packed : DiagGroup<"packed">;
 def Padded : DiagGroup<"padded">;
+def UnalignedAccess : DiagGroup<"unaligned-access">;
 
 def PessimizingMove : DiagGroup<"pessimizing-move">;
 def ReturnStdMove : DiagGroup<"return-std-move">;
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -590,4 +590,9 @@
   InGroup<Padded>, DefaultIgnore;
 def warn_unnecessary_packed : Warning<
   "packed attribute is unnecessary for %0">, InGroup<Packed>, DefaultIgnore;
+
+// -Wunaligned-access
+def warn_unaligned_access : Warning<
+  "field %1 within its parent %0 has an alignment greater than its parent "
+  "this may be caused by %0 being packed and can lead to unaligned accesses">, InGroup<UnalignedAccess>, DefaultIgnore;
 }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to