[clang] a61b202 - [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function
Zarko Todorovski via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 13 12:32:33 PDT 2022
Author: Zarko Todorovski
Date: 2022-07-13T15:32:29-04:00
New Revision: a61b202d4e3b00bf6bfd71dc1ea354d37f73b791
URL: https://github.com/llvm/llvm-project/commit/a61b202d4e3b00bf6bfd71dc1ea354d37f73b791
DIFF: https://github.com/llvm/llvm-project/commit/a61b202d4e3b00bf6bfd71dc1ea354d37f73b791.diff
LOG: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function
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
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ca6390a02ce82..550029f58b546 100644
--- 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 75a2e7eb31d19..e51b9daef7d3e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -13039,6 +13039,8 @@ class Sema final {
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 d3929361213ad..df602f168dbc6 100644
--- 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 de8d795977ec9..66fe7e76546b7 100644
--- 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 869e443c4785e..20d1b837a92f6 100644
--- 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 3d4055a3ae472..f0e8beacda763 100644
--- 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 a9c2ddbf47840..ad87c7295cfe8 100644
--- 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 0fd6af4ee4c13..ef05c97652b63 100644
--- 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 8cd0b335ffbdd..1e94614f371da 100644
--- 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 {
More information about the cfe-commits
mailing list