On 5/10/25 11:41 AM, 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 __builtin_offsetof (C, i); // L5 } === cut here === Even though L4 is UB, it's technically not invalid, and this patch transforms the error into a warning categorized under -Waddress (so that it can be controlled by the user, and also deactivated for offsetof). It also fixes the error message for L5 to not be confused by the artificial null pointer created by cp_parser_builtin_offsetof. Successfully tested on x86_64-pc-linux-gnu. PR c++/118346 gcc/cp/ChangeLog: * parser.cc (cp_parser_builtin_offsetof): Temporarily disable -Waddress warnings. * semantics.cc (finish_offsetof): Reject accesses to members in virtual bases. * typeck.cc (build_class_member_access_expr): Don't error but warn about accesses to members in virtual bases. gcc/testsuite/ChangeLog: * g++.dg/other/offsetof8.C: Avoid -Wnarrowing warning. * g++.dg/other/offsetof9.C: Adjust test expectations. * g++.dg/other/offsetof10.C: New test. --- gcc/cp/parser.cc | 13 ++++++--- gcc/cp/semantics.cc | 28 ++++++++++++++----- gcc/cp/typeck.cc | 13 +++------ gcc/testsuite/g++.dg/other/offsetof10.C | 36 +++++++++++++++++++++++++ gcc/testsuite/g++.dg/other/offsetof8.C | 6 ++--- gcc/testsuite/g++.dg/other/offsetof9.C | 6 ++--- 6 files changed, 76 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/g++.dg/other/offsetof10.C diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 1fb9e7fd872..43bbc69b196 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -11521,10 +11521,15 @@ cp_parser_builtin_offsetof (cp_parser *parser) tree object_ptr = build_static_cast (input_location, build_pointer_type (type), null_pointer_node, tf_warning_or_error); - - /* Parse the offsetof-member-designator. We begin as if we saw "expr->". */ - expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, object_ptr, - true, &dummy, token->location); + { + /* PR c++/118346: don't complain about object_ptr being null. */ + warning_sentinel s(warn_address); + /* Parse the offsetof-member-designator. We begin as if we saw + "expr->". */ + expr = cp_parser_postfix_dot_deref_expression (parser, CPP_DEREF, + object_ptr, true, &dummy, + token->location); + } while (true) { token = cp_lexer_peek_token (parser->lexer); 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");
This will error if the class has virtual bases even if the member doesn't belong to one of them. Instead of trying to duplicate the virtual base logic here, maybe we could mark the cast from null to indicate that it's coming from offsetof (i.e. with a TREE_LANG_FLAG_), and then check that in build_class_member_access_expr?
Jason
