[PATCH v3] c++: warn on undefined casts from Base to Derived ref/ptr [PR96765]

Zhao Wei Liew zhaoweiliew@gmail.com
Sun Mar 27 12:37:35 GMT 2022


On Fri, 25 Mar 2022 at 05:58, Jason Merrill <jason@redhat.com> wrote:
> >
> >>> +      if (current_function_decl
> >>> +          && (DECL_CONSTRUCTOR_P (current_function_decl)
> >>> +              || DECL_DESTRUCTOR_P (current_function_decl))
> >>> +          && TREE_CODE (expr) == NOP_EXPR
> >>> +          && is_this_parameter (TREE_OPERAND (expr, 0)))
> >>
> >> I think the resolves_to_fixed_type_p function would be useful here;
> >> you're testing for a subset of the cases it handles.
> >
> > Does the new patch look like how you intended it to? The new patch
> > passes all regression tests and newly added tests. However, I don't
> > fully understand the code for resolves_to_fixed_type_p() and
> > fixed_type_or_null(), except that I see that they contain logic to
> > return -1 when within a ctor/dtor.
>
> > +      if (TREE_CODE (expr) == NOP_EXPR
> > +          && resolves_to_fixed_type_p (TREE_OPERAND (expr, 0)) == -1)
>
> Why not just pass expr itself to resolves_to_fixed_type_p?

You're right. I didn't realise that fixed_type_or_null handles the
NOP_EXPR case as part of CASE_CONVERT.

> We might as well also warn if it returns >0, e.g.
>
> struct A { } a;
> struct B: A { };
> auto p = static_cast<B*>(&a); // undefined
>
> You should also handle reference casts.

I've made some changes in v3 to handle reference casts and the case
where resolves_to_fixed_type_p > 0. I've also slightly modified the
patch title to reflect the new changes.

Thanks as always!

v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591894.html
Changes since v2:
- Handle reference casts and add new tests.
- Warn if resolves_to_fixed_type_p > 0 to handle more general cases.

v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591683.html
- Add new warning flag.
- Use resolves_to_fixed_type_p.
-------------- next part --------------
From 261fcff504d2a4971a501747979b1b424d6b09e4 Mon Sep 17 00:00:00 2001
From: Zhao Wei Liew <zhaoweiliew@gmail.com>
Date: Tue, 22 Feb 2022 16:03:17 +0800
Subject: [PATCH] c++: warn on undefined casts from Base to Derived ref/ptr
 [PR96765]

In some cases, converting a Base class reference/pointer to Derived
is undefined behavior.

This patch adds a new warning for them (-Wderived-cast-undefined).

Signed-off-by: Zhao Wei Liew <zhaoweiliew@gmail.com>

	PR c++/96765

gcc/c-family/ChangeLog:

	* c.opt (-Wderived-cast-undefined): New option.

gcc/cp/ChangeLog:

	* typeck.cc (build_static_cast_1): Add warnings.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Wderived-cast-undefined.C: New test.
---
 gcc/c-family/c.opt                            |  4 ++
 gcc/cp/typeck.cc                              | 13 +++++
 .../g++.dg/warn/Wderived-cast-undefined.C     | 49 +++++++++++++++++++
 3 files changed, 66 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9a4828ebe37..1928aee62e6 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -598,6 +598,10 @@ C++ ObjC++ Var(warn_deprecated_enum_float_conv) Warning
 Warn about deprecated arithmetic conversions on operands where one is of enumeration
 type and the other is of a floating-point type.
 
+Wderived-cast-undefined
+C++ Var(warn_derived_cast_undefined) Warning LangEnabledBy(C++, Wall)
+Warn about undefined casts from Base* to Derived* in the Base constructor or destructor.
+
 Wdesignated-init
 C ObjC Var(warn_designated_init) Init(1) Warning
 Warn about positional initialization of structs requiring designated initializers.
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 516fa574ef6..6e19ba5af7f 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -7894,6 +7894,13 @@ build_static_cast_1 (location_t loc, tree type, tree expr, bool c_cast_p,
     {
       tree base;
 
+      if (TYPE_MAIN_VARIANT (intype) != TYPE_MAIN_VARIANT (TREE_TYPE (type))
+          && resolves_to_fixed_type_p (expr))
+        warning_at (loc, OPT_Wderived_cast_undefined,
+                   "invalid %<static_cast%> from type %qT to type %qT "
+                   "before the latter is constructed",
+                   intype, type);
+
       if (processing_template_decl)
 	return expr;
 
@@ -8079,6 +8086,12 @@ build_static_cast_1 (location_t loc, tree type, tree expr, bool c_cast_p,
     {
       tree base;
 
+      if (resolves_to_fixed_type_p (expr))
+        warning_at (loc, OPT_Wderived_cast_undefined,
+                   "invalid %<static_cast%> from type %qT to type %qT "
+                   "before the latter is constructed",
+                   intype, type);
+
       if (processing_template_decl)
 	return expr;
 
diff --git a/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C b/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
new file mode 100644
index 00000000000..3459deadd3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
@@ -0,0 +1,49 @@
+// PR c++/96765
+// { dg-options "-Wderived-cast-undefined" }
+
+struct Derived;
+struct Base {
+  Derived *x;
+  Derived &y;
+  Base();
+  ~Base();
+};
+
+struct Derived : Base {};
+
+Base::Base()
+    : x(static_cast<Derived *>(this)), // { dg-warning "invalid 'static_cast'" }
+      y(static_cast<Derived &>(*this)) // { dg-warning "invalid 'static_cast'" }
+{ 
+  static_cast<Derived *>(this); // { dg-warning "invalid 'static_cast'" }
+  static_cast<Derived &>(*this); // { dg-warning "invalid 'static_cast'" }
+}
+
+Base::~Base() {
+  static_cast<Derived *>(this); // { dg-warning "invalid 'static_cast'" }
+  static_cast<Derived &>(*this); // { dg-warning "invalid 'static_cast'" }
+}
+
+void f1() {
+  Base b;
+  static_cast<Derived *>(&b); // { dg-warning "invalid 'static_cast'" }
+  static_cast<Derived &>(b); // { dg-warning "invalid 'static_cast'" }
+}
+
+void f2(Base *b1, Base &b2) {
+  static_cast<Derived *>(b1);
+  static_cast<Derived &>(*b1);
+  static_cast<Derived *>(&b2);
+  static_cast<Derived &>(b2);
+}
+
+void f3(Derived *d1, Derived &d2) {
+  Base *b = d1;
+  static_cast<Derived *>(b);
+  static_cast<Derived &>(*b);
+
+  b = &d2;
+  static_cast<Derived *>(b);
+  static_cast<Derived &>(*b);
+}
+
-- 
2.25.1



More information about the Gcc-patches mailing list