[PowerPC][AIX] Warn when using pragma align(packed) on AIX.

[sfertile]

Summary

With xlc and xlC pragma align(packed) will pack bitfields the same way as pragma align(bit_packed). xlclang, xlclang++ and clang will pack bitfields the same way as pragma pack(1). Issue a warning when source code using pragma align(packed) to alert the user it may not be compatible with xlc/xlC.

[cebowleratibm]

clang/lib/Sema/SemaAttr.cpp

238 ↗

(On Diff #364261)

Does this diagnostic need to be emitted for any struct or only structs that contain bitfield members? As it is it will be very verbose.

[sfertile]

Only emit diagnostic on bitfield members, which is the only difference in align(packed) behaviour XL and clang/xlclang.

[sfertile]

Good point Chris. The only difference in layout is related to bitfield members, so I have moved the warning to VerifyBitField as suggested.

[cebowleratibm]

It would be nice if the diagnostic could be deferred to layout, in case the struct is defined but not used in a header, but I understand that #pragma pack(1) and #pragma align(packed) become ambiguous at the point of layout. I think this is a reasonable diagnostic given the impact of silently incompatible codegen.

[cebowleratibm]

clang/test/Sema/aix-pragma-align-packed-warn.c

14

It's undesirable to warn for each bitfield member. Perhaps diagnose this in Sema::ActOnTagFinishDefinition?

[sfertile]

Moved the diagnostic emission to Sema::ActOnTagFinishDefinition as suggested.

[sfertile]

In D107506#2931095, @sfertile wrote:

Moved the diagnostic emission to Sema::ActOnTagFinishDefinition as suggested.

Sorry, uploaded wrong diff. Will updated again shortly.

[sfertile]

Fixed diagnostic to only emit when there is a bitfield member.

[sfertile]

clang/test/Sema/aix-pragma-align-packed-warn.c

14

Thanks for the suggestion Chris, I've moved it as suggested.

[sfertile]

Add a couple more struct layouts to the testing to show cases diagnostic is not issued.

[cebowleratibm]

The new version is better than the previous. LGTM.

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_opencl_invalid_return : Error<
   "declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">;
 def warn_enum_value_overflow : Warning<"overflow in enumeration value">;
 def warn_pragma_options_align_reset_failed : Warning<
   "#pragma options align=reset failed: %0">,
   InGroup<IgnoredPragmas>;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++">,
+  InGroup<AIXCompat>;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup<IgnoredPragmas>;
 def err_pragma_pack_invalid_alignment : Error<
   warn_pragma_pack_invalid_alignment.Text>;
 def warn_pragma_pack_non_default_at_include : Warning<
   "non-default #pragma pack value changes the alignment of struct or union "
   "members in the included file">, InGroup<PragmaPackSuspiciousInclude>,

clang/lib/Sema/SemaDecl.cpp

void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 
   if (getCurLexicalContext()->isObjCContainer() &&
       Tag->getDeclContext()->isFileContext())
     Tag->setTopLevelDeclInObjCContainer();
 
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
     Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+      AlignPackStack.hasValue()) {
+    AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+    // Only diagnose #pragma align(packed).
+    if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != AlignPackInfo::Packed)
+      return;
+    const RecordDecl *RD = dyn_cast<RecordDecl>(Tag);
+    if (!RD)
+      return;
+    // Only warn if there is at least 1 bitfield member.
+    if (llvm::any_of(RD->fields(),
+                     [](const FieldDecl *FD) { return FD->isBitField(); }))
+      Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
   // Exit this scope of this interface definition.
   PopDeclContext();
 }
 
 void Sema::ActOnObjCTemporaryExitContainerContext(DeclContext *DC) {

clang/test/Sema/aix-pragma-align-packed-warn.c

+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+#pragma align(reset)