ZarkoCA updated this revision to Diff 411912.
ZarkoCA retitled this revision from "[Clang][Sema][AIX][PowerPC] Emit byval 
alignment warning only when struct member is passed to a function" to 
"[Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is 
passed to a function".
ZarkoCA edited the summary of this revision.
ZarkoCA added a comment.
This revision is now accepted and ready to land.

Reworked implementation so the diagnosis is more correct:

- Warn when the struct is passed that contains the member
- Warn when alignment is 16 and not 16 and greater


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118350/new/

https://reviews.llvm.org/D118350

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Analysis/padding_cpp.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/Sema/aix-attr-align.c
  clang/test/SemaTemplate/instantiate-attr.cpp

Index: clang/test/SemaTemplate/instantiate-attr.cpp
===================================================================
--- clang/test/SemaTemplate/instantiate-attr.cpp
+++ clang/test/SemaTemplate/instantiate-attr.cpp
@@ -1,7 +1,4 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-aix-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 // expected-no-diagnostics
 template <typename T>
 struct A {
Index: clang/test/Sema/aix-attr-align.c
===================================================================
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -6,17 +6,31 @@
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
 struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 16.1.0 or older}}
 };
 
 struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
-};
-
-struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+  int a[8] __attribute__((aligned(8))); // no-warning
+  int b[8] __attribute__((aligned(4))); // no-warning
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
 int b[4] __attribute__((aligned(16))); // no-warning
-int c[2] __attribute__((aligned(32))); // no-warning
+
+void baz(int a, int b, int *c, int d, int *e, int f, struct S);
+void jaz(int a, int b, int *c, int d, int *e, int f, struct T);
+void vararg_baz(int a,...);
+static void static_baz(int a, int b, int *c, int d, int *e, int f, struct S sp2) {
+  a = *sp2.b + *c + *e;
+}
+
+void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8,
+         struct S s, struct T t) {
+
+  baz(p1, p2, s.b, p3, b, p5, s);        // expected-note {{'b' used with potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);        // no-note
+  jaz(p1, p2, s.b, p3, b, p5, t);        // no-note
+  vararg_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+  static_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+}
Index: clang/test/CXX/drs/dr6xx.cpp
===================================================================
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1,10 +1,8 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
+// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
 
 namespace std {
   struct type_info {};
Index: clang/test/Analysis/padding_cpp.cpp
===================================================================
--- clang/test/Analysis/padding_cpp.cpp
+++ clang/test/Analysis/padding_cpp.cpp
@@ -1,6 +1,4 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify -Wno-aix-compat %s
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
 
 // Make sure that the C cases still work fine, even when compiled as C++.
 #include "padding_c.c"
Index: clang/test/Analysis/padding_c.c
===================================================================
--- clang/test/Analysis/padding_c.c
+++ clang/test/Analysis/padding_c.c
@@ -1,10 +1,8 @@
-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_analyze_cc1 -verify -Wno-aix-compat %s \
+// RUN: %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=optin.performance \
 // RUN:   -analyzer-config optin.performance.Padding:AllowedPad=2
 
-// RUN: not %clang_analyze_cc1 -verify -Wno-aix-compat %s \
+// RUN: not %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=optin.performance.Padding \
 // RUN:   -analyzer-config optin.performance.Padding:AllowedPad=-10 \
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4345,13 +4345,6 @@
     return;
 
   uint64_t AlignVal = Alignment.getZExtValue();
-  // 16 byte ByVal alignment not due to a vector member is not honoured by XL
-  // on AIX. Emit a warning here that users are generating binary incompatible
-  // code to be safe.
-  if (AlignVal >= 16 && isa<FieldDecl>(D) &&
-      Context.getTargetInfo().getTriple().isOSAIX())
-    Diag(AttrLoc, diag::warn_not_xl_compatible) << E->getSourceRange();
-
   // C++11 [dcl.align]p2:
   //   -- if the constant expression evaluates to zero, the alignment
   //      specifier shall have no effect
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5235,6 +5235,36 @@
   }
 }
 
+// 16 byte ByVal alignment not due to a vector member is not honoured by XL
+// on AIX. Emit a warning here that users are generating binary incompatible
+// code to be safe.
+// Here we try to get information about the alignment of the struct member
+// from the struct passed to the caller function.
+void Sema::checkAIXMemberAlignment(SourceLocation Loc, NamedDecl *FDecl,
+                                   StringRef ParamName, const Expr *Arg) {
+
+  if (const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens())) {
+    if (const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr())) {
+      if (auto *PD = dyn_cast<ParmVarDecl>(DR->getDecl())) {
+        if (PD->getType()->isRecordType()) {
+          QualType ArgType = Arg->getType();
+          for (const FieldDecl *FD :
+               ArgType->castAs<RecordType>()->getDecl()->fields()) {
+            if (const auto *AA = FD->getAttr<AlignedAttr>()) {
+              CharUnits Alignment =
+                  Context.toCharUnitsFromBits(AA->getAlignment(Context));
+              if (Alignment.getQuantity() == 16) {
+                Diag(FD->getLocation(), diag::warn_not_xl_compatible) << FD;
+                Diag(Loc, diag::note_misaligned_member_used_here) << FD;
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
+
 /// Warn if a pointer or reference argument passed to a function points to an
 /// object that is less aligned than the parameter. This can happen when
 /// creating a typedef with a lower alignment than the original type and then
@@ -5347,6 +5377,13 @@
 
         QualType ParamTy = Proto->getParamType(ArgIdx);
         QualType ArgTy = Arg->getType();
+        if (Context.getTargetInfo().getTriple().isOSAIX() && Arg &&
+            FDecl->hasLinkage() &&
+            FDecl->getFormalLinkage() != InternalLinkage &&
+            CallType == VariadicDoesNotApply)
+          checkAIXMemberAlignment((Arg->getExprLoc()), FDecl,
+                                  std::to_string(ArgIdx + 1), Arg);
+
         CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
                           ArgTy, ParamTy);
       }
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12731,6 +12731,9 @@
                             ArrayRef<const Expr *> Args,
                             const FunctionProtoType *Proto, SourceLocation Loc);
 
+  void checkAIXMemberAlignment(SourceLocation Loc, NamedDecl *FDecl,
+                               StringRef ParamName, const Expr *Arg);
+
   void CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
                          StringRef ParamName, QualType ArgTy, QualType ParamTy);
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3296,10 +3296,11 @@
               "alignment assumed">,
       InGroup<DiagGroup<"builtin-assume-aligned-alignment">>;
 def warn_not_xl_compatible
-    : Warning<"requesting an alignment of 16 bytes or greater for struct"
-              " members is not binary compatible with IBM XL C/C++ for AIX"
-              " 16.1.0 and older">,
+    : Warning<"alignment of 16 bytes for a struct member is not binary "
+              "compatible with IBM XL C/C++ for AIX 16.1.0 or older">,
       InGroup<AIXCompat>;
+def note_misaligned_member_used_here : Note<
+    "%0 used with potentially incompatible alignment here">;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,
   InGroup<MicrosoftInconsistentDllImport>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to