[llvm-branch-commits] [cfe-branch] r333571 - Merging r326476:

[Tom Stellard via llvm-branch-commits]

Author: tstellar
Date: Wed May 30 11:19:45 2018
New Revision: 333571

URL: http://llvm.org/viewvc/llvm-project?rev=333571&view=rev
Log:
Merging r326476:

------------------------------------------------------------------------
r326476 | mstorsjo | 2018-03-01 12:22:57 -0800 (Thu, 01 Mar 2018) | 7 lines

[RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

Make types with sizes that aren't a power of two an error (that can
be disabled) in structs with ms_struct layout, except on mingw where
the situation is quite likely to occur and GCC handles it silently.

Differential Revision: https://reviews.llvm.org/D43908
------------------------------------------------------------------------

Added:
    cfe/branches/release_60/test/CodeGen/ms_struct-long-double.c
Modified:
    cfe/branches/release_60/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/branches/release_60/lib/AST/RecordLayoutBuilder.cpp

Modified: cfe/branches/release_60/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_60/include/clang/Basic/DiagnosticSemaKinds.td?rev=333571&r1=333570&r2=333571&view=diff
==============================================================================
--- cfe/branches/release_60/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/branches/release_60/include/clang/Basic/DiagnosticSemaKinds.td Wed May 30 11:19:45 2018
@@ -759,6 +759,10 @@ def warn_cxx_ms_struct :
   Warning<"ms_struct may not produce Microsoft-compatible layouts for classes "
           "with base classes or virtual functions">,
   DefaultError, InGroup<IncompatibleMSStruct>;
+def warn_npot_ms_struct :
+  Warning<"ms_struct may not produce Microsoft-compatible layouts with fundamental "
+          "data types with sizes that aren't a power of two">,
+  DefaultError, InGroup<IncompatibleMSStruct>;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
 def err_no_base_classes : Error<"invalid use of '__super', %0 has no base classes">;
 def err_invalid_super_scope : Error<"invalid use of '__super', "

Modified: cfe/branches/release_60/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_60/lib/AST/RecordLayoutBuilder.cpp?rev=333571&r1=333570&r2=333571&view=diff
==============================================================================
--- cfe/branches/release_60/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/branches/release_60/lib/AST/RecordLayoutBuilder.cpp Wed May 30 11:19:45 2018
@@ -1751,10 +1751,32 @@ void ItaniumRecordLayoutBuilder::LayoutF
       QualType T = Context.getBaseElementType(D->getType());
       if (const BuiltinType *BTy = T->getAs<BuiltinType>()) {
         CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
-        assert(
-            (llvm::isPowerOf2_64(TypeSize.getQuantity()) ||
-             Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
-            "Non PowerOf2 size outside of GNU mode");
+
+        if (!llvm::isPowerOf2_64(TypeSize.getQuantity())) {
+          assert(
+              !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment() &&
+              "Non PowerOf2 size in MSVC mode");
+          // Base types with sizes that aren't a power of two don't work
+          // with the layout rules for MS structs. This isn't an issue in
+          // MSVC itself since there are no such base data types there.
+          // On e.g. x86_32 mingw and linux, long double is 12 bytes though.
+          // Any structs involving that data type obviously can't be ABI
+          // compatible with MSVC regardless of how it is laid out.
+
+          // Since ms_struct can be mass enabled (via a pragma or via the
+          // -mms-bitfields command line parameter), this can trigger for
+          // structs that don't actually need MSVC compatibility, so we
+          // need to be able to sidestep the ms_struct layout for these types.
+
+          // Since the combination of -mms-bitfields together with structs
+          // like max_align_t (which contains a long double) for mingw is
+          // quite comon (and GCC handles it silently), just handle it
+          // silently there. For other targets that have ms_struct enabled
+          // (most probably via a pragma or attribute), trigger a diagnostic
+          // that defaults to an error.
+          if (!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+            Diag(D->getLocation(), diag::warn_npot_ms_struct);
+        }
         if (TypeSize > FieldAlign &&
             llvm::isPowerOf2_64(TypeSize.getQuantity()))
           FieldAlign = TypeSize;

Added: cfe/branches/release_60/test/CodeGen/ms_struct-long-double.c
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_60/test/CodeGen/ms_struct-long-double.c?rev=333571&view=auto
==============================================================================
--- cfe/branches/release_60/test/CodeGen/ms_struct-long-double.c (added)
+++ cfe/branches/release_60/test/CodeGen/ms_struct-long-double.c Wed May 30 11:19:45 2018
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-windows-gnu -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts -Wno-incompatible-ms-struct %s | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts %s 2>&1 | FileCheck %s -check-prefix=ERROR
+
+struct ldb_struct {
+  char c;
+  long double ldb;
+} __attribute__((__ms_struct__));
+
+struct ldb_struct a;
+
+// CHECK:             0 | struct ldb_struct
+// CHECK-NEXT:        0 |   char c
+// CHECK-NEXT:        4 |   long double ldb
+// CHECK-NEXT:          | [sizeof=16, align=4]
+
+// ERROR: error: ms_struct may not produce Microsoft-compatible layouts with fundamental data types with sizes that aren't a power of two