[Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed...

Previous warning went on whenever a struct with a struct member with alignment => 16
was declared. This led to too many false positives and led to diagnostic lit failures
due to it being emitted too frequently. Only emit the warning when such a struct and
that struct contains a member that has an alignment of 16 bytes is passed to a caller
function since this is where the potential binary compatibility issue with XL 16.1.0
and older exists.

Reviewed By: sfertile, aaron.ballman

Differential Revision: https://reviews.llvm.org/D118350

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ca6390a02ce82e0b8de380721f9af900c517103b..550029f58b546a8c8a5cc7164bc8c729ab69a0df 100644 (file)
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3341,10 +3341,11 @@ def warn_assume_aligned_too_great
               "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<
+    "passing byval argument %0 with potentially incompatible alignment here">;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,
   InGroup<MicrosoftInconsistentDllImport>;

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 75a2e7eb31d19c4136da2c389b4e7b4eee049634..e51b9daef7d3eb0166f8ccc7abdaf0b554130442 100644 (file)
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13039,6 +13039,8 @@ private:
                             ArrayRef<const Expr *> Args,
                             const FunctionProtoType *Proto, SourceLocation Loc);
 
+  void checkAIXMemberAlignment(SourceLocation Loc, const Expr *Arg);
+
   void CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
                          StringRef ParamName, QualType ArgTy, QualType ParamTy);
 

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d3929361213ad9d8f148cef7f67dcbdf4e2a8d63..df602f168dbc6145becb36839356db31071bf759 100644 (file)
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5626,6 +5626,40 @@ static void CheckNonNullArguments(Sema &S,
   }
 }
 
+// 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. We only warn when the struct
+// is passed byval, hence the series of checks and early returns if we are a not
+// passing a struct byval.
+void Sema::checkAIXMemberAlignment(SourceLocation Loc, const Expr *Arg) {
+  const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens());
+  if (!ICE)
+    return;
+
+  const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr());
+  if (!DR)
+    return;
+
+  const auto *PD = dyn_cast<ParmVarDecl>(DR->getDecl());
+  if (!PD || !PD->getType()->isRecordType())
+    return;
+
+  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) << PD;
+      }
+    }
+  }
+}
+
 /// 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
@@ -5736,6 +5770,12 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
         if (Arg->containsErrors())
           continue;
 
+        if (Context.getTargetInfo().getTriple().isOSAIX() && FDecl && Arg &&
+            FDecl->hasLinkage() &&
+            FDecl->getFormalLinkage() != InternalLinkage &&
+            CallType == VariadicDoesNotApply)
+          checkAIXMemberAlignment((Arg->getExprLoc()), Arg);
+
         QualType ParamTy = Proto->getParamType(ArgIdx);
         QualType ArgTy = Arg->getType();
         CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index de8d795977ec99b02bd83c58fb8fca45787ea9d2..66fe7e76546b71a97ace893f55cfbbac158d2106 100644 (file)
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -4312,13 +4312,6 @@ void Sema::AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
     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

diff --git a/clang/test/Analysis/padding_c.c b/clang/test/Analysis/padding_c.c
index 869e443c4785e1f90e174a4563f6ecf24edb5bec..20d1b837a92f673a4712362008fc739e6b747cec 100644 (file)
--- a/clang/test/Analysis/padding_c.c
+++ b/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 \

diff --git a/clang/test/Analysis/padding_cpp.cpp b/clang/test/Analysis/padding_cpp.cpp
index 3d4055a3ae472dbac3ad35fbd024e4270348b122..f0e8beacda76305afd516cb183180bfb75de0459 100644 (file)
--- a/clang/test/Analysis/padding_cpp.cpp
+++ b/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"

diff --git a/clang/test/CXX/drs/dr6xx.cpp b/clang/test/CXX/drs/dr6xx.cpp
index a9c2ddbf47840bde199843a29161fb4881d6c046..ad87c7295cfe878d25adf3a5d50f4b493efe2aad 100644 (file)
--- a/clang/test/CXX/drs/dr6xx.cpp
+++ b/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 {};

diff --git a/clang/test/Sema/aix-attr-align.c b/clang/test/Sema/aix-attr-align.c
index 0fd6af4ee4c13d5e401190479edff0571534efa8..ef05c97652b63a8cd10ab5e3f5aa4c7683a6c60c 100644 (file)
--- a/clang/test/Sema/aix-attr-align.c
+++ b/clang/test/Sema/aix-attr-align.c
@@ -5,18 +5,37 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify=off -Wno-aix-compat -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
-struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+// We do not warn on any declaration with a member aligned 16. Only when the struct is passed byval.
+struct R {
+  int b[8] __attribute__((aligned(16))); // no-warning
 };
 
-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 S {
+  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 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}}
+struct T {
+  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 {{passing byval argument 's' 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
+}

diff --git a/clang/test/SemaTemplate/instantiate-attr.cpp b/clang/test/SemaTemplate/instantiate-attr.cpp
index 8cd0b335ffbddcc0e5c0ac71a6286b839bf4c584..1e94614f371daa9415f99e37dafbe579185df215 100644 (file)
--- a/clang/test/SemaTemplate/instantiate-attr.cpp
+++ b/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 {