On 5/6/25 3:05 PM, Simon Martin wrote:
The following test case highlights two issues - see
https://godbolt.org/z/7E1KGYreh:
  1. We error out at both L4 and L5, while (at least) clang, EDG and MSVC
     only reject L5
  2. Our error message for L5 incorrectly mentions using a null pointer

=== cut here ===
struct A { int i; };
struct C : virtual public A { };
void foo () {
   auto res = ((C*)0)->i;      // L4

I assume you meant to include a & in there.

   __builtin_offsetof (C, i);  // L5
}
=== cut here ===

I am not aware of anything in the standard that'd make L4 invalid,

I think it's undefined under https://eel.is/c++draft/expr#ref-9 and/or https://eel.is/c++draft/expr#unary.op-1

There's also no reasonable answer, because the offset of a virtual base depends on the most-derived type of the object. This is a bad question to ask, which is why it's ill-formed for offsetof. I wonder why the OP wants to write this code?

But I guess pedantically we shouldn't error out on code with undefined behavior, and the diagnostic for L4 should only be a warning.

and this patch aligns our behaviour with that of other compilers by
removing the check for a null instance (that was primarily targetting
offsetof according to its comment) from build_class_member_access_expr,
and having finish_offsetof check and report accesses to members from a
virtual base class.

Successfully tested on x86_64-pc-linux-gnu.

        PR c++/118346

gcc/cp/ChangeLog:

        * semantics.cc (finish_offsetof): Reject accesses to a virtual
        base member.
        * typeck.cc (build_class_member_access_expr): Don't reject
        virtual base member access of null objects.

gcc/testsuite/ChangeLog:

        * g++.dg/other/offsetof8.C: Add untested scenarios.
        * g++.dg/other/offsetof9.C: Adjust test expectations.

---
  gcc/cp/semantics.cc                    | 28 ++++++++++++++++++------
  gcc/cp/typeck.cc                       | 20 -----------------
  gcc/testsuite/g++.dg/other/offsetof8.C | 30 +++++++++++++++++++++-----
  gcc/testsuite/g++.dg/other/offsetof9.C |  6 +++---
  4 files changed, 50 insertions(+), 34 deletions(-)

diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 43a0eabfa12..9ba00196542 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -5324,13 +5324,29 @@ finish_offsetof (tree object_ptr, tree expr, location_t 
loc)
      expr = TREE_OPERAND (expr, 0);
    if (!complete_type_or_else (TREE_TYPE (TREE_TYPE (object_ptr)), object_ptr))
      return error_mark_node;
+
    if (warn_invalid_offsetof
-      && CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (object_ptr)))
-      && CLASSTYPE_NON_STD_LAYOUT (TREE_TYPE (TREE_TYPE (object_ptr)))
-      && cp_unevaluated_operand == 0)
-    warning_at (loc, OPT_Winvalid_offsetof, "%<offsetof%> within "
-               "non-standard-layout type %qT is conditionally-supported",
-               TREE_TYPE (TREE_TYPE (object_ptr)));
+      && CLASS_TYPE_P (TREE_TYPE (TREE_TYPE (object_ptr))))
+    {
+      bool member_in_base_p = false;
+      if (TREE_CODE (expr) == COMPONENT_REF)
+       {
+         tree member = expr;
+         do {
+             member = TREE_OPERAND (member, 1);
+         } while (TREE_CODE (member) == COMPONENT_REF);
+         member_in_base_p = !same_type_p (TREE_TYPE (TREE_TYPE (object_ptr)),
+                                          DECL_CONTEXT (member));
+       }
+      if (member_in_base_p
+         && CLASSTYPE_VBASECLASSES (TREE_TYPE (TREE_TYPE (object_ptr))))
+       error_at (loc, "invalid %<offsetof%> to data member in virtual base");
+      else if (CLASSTYPE_NON_STD_LAYOUT (TREE_TYPE (TREE_TYPE (object_ptr)))
+              && cp_unevaluated_operand == 0)
+       warning_at (loc, OPT_Winvalid_offsetof, "%<offsetof%> within "
+                   "non-standard-layout type %qT is conditionally-supported",
+                   TREE_TYPE (TREE_TYPE (object_ptr)));
+    }
    return fold_offsetof (expr);
  }
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 1b9fdf5b21d..4e5d0507493 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -2926,16 +2926,9 @@ build_class_member_access_expr (cp_expr object, tree 
member,
    else if (TREE_CODE (member) == FIELD_DECL)
      {
        /* A non-static data member.  */
-      bool null_object_p;
        int type_quals;
        tree member_type;
- if (INDIRECT_REF_P (object))
-       null_object_p =
-         integer_zerop (tree_strip_nop_conversions (TREE_OPERAND (object, 0)));
-      else
-       null_object_p = false;
-
        /* Convert OBJECT to the type of MEMBER.  */
        if (!same_type_p (TYPE_MAIN_VARIANT (object_type),
                        TYPE_MAIN_VARIANT (member_scope)))
@@ -2957,19 +2950,6 @@ build_class_member_access_expr (cp_expr object, tree 
member,
          if (binfo == error_mark_node)
            return error_mark_node;
- /* It is invalid to try to get to a virtual base of a
-            NULL object.  The most common cause is invalid use of
-            offsetof macro.  */
-         if (null_object_p && kind == bk_via_virtual)
-           {
-             if (complain & tf_error)
-               {
-                 error ("invalid access to non-static data member %qD in "
-                        "virtual base of NULL object", member);
-               }
-             return error_mark_node;
-           }
-
          /* Convert to the base.  */
          object = build_base_path (PLUS_EXPR, object, binfo,
                                    /*nonnull=*/1, complain);
diff --git a/gcc/testsuite/g++.dg/other/offsetof8.C 
b/gcc/testsuite/g++.dg/other/offsetof8.C
index daca70a6fe4..96f1507b845 100644
--- a/gcc/testsuite/g++.dg/other/offsetof8.C
+++ b/gcc/testsuite/g++.dg/other/offsetof8.C
@@ -1,12 +1,32 @@
  // PR c++/68711 - [5 regression] SEGV on an invalid offsetof of a member
  //                of a virtual base
+// PR c++/118346
  // { dg-do compile }
-struct A { int i; };
+struct A {
+  int i;
+  static int s;
+};
+
+struct B: virtual A { int j; };
+
+long unsigned a[] = {
+  // Non static data member in virtual base.
+  !&((B*)0)->i,
+  __builtin_offsetof (B, i),  // { dg-error "data member in virtual base" }
+
+  // Non static data member in class with virtual base.
+  !&((B*)0)->j,
+  __builtin_offsetof (B, j), // { dg-warning "within non-standard-layout type" 
}
-struct B: virtual A { };
+  // Static data member in virtual base.
+  !&((B*)0)->s,
+  __builtin_offsetof (B, s),  // { dg-error "to static data" }
+  // { dg-warning "within non-standard-layout type" "" { target *-*-* } .-1 }
-int a[] = {
-  !&((B*)0)->i,    // { dg-error "invalid access to non-static data member" }
-  __builtin_offsetof (B, i)   // { dg-error "invalid access to non-static" }
+  // No virtual base involvement.
+  !&((A*)0)->i,
+  __builtin_offsetof (A, i),
+  !&((A*)0)->s,
+  __builtin_offsetof (A, s)   // { dg-error "to static data" }
  };
diff --git a/gcc/testsuite/g++.dg/other/offsetof9.C 
b/gcc/testsuite/g++.dg/other/offsetof9.C
index 1936f2e5f76..ac07c21ef34 100644
--- a/gcc/testsuite/g++.dg/other/offsetof9.C
+++ b/gcc/testsuite/g++.dg/other/offsetof9.C
@@ -4,14 +4,14 @@
struct A { int i; };
  struct B : virtual A { };
-__SIZE_TYPE__ s = __builtin_offsetof (B, A::i);        // { dg-warning "'offsetof' 
within non-standard-layout type" }
+__SIZE_TYPE__ s = __builtin_offsetof (B, A::i);        // { dg-error "'offsetof' to 
data member in virtual base" }
template <typename T>
  __SIZE_TYPE__
  foo ()
  {
-  return __builtin_offsetof (T, A::i)          // { dg-warning "'offsetof' within 
non-standard-layout type" }
-        + __builtin_offsetof (B, A::i);        // { dg-warning "'offsetof' within 
non-standard-layout type" }
+  return __builtin_offsetof (T, A::i)          // { dg-error "'offsetof' to data 
member in virtual base" }
+        + __builtin_offsetof (B, A::i);        // { dg-error "'offsetof' to data 
member in virtual base" }
  }
__SIZE_TYPE__ t = foo<B> ();

Reply via email to