public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
@ 2015-11-09  3:31 Patrick Palka
  2015-11-09  3:37 ` Patrick Palka
  2016-02-03 16:06 ` Jason Merrill
  0 siblings, 2 replies; 7+ messages in thread
From: Patrick Palka @ 2015-11-09  3:31 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

When either the static type or the target type of a dynamic cast is
marked 'final', and the type marked final is not derived from the other,
then there is no way to declare a class that is derived from both types
(since one of the types is marked final) so there is no way for such a
dynamic cast to possibly succeed.

This patch detects this case and emits a warning accordingly.

Bootstrap + regtest in progress. Is this OK to commit if testing
succeeds?

gcc/cp/ChangeLog:

	PR c++/12277
	* rtti.c (build_dynamic_cast_1): Warn on dynamic_cast that can
	never succeed due to either the target type or the static type
	being marked final.

gcc/testsuite/ChangeLog:

	PR c++/12277
	* g++.dg/rtti/dyncast8.C: New test.
---
 gcc/cp/rtti.c                        | 18 ++++++++++++++
 gcc/testsuite/g++.dg/rtti/dyncast8.C | 47 ++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/rtti/dyncast8.C

diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index be25be4..b823f8b 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -698,6 +698,24 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst_flags_t complain)
 
 	  target_type = TYPE_MAIN_VARIANT (TREE_TYPE (type));
 	  static_type = TYPE_MAIN_VARIANT (TREE_TYPE (exprtype));
+
+	  if ((CLASSTYPE_FINAL (static_type)
+	       && !DERIVED_FROM_P (target_type, static_type))
+	      || (CLASSTYPE_FINAL (target_type)
+		  && !DERIVED_FROM_P (static_type, target_type)))
+	    {
+	      if (complain & tf_warning)
+		{
+		  if (VAR_P (old_expr))
+	            warning (0, "dynamic_cast of %q#D to %q#T can never succeed",
+				old_expr, type);
+		  else
+	            warning (0, "dynamic_cast of %q#E to %q#T can never succeed",
+				old_expr, type);
+		}
+	      return build_zero_cst (type);
+	    }
+
 	  td2 = get_tinfo_decl (target_type);
 	  if (!mark_used (td2, complain) && !(complain & tf_error))
 	    return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/rtti/dyncast8.C b/gcc/testsuite/g++.dg/rtti/dyncast8.C
new file mode 100644
index 0000000..d98878c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/rtti/dyncast8.C
@@ -0,0 +1,47 @@
+// PR c++/12277
+// { dg-do require { target c++11 } }
+
+struct A1 { virtual ~A1 () { } };
+struct A2 { virtual ~A2 () { } };
+
+struct B1 { virtual ~B1 () { } };
+struct B2 final : B1 { virtual ~B2 () { } };
+
+struct C1 { virtual ~C1 () { } };
+struct C2 final { virtual ~C2 () { } };
+
+A1 *a1;
+
+B1 *b1;
+B2 *b2;
+
+C1 *c1;
+C2 *c2;
+
+void
+foo (void)
+{
+  {
+    A2 *x = dynamic_cast<A2 *> (a1); // { dg-bogus "can never succeed" }
+  }
+
+  {
+    B2 *x = dynamic_cast<B2 *> (b1); // { dg-bogus "can never suceed" }
+    B2 &y = dynamic_cast<B2 &> (*b1); // { dg-bogus "can never suceed" }
+  }
+
+  {
+    B1 *x = dynamic_cast<B1 *> (b2); // { dg-bogus "can never suceed" }
+    B1 &y = dynamic_cast<B1 &> (*b2); // { dg-bogus "can never suceed" }
+  }
+
+  {
+    C2 *x = dynamic_cast<C2 *> (c1); // { dg-warning "can never succeed" }
+    C2 &y = dynamic_cast<C2 &> (*c1); // { dg-warning "can never suceed" }
+  }
+
+  {
+    C1 *x = dynamic_cast<C1 *> (c2); // { dg-warning "can never succeed" }
+    C1 &y = dynamic_cast<C1 &> (*c2); // { dg-warning "can never suceed" }
+  }
+}
-- 
2.6.3.412.gac8e876.dirty

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2015-11-09  3:31 [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results) Patrick Palka
@ 2015-11-09  3:37 ` Patrick Palka
  2016-02-03 16:06 ` Jason Merrill
  1 sibling, 0 replies; 7+ messages in thread
From: Patrick Palka @ 2015-11-09  3:37 UTC (permalink / raw)
  To: GCC Patches; +Cc: Jason Merrill, Patrick Palka

On Sun, Nov 8, 2015 at 10:30 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> When either the static type or the target type of a dynamic cast is
> marked 'final', and the type marked final is not derived from the other,
> then there is no way to declare a class that is derived from both types
> (since one of the types is marked final) so there is no way for such a
> dynamic cast to possibly succeed.
>
> This patch detects this case and emits a warning accordingly.
>
> Bootstrap + regtest in progress. Is this OK to commit if testing
> succeeds?
>
> gcc/cp/ChangeLog:
>
>         PR c++/12277
>         * rtti.c (build_dynamic_cast_1): Warn on dynamic_cast that can
>         never succeed due to either the target type or the static type
>         being marked final.
>
> gcc/testsuite/ChangeLog:
>
>         PR c++/12277
>         * g++.dg/rtti/dyncast8.C: New test.
> ---
>  gcc/cp/rtti.c                        | 18 ++++++++++++++
>  gcc/testsuite/g++.dg/rtti/dyncast8.C | 47 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/rtti/dyncast8.C
>
> diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
> index be25be4..b823f8b 100644
> --- a/gcc/cp/rtti.c
> +++ b/gcc/cp/rtti.c
> @@ -698,6 +698,24 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst_flags_t complain)
>
>           target_type = TYPE_MAIN_VARIANT (TREE_TYPE (type));
>           static_type = TYPE_MAIN_VARIANT (TREE_TYPE (exprtype));
> +
> +         if ((CLASSTYPE_FINAL (static_type)
> +              && !DERIVED_FROM_P (target_type, static_type))
> +             || (CLASSTYPE_FINAL (target_type)
> +                 && !DERIVED_FROM_P (static_type, target_type)))
> +           {
> +             if (complain & tf_warning)
> +               {
> +                 if (VAR_P (old_expr))
> +                   warning (0, "dynamic_cast of %q#D to %q#T can never succeed",
> +                               old_expr, type);
> +                 else
> +                   warning (0, "dynamic_cast of %q#E to %q#T can never succeed",
> +                               old_expr, type);
> +               }
> +             return build_zero_cst (type);
> +           }
> +
>           td2 = get_tinfo_decl (target_type);
>           if (!mark_used (td2, complain) && !(complain & tf_error))
>             return error_mark_node;
> diff --git a/gcc/testsuite/g++.dg/rtti/dyncast8.C b/gcc/testsuite/g++.dg/rtti/dyncast8.C
> new file mode 100644
> index 0000000..d98878c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/rtti/dyncast8.C
> @@ -0,0 +1,47 @@
> +// PR c++/12277
> +// { dg-do require { target c++11 } }
> +
> +struct A1 { virtual ~A1 () { } };
> +struct A2 { virtual ~A2 () { } };
> +
> +struct B1 { virtual ~B1 () { } };
> +struct B2 final : B1 { virtual ~B2 () { } };
> +
> +struct C1 { virtual ~C1 () { } };
> +struct C2 final { virtual ~C2 () { } };
> +
> +A1 *a1;
> +
> +B1 *b1;
> +B2 *b2;
> +
> +C1 *c1;
> +C2 *c2;
> +
> +void
> +foo (void)
> +{
> +  {
> +    A2 *x = dynamic_cast<A2 *> (a1); // { dg-bogus "can never succeed" }
> +  }
> +
> +  {
> +    B2 *x = dynamic_cast<B2 *> (b1); // { dg-bogus "can never suceed" }
> +    B2 &y = dynamic_cast<B2 &> (*b1); // { dg-bogus "can never suceed" }
> +  }
> +
> +  {
> +    B1 *x = dynamic_cast<B1 *> (b2); // { dg-bogus "can never suceed" }
> +    B1 &y = dynamic_cast<B1 &> (*b2); // { dg-bogus "can never suceed" }
> +  }
> +
> +  {
> +    C2 *x = dynamic_cast<C2 *> (c1); // { dg-warning "can never succeed" }
> +    C2 &y = dynamic_cast<C2 &> (*c1); // { dg-warning "can never suceed" }
> +  }
> +
> +  {
> +    C1 *x = dynamic_cast<C1 *> (c2); // { dg-warning "can never succeed" }
> +    C1 &y = dynamic_cast<C1 &> (*c2); // { dg-warning "can never suceed" }
> +  }
> +}
> --
> 2.6.3.412.gac8e876.dirty
>

Oops, this is the wrong test case (forgot to amend the patch before
sending).  The correct test case has s/require/compile/ and
s/suceed/succeed/g done on it.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2015-11-09  3:31 [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results) Patrick Palka
  2015-11-09  3:37 ` Patrick Palka
@ 2016-02-03 16:06 ` Jason Merrill
  2016-02-03 18:40   ` Patrick Palka
  1 sibling, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2016-02-03 16:06 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 11/09/2015 04:30 AM, Patrick Palka wrote:
> +	      if (complain & tf_warning)
> +		{
> +		  if (VAR_P (old_expr))
> +	            warning (0, "dynamic_cast of %q#D to %q#T can never succeed",
> +				old_expr, type);
> +		  else
> +	            warning (0, "dynamic_cast of %q#E to %q#T can never succeed",
> +				old_expr, type);
> +		}
> +	      return build_zero_cst (type);

You also need to handle throwing bad_cast in the reference case.

Jason


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2016-02-03 16:06 ` Jason Merrill
@ 2016-02-03 18:40   ` Patrick Palka
  2016-02-03 18:48     ` Jason Merrill
  2016-02-04  3:08     ` Patrick Palka
  0 siblings, 2 replies; 7+ messages in thread
From: Patrick Palka @ 2016-02-03 18:40 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, gcc-patches

On Tue, 2 Feb 2016, Jason Merrill wrote:

> On 11/09/2015 04:30 AM, Patrick Palka wrote:
>> +	      if (complain & tf_warning)
>> +		{
>> +		  if (VAR_P (old_expr))
>> +	            warning (0, "dynamic_cast of %q#D to %q#T can never 
>> succeed",
>> +				old_expr, type);
>> +		  else
>> +	            warning (0, "dynamic_cast of %q#E to %q#T can never 
>> succeed",
>> +				old_expr, type);
>> +		}
>> +	      return build_zero_cst (type);
>
> You also need to handle throwing bad_cast in the reference case.

Oops, fixed.  I also updated the test case to confirm that the expected
number of calls to bad_cast is generated.

-- >8 --

gcc/cp/ChangeLog:

 	PR c++/12277
 	* rtti.c (build_dynamic_cast_1): Warn on dynamic_cast that can
 	never succeed due to either the target type or the static type
 	being marked final.

gcc/testsuite/ChangeLog:

 	PR c++/12277
 	* g++.dg/rtti/dyncast8.C: New test.
---
  gcc/cp/rtti.c                        | 26 ++++++++++++++++++
  gcc/testsuite/g++.dg/rtti/dyncast8.C | 53 ++++++++++++++++++++++++++++++++++++
  2 files changed, 79 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/rtti/dyncast8.C

diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index a43ff85..b1454d9 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -694,6 +694,32 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst_flags_t complain)

  	  target_type = TYPE_MAIN_VARIANT (TREE_TYPE (type));
  	  static_type = TYPE_MAIN_VARIANT (TREE_TYPE (exprtype));
+
+	  if ((CLASSTYPE_FINAL (static_type)
+	       && !DERIVED_FROM_P (target_type, static_type))
+	      || (CLASSTYPE_FINAL (target_type)
+		  && !DERIVED_FROM_P (static_type, target_type)))
+	    {
+	      if (complain & tf_warning)
+		{
+		  if (VAR_P (old_expr))
+	            warning (0, "dynamic_cast of %q#D to %q#T can never succeed",
+				old_expr, type);
+		  else
+	            warning (0, "dynamic_cast of %q#E to %q#T can never succeed",
+				old_expr, type);
+		}
+
+	      if (tc == REFERENCE_TYPE)
+		{
+		  tree expr = throw_bad_cast ();
+		  TREE_TYPE (expr) = type;
+		  return expr;
+		}
+	      else
+		return build_zero_cst (type);
+	    }
+
  	  td2 = get_tinfo_decl (target_type);
  	  if (!mark_used (td2, complain) && !(complain & tf_error))
  	    return error_mark_node;
diff --git a/gcc/testsuite/g++.dg/rtti/dyncast8.C b/gcc/testsuite/g++.dg/rtti/dyncast8.C
new file mode 100644
index 0000000..f63ed5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/rtti/dyncast8.C
@@ -0,0 +1,53 @@
+// PR c++/12277
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "-fdump-tree-original" }
+
+struct A1 { virtual ~A1 () { } };
+struct A2 { virtual ~A2 () { } };
+
+struct B1 { virtual ~B1 () { } };
+struct B2 final : B1 { virtual ~B2 () { } };
+
+struct C1 { virtual ~C1 () { } };
+struct C2 final { virtual ~C2 () { } };
+
+A1 *a1;
+
+B1 *b1;
+B2 *b2;
+
+C1 *c1;
+C2 *c2;
+
+void
+foo (void)
+{
+  {
+    A2 *x = dynamic_cast<A2 *> (a1);
+  }
+
+  {
+    B2 *x = dynamic_cast<B2 *> (b1);
+    // The following cast may throw bad_cast.
+    B2 &y = dynamic_cast<B2 &> (*b1);
+  }
+
+  {
+    B1 *x = dynamic_cast<B1 *> (b2);
+    B1 &y = dynamic_cast<B1 &> (*b2);
+  }
+
+  {
+    C2 *x = dynamic_cast<C2 *> (c1); // { dg-warning "can never succeed" }
+    // The following cast may throw bad_cast.
+    C2 &y = dynamic_cast<C2 &> (*c1); // { dg-warning "can never succeed" }
+  }
+
+  {
+    C1 *x = dynamic_cast<C1 *> (c2); // { dg-warning "can never succeed" }
+    // The following cast may throw bad_cast.
+    C1 &y = dynamic_cast<C1 &> (*c2); // { dg-warning "can never succeed" }
+  }
+}
+
+// { dg-final { scan-tree-dump-times "bad_cast" 3 "original" } }
-- 
2.7.0.240.g19e8eb6

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2016-02-03 18:40   ` Patrick Palka
@ 2016-02-03 18:48     ` Jason Merrill
  2016-02-04  3:08     ` Patrick Palka
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2016-02-03 18:48 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches

OK.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2016-02-03 18:40   ` Patrick Palka
  2016-02-03 18:48     ` Jason Merrill
@ 2016-02-04  3:08     ` Patrick Palka
  2016-02-04 14:14       ` Jason Merrill
  1 sibling, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2016-02-04  3:08 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, GCC Patches

On Wed, Feb 3, 2016 at 1:40 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Tue, 2 Feb 2016, Jason Merrill wrote:
>
>> On 11/09/2015 04:30 AM, Patrick Palka wrote:
>>>
>>> +             if (complain & tf_warning)
>>> +               {
>>> +                 if (VAR_P (old_expr))
>>> +                   warning (0, "dynamic_cast of %q#D to %q#T can never
>>> succeed",
>>> +                               old_expr, type);
>>> +                 else
>>> +                   warning (0, "dynamic_cast of %q#E to %q#T can never
>>> succeed",
>>> +                               old_expr, type);
>>> +               }
>>> +             return build_zero_cst (type);
>>
>>
>> You also need to handle throwing bad_cast in the reference case.
>
>
> Oops, fixed.  I also updated the test case to confirm that the expected
> number of calls to bad_cast is generated.
>
> -- >8 --
>
> gcc/cp/ChangeLog:
>
>         PR c++/12277
>         * rtti.c (build_dynamic_cast_1): Warn on dynamic_cast that can
>         never succeed due to either the target type or the static type
>         being marked final.
>
> gcc/testsuite/ChangeLog:
>
>         PR c++/12277
>         * g++.dg/rtti/dyncast8.C: New test.
> ---
>  gcc/cp/rtti.c                        | 26 ++++++++++++++++++
>  gcc/testsuite/g++.dg/rtti/dyncast8.C | 53
> ++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/rtti/dyncast8.C
>
> diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
> index a43ff85..b1454d9 100644
> --- a/gcc/cp/rtti.c
> +++ b/gcc/cp/rtti.c
> @@ -694,6 +694,32 @@ build_dynamic_cast_1 (tree type, tree expr,
> tsubst_flags_t complain)
>
>           target_type = TYPE_MAIN_VARIANT (TREE_TYPE (type));
>           static_type = TYPE_MAIN_VARIANT (TREE_TYPE (exprtype));
> +
> +         if ((CLASSTYPE_FINAL (static_type)
> +              && !DERIVED_FROM_P (target_type, static_type))
> +             || (CLASSTYPE_FINAL (target_type)
> +                 && !DERIVED_FROM_P (static_type, target_type)))
> +           {
> +             if (complain & tf_warning)
> +               {
> +                 if (VAR_P (old_expr))
> +                   warning (0, "dynamic_cast of %q#D to %q#T can never
> succeed",
> +                               old_expr, type);
> +                 else
> +                   warning (0, "dynamic_cast of %q#E to %q#T can never
> succeed",
> +                               old_expr, type);
> +               }

It just occurred to me that issuing this warning during template
instantiation may be undesirable if the dynamic_cast being built was
originally dependent (for the same reasons it is undesirable to issue
-Wuseless-cast warnings during instantiation, which we already avoid
doing).  Especially so since this particular warning is not associated
with any warning flag.  I'll try to come up with a more careful patch
for stage 1.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results)
  2016-02-04  3:08     ` Patrick Palka
@ 2016-02-04 14:14       ` Jason Merrill
  0 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2016-02-04 14:14 UTC (permalink / raw)
  To: Patrick Palka; +Cc: GCC Patches

On 02/03/2016 10:07 PM, Patrick Palka wrote:
> It just occurred to me that issuing this warning during template
> instantiation may be undesirable if the dynamic_cast being built was
> originally dependent (for the same reasons it is undesirable to issue
> -Wuseless-cast warnings during instantiation, which we already avoid
> doing).  Especially so since this particular warning is not associated
> with any warning flag.  I'll try to come up with a more careful patch
> for stage 1.

Good point, thanks.

Jason


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-02-04 14:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-09  3:31 [PATCH] Partially fix PR c++/12277 (Warn on dynamic cast with known NULL results) Patrick Palka
2015-11-09  3:37 ` Patrick Palka
2016-02-03 16:06 ` Jason Merrill
2016-02-03 18:40   ` Patrick Palka
2016-02-03 18:48     ` Jason Merrill
2016-02-04  3:08     ` Patrick Palka
2016-02-04 14:14       ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).