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"); + 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..ec35ab6dbb4 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -2958,17 +2958,10 @@ build_class_member_access_expr (cp_expr object, tree member, 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. */ + NULL object. */ 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; - } + warning (OPT_Waddress, "undefined access to non-static data member " + "%qD in virtual base of NULL object", member); /* Convert to the base. */ object = build_base_path (PLUS_EXPR, object, binfo, diff --git a/gcc/testsuite/g++.dg/other/offsetof10.C b/gcc/testsuite/g++.dg/other/offsetof10.C new file mode 100644 index 00000000000..699f1488ba4 --- /dev/null +++ b/gcc/testsuite/g++.dg/other/offsetof10.C @@ -0,0 +1,36 @@ +// PR c++/118346 +// { dg-do compile } +// { dg-options "-Winvalid-offsetof -Waddress" } + +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, // { dg-warning "in virtual base of NULL" } + &(((B*)0)->i), // { dg-warning "in virtual base of NULL" } + __builtin_offsetof (B, i), // { dg-error "data member in virtual base" } + + // Non static data member in class with virtual base. + ((B*)0)->j, + &((B*)0)->j, + __builtin_offsetof (B, j), // { dg-warning "within non-standard-layout type" } + + // Static data member in virtual base. + ((B*)0)->s, + &((B*)0)->s, + __builtin_offsetof (B, s), // { dg-error "to static data" } + // { dg-warning "within non-standard-layout type" "" { target *-*-* } .-1 } + + // No virtual base involvement. + ((A*)0)->i, + &((A*)0)->i, + __builtin_offsetof (A, i), + ((A*)0)->s, + &((A*)0)->s, + __builtin_offsetof (A, s) // { dg-error "to static data" } +}; diff --git a/gcc/testsuite/g++.dg/other/offsetof8.C b/gcc/testsuite/g++.dg/other/offsetof8.C index daca70a6fe4..56cff4ef819 100644 --- a/gcc/testsuite/g++.dg/other/offsetof8.C +++ b/gcc/testsuite/g++.dg/other/offsetof8.C @@ -6,7 +6,7 @@ struct A { int i; }; struct B: virtual A { }; -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" } +long unsigned a[] = { + !&((B*)0)->i, + __builtin_offsetof (B, i) // { dg-error "data member in virtual base" } }; 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> (); -- 2.44.0