[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 5 18:05:58 PDT 2023


https://github.com/kasuga-fj updated https://github.com/llvm/llvm-project/pull/65246

>From 01ca65833b2248547f1fb4f7d739517e619ac4cd Mon Sep 17 00:00:00 2001
From: "kasuga.ryotaro" <kasuga.ryotaro at fujitsu.com>
Date: Wed, 30 Aug 2023 13:26:31 +0900
Subject: [PATCH] [Clang] Fix missing diagnostic for non-standard layout type
 in `offsetof`

Fixes #64619

Clang warns diagnostic for non-standard layout types in `offsetof` only
if they are in evaluated context. With this patch, you'll also get
diagnostic if you use `offsetof` on non-standard layout types in any
other contexts
---
 clang/docs/ReleaseNotes.rst                   |  2 +
 clang/lib/Sema/SemaExpr.cpp                   |  9 ++-
 clang/test/SemaCXX/class-layout.cpp           | 62 +++++++++----------
 clang/test/SemaCXX/ms_struct.cpp              |  6 +-
 clang/test/SemaCXX/offsetof.cpp               |  9 +--
 libcxx/include/__type_traits/datasizeof.h     |  5 ++
 .../non_trivial_copy_move_ABI.pass.cpp        |  2 +-
 .../pairs.pair/trivial_copy_move_ABI.pass.cpp |  1 +
 8 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ae67a783c2fa2d5..eef9a119bcb41a0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -270,6 +270,8 @@ Improvements to Clang's diagnostics
     source:1:24: note: expression evaluates to ''\n' (0x0A, 10) == U'🌍' (0x1F30D, 127757)'
     1 | static_assert("A\n"[1] == U'🌍');
       |               ~~~~~~~~~^~~~~~~~
+- Clang now always diagnoses when using non-standard layout types in ``offsetof`` .
+  (`#64619: <https://github.com/llvm/llvm-project/issues/64619>`_)
 
 Bug Fixes in This Version
 -------------------------
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 9c5f96eebd04165..15e2a7978f6b723 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -16796,12 +16796,11 @@ ExprResult Sema::BuildBuiltinOffsetOf(SourceLocation BuiltinLoc,
         LangOpts.CPlusPlus11? diag::ext_offsetof_non_standardlayout_type
                             : diag::ext_offsetof_non_pod_type;
 
-      if (!IsSafe && !DidWarnAboutNonPOD &&
-          DiagRuntimeBehavior(BuiltinLoc, nullptr,
-                              PDiag(DiagID)
-                              << SourceRange(Components[0].LocStart, OC.LocEnd)
-                              << CurrentType))
+      if (!IsSafe && !DidWarnAboutNonPOD && !isUnevaluatedContext()) {
+        Diag(BuiltinLoc, DiagID)
+            << SourceRange(Components[0].LocStart, OC.LocEnd) << CurrentType;
         DidWarnAboutNonPOD = true;
+      }
     }
 
     // Look for the field.
diff --git a/clang/test/SemaCXX/class-layout.cpp b/clang/test/SemaCXX/class-layout.cpp
index 9782ff08100b2d6..22fb34b8419c583 100644
--- a/clang/test/SemaCXX/class-layout.cpp
+++ b/clang/test/SemaCXX/class-layout.cpp
@@ -1,34 +1,34 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base -Wno-c++11-extensions
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
-// RUN: %clang_cc1 -triple x86_64-apple-darwin    %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple x86_64-scei-ps4        %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -triple x86_64-sie-ps5         %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16
-// RUN: %clang_cc1 -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
-// RUN: %clang_cc1 -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base -Wno-c++11-extensions
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-apple-darwin    %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-scei-ps4        %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-sie-ps5         %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base -Wno-invalid-offsetof -Wno-c++11-extensions
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof
+// RUN: %clang_cc1 -triple x86_64-apple-darwin    %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple x86_64-scei-ps4        %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-sie-ps5         %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
+// RUN: %clang_cc1 -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof
+// RUN: %clang_cc1 -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15
+
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base -Wno-c++11-extensions -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-apple-darwin    %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-scei-ps4        %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-sie-ps5         %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=14 -DCLANG_ABI_COMPAT=14 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -Wno-invalid-offsetof
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -triple s390x-none-zos %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 -Wno-invalid-offsetof
 
 
 
diff --git a/clang/test/SemaCXX/ms_struct.cpp b/clang/test/SemaCXX/ms_struct.cpp
index e6f0a25b38ea80c..995e424d1f87640 100644
--- a/clang/test/SemaCXX/ms_struct.cpp
+++ b/clang/test/SemaCXX/ms_struct.cpp
@@ -25,6 +25,7 @@ struct B : public A {
 
 static_assert(__builtin_offsetof(B, d) == 12,
   "We can't allocate the bitfield into the padding under ms_struct");
+// expected-warning at -2 {{offset of on non-standard-layout type 'B'}}
 
 struct C {
 #ifdef TEST_FOR_ERROR
@@ -38,6 +39,5 @@ struct C {
 
 static_assert(__builtin_offsetof(C, n) == 8,
               "long long field in ms_struct should be 8-byte aligned");
-#if !defined(TEST_FOR_ERROR) && !defined(TEST_FOR_WARNING)
-// expected-no-diagnostics
-#endif
+// expected-warning at -2 {{offset of on non-standard-layout type 'C'}}
+
diff --git a/clang/test/SemaCXX/offsetof.cpp b/clang/test/SemaCXX/offsetof.cpp
index c984657ebe1f0ed..c4e86369f007dcb 100644
--- a/clang/test/SemaCXX/offsetof.cpp
+++ b/clang/test/SemaCXX/offsetof.cpp
@@ -51,9 +51,9 @@ struct Derived2 : public Base1, public Base2 {
   int z; 
 };
 
-int derived1[__builtin_offsetof(Derived2, x) == 0? 1 : -1];
-int derived2[__builtin_offsetof(Derived2, y)  == 4? 1 : -1];
-int derived3[__builtin_offsetof(Derived2, z)  == 8? 1 : -1];
+int derived1[__builtin_offsetof(Derived2, x) == 0? 1 : -1]; // expected-warning{{offset of on non-POD type 'Derived2'}}
+int derived2[__builtin_offsetof(Derived2, y)  == 4? 1 : -1]; // expected-warning{{offset of on non-POD type 'Derived2'}}
+int derived3[__builtin_offsetof(Derived2, z)  == 8? 1 : -1]; // expected-warning{{offset of on non-POD type 'Derived2'}}
 
 // offsetof referring to anonymous struct in base.
 // PR7769
@@ -66,7 +66,8 @@ struct foo {
 struct bar : public foo  {
 };
 
-int anonstruct[__builtin_offsetof(bar, x) == 0 ? 1 : -1];
+int anonstruct[__builtin_offsetof(bar, x) == 0 ? 1 : -1]; // expected-warning{{offset of on non-POD type 'bar'}}
+
 
 struct LtoRCheck {
   int a[10];
diff --git a/libcxx/include/__type_traits/datasizeof.h b/libcxx/include/__type_traits/datasizeof.h
index 019099a9cf183b5..5688e3293a69eb5 100644
--- a/libcxx/include/__type_traits/datasizeof.h
+++ b/libcxx/include/__type_traits/datasizeof.h
@@ -47,7 +47,12 @@ struct __libcpp_datasizeof {
   };
 #endif
 
+  // _FirstPaddingByte<> is sometimes non-standard layout. Using `offsetof` is UB in that case, but GCC and Clang allow
+  // the use as an extension.
+  _LIBCPP_DIAGNOSTIC_PUSH
+  _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Winvalid-offsetof")
   static const size_t value = offsetof(_FirstPaddingByte<>, __first_padding_byte_);
+  _LIBCPP_DIAGNOSTIC_POP
 };
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp
index 7ba91f82bc3bbce..1f5dae1232e37b4 100644
--- a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/non_trivial_copy_move_ABI.pass.cpp
@@ -17,7 +17,7 @@
 // Test that we provide the non-trivial copy operations when _LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR
 // is specified.
 // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEPRECATED_ABI_DISABLE_PAIR_TRIVIAL_COPY_CTOR
-// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined
+// ADDITIONAL_COMPILE_FLAGS: -Wno-macro-redefined -Wno-invalid-offsetof
 
 #include <utility>
 #include <type_traits>
diff --git a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp
index 013b79b1425a217..fe098a7af7e8fc9 100644
--- a/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp
+++ b/libcxx/test/libcxx/utilities/utility/pairs/pairs.pair/trivial_copy_move_ABI.pass.cpp
@@ -14,6 +14,7 @@
 
 // FreeBSD still provides the old ABI for std::pair.
 // XFAIL: freebsd
+// ADDITIONAL_COMPILE_FLAGS: -Wno-invalid-offsetof
 
 #include <utility>
 #include <type_traits>



More information about the cfe-commits mailing list