public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ Patch] PR 43452
@ 2013-09-05 22:44 Paolo Carlini
  2013-09-09  6:15 ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Carlini @ 2013-09-05 22:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 588 bytes --]

Hi,

this very old minor issue is about the kind of diagnostic we want to 
produce for something like the existing g++.dg/init/delete1.C:

class C;

  void foo(void *p) {
   delete [] ((C*)p) ;
  }

that is when the array delete is called for a pointer to incomplete 
type. Currently we emit an hard error (we used to ICE) but arguably we 
should only warn and explain the possible undefined behavior at runtime, 
consistently with what we do for the non-array variant of delete (clang 
and icc likewise warn). I tested the below on x86_64-linux.

Thanks!
Paolo.

///////////////////////

[-- Attachment #2: CL_43452 --]
[-- Type: text/plain, Size: 192 bytes --]

/cp
2013-09-06

	PR c++/43452
	* init.c (build_vec_delete_1): When the type is incomplete emit
	a warning not an error.

/testsuite
2013-09-06

	PR c++/43452
	* g++.dg/init/delete1.C: Adjust.

[-- Attachment #3: patch_43452 --]
[-- Type: text/plain, Size: 1734 bytes --]

Index: cp/init.c
===================================================================
--- cp/init.c	(revision 202296)
+++ cp/init.c	(working copy)
@@ -3078,7 +3078,7 @@ build_vec_delete_1 (tree base, tree maxindex, tree
 {
   tree virtual_size;
   tree ptype = build_pointer_type (type = complete_type (type));
-  tree size_exp = size_in_bytes (type);
+  tree size_exp;
 
   /* Temporary variables used by the loop.  */
   tree tbase, tbase_init;
@@ -3106,6 +3106,22 @@ build_vec_delete_1 (tree base, tree maxindex, tree
   if (base == error_mark_node || maxindex == error_mark_node)
     return error_mark_node;
 
+  if (!COMPLETE_TYPE_P (type))
+    {
+      if ((complain & tf_warning)
+	  && warning (0, "possible problem detected in invocation of "
+		      "delete [] operator:"))
+       {
+         cxx_incomplete_type_diagnostic (base, type, DK_WARNING);
+         inform (input_location, "neither the destructor nor the "
+                 "class-specific operator delete [] will be called, "
+                 "even if they are declared when the class is defined");
+       }
+      return build_builtin_delete_call (base);
+    } 
+
+  size_exp = size_in_bytes (type);
+
   if (! MAYBE_CLASS_TYPE_P (type) || TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
     goto no_destructor;
 
Index: testsuite/g++.dg/init/delete1.C
===================================================================
--- testsuite/g++.dg/init/delete1.C	(revision 202284)
+++ testsuite/g++.dg/init/delete1.C	(working copy)
@@ -1,7 +1,7 @@
 // PR c++/19811
 
-class C; // { dg-error "forward" }
+class C; // { dg-warning "forward" }
 
 void foo(void *p) {
-  delete [] ((C*)p) ; // { dg-error "" }
+  delete [] ((C*)p) ; // { dg-warning "problem|incomplete" }
 }

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

* Re: [C++ Patch] PR 43452
  2013-09-05 22:44 [C++ Patch] PR 43452 Paolo Carlini
@ 2013-09-09  6:15 ` Jason Merrill
  2013-09-09 10:38   ` Paolo Carlini
  0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2013-09-09  6:15 UTC (permalink / raw)
  To: Paolo Carlini, gcc-patches

On 09/05/2013 06:44 PM, Paolo Carlini wrote:
> +	  && warning (0, "possible problem detected in invocation of "
> +		      "delete [] operator:"))

The warning should probably be suppressible by some flag.

Jason

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

* Re: [C++ Patch] PR 43452
  2013-09-09  6:15 ` Jason Merrill
@ 2013-09-09 10:38   ` Paolo Carlini
  2013-09-09 13:23     ` Jason Merrill
  0 siblings, 1 reply; 4+ messages in thread
From: Paolo Carlini @ 2013-09-09 10:38 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

Hi,

On 09/09/2013 06:35 AM, Jason Merrill wrote:
> On 09/05/2013 06:44 PM, Paolo Carlini wrote:
>> +      && warning (0, "possible problem detected in invocation of "
>> +              "delete [] operator:"))
>
> The warning should probably be suppressible by some flag.
In fact clang has such a flag - I wasn't sure we wanted it because we 
come from an hard error - thus instead of inventing a name I choose the 
same name. Enabled by default anyway.

Thanks!

Paolo.

/////////////////////

[-- Attachment #2: CL_43452_2 --]
[-- Type: text/plain, Size: 689 bytes --]

2013-09-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/43452
	* doc/invoke.texi (-Wdelete-incomplete): Document it.

/c-family
2013-09-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/43452
	* c.opt (Wdelete-incomplete): Add.

/cp
2013-09-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/43452
	* init.c (build_vec_delete_1): When the type is incomplete emit a
	warning, enabled by default (not an error).
	(build_delete): Adjust to use OPT_Wdelete_incomplete.

/testsuite
2013-09-09  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/43452
	* g++.dg/warn/Wdelete-incomplete-1.C: New.
	* g++.dg/warn/Wdelete-incomplete-2.C: Likewise.
	* g++.dg/init/delete1.C: Adjust.

[-- Attachment #3: patch_43452_2 --]
[-- Type: text/plain, Size: 5322 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 202384)
+++ c-family/c.opt	(working copy)
@@ -339,6 +339,10 @@ Wdeclaration-after-statement
 C ObjC Var(warn_declaration_after_statement) Warning
 Warn when a declaration is found after a statement
 
+Wdelete-incomplete
+C++ ObjC++ Var(warn_delete_incomplete) Init(1) Warning
+Warn when deleting a pointer to incomplete type
+
 Wdelete-non-virtual-dtor
 C++ ObjC++ Var(warn_delnonvdtor) Warning LangEnabledBy(C++ ObjC++,Wall)
 Warn about deleting polymorphic objects with non-virtual destructors
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 202384)
+++ cp/init.c	(working copy)
@@ -3078,7 +3078,7 @@ build_vec_delete_1 (tree base, tree maxindex, tree
 {
   tree virtual_size;
   tree ptype = build_pointer_type (type = complete_type (type));
-  tree size_exp = size_in_bytes (type);
+  tree size_exp;
 
   /* Temporary variables used by the loop.  */
   tree tbase, tbase_init;
@@ -3106,6 +3106,23 @@ build_vec_delete_1 (tree base, tree maxindex, tree
   if (base == error_mark_node || maxindex == error_mark_node)
     return error_mark_node;
 
+  if (!COMPLETE_TYPE_P (type))
+    {
+      if ((complain & tf_warning)
+	  && warning (OPT_Wdelete_incomplete,
+		      "possible problem detected in invocation of "
+		      "delete [] operator:"))
+       {
+         cxx_incomplete_type_diagnostic (base, type, DK_WARNING);
+         inform (input_location, "neither the destructor nor the "
+                 "class-specific operator delete [] will be called, "
+                 "even if they are declared when the class is defined");
+       }
+      return build_builtin_delete_call (base);
+    } 
+
+  size_exp = size_in_bytes (type);
+
   if (! MAYBE_CLASS_TYPE_P (type) || TYPE_HAS_TRIVIAL_DESTRUCTOR (type))
     goto no_destructor;
 
@@ -3820,11 +3837,13 @@ build_delete (tree type, tree addr, special_functi
 	  if (!COMPLETE_TYPE_P (type))
 	    {
 	      if ((complain & tf_warning)
-		  && warning (0, "possible problem detected in invocation of "
+		  && warning (OPT_Wdelete_incomplete,
+			      "possible problem detected in invocation of "
 			      "delete operator:"))
 		{
 		  cxx_incomplete_type_diagnostic (addr, type, DK_WARNING);
-		  inform (input_location, "neither the destructor nor the class-specific "
+		  inform (input_location,
+			  "neither the destructor nor the class-specific "
 			  "operator delete will be called, even if they are "
 			  "declared when the class is defined");
 		}
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 202384)
+++ doc/invoke.texi	(working copy)
@@ -240,8 +240,8 @@ Objective-C and Objective-C++ Dialects}.
 -Wno-attributes -Wno-builtin-macro-redefined @gol
 -Wc++-compat -Wc++11-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
--Wconversion  -Wcoverage-mismatch  -Wno-cpp  -Wno-deprecated  @gol
--Wno-deprecated-declarations -Wdisabled-optimization  @gol
+-Wconversion -Wcoverage-mismatch  -Wdelete-incomplete -Wno-cpp  @gol
+-Wno-deprecated -Wno-deprecated-declarations -Wdisabled-optimization  @gol
 -Wno-div-by-zero -Wdouble-promotion -Wempty-body  -Wenum-compare @gol
 -Wno-endif-labels -Werror  -Werror=* @gol
 -Wfatal-errors  -Wfloat-equal  -Wformat  -Wformat=2 @gol
@@ -4490,6 +4490,12 @@ types. @option{-Wconversion-null} is enabled by de
 Warn when a literal '0' is used as null pointer constant.  This can
 be useful to facilitate the conversion to @code{nullptr} in C++11.
 
+@item -Wdelete-incomplete @r{(C++ and Objective-C++ only)}
+@opindex Wdelete-incomplete
+@opindex Wno-delete-incomplete
+Warn when deleting a pointer to incomplete type, which may cause
+undefined behavior at runtime.  This warning is enabled by default.
+
 @item -Wuseless-cast @r{(C++ and Objective-C++ only)}
 @opindex Wuseless-cast
 @opindex Wno-useless-cast
Index: testsuite/g++.dg/init/delete1.C
===================================================================
--- testsuite/g++.dg/init/delete1.C	(revision 202384)
+++ testsuite/g++.dg/init/delete1.C	(working copy)
@@ -1,7 +1,7 @@
 // PR c++/19811
 
-class C; // { dg-error "forward" }
+class C; // { dg-warning "forward" }
 
 void foo(void *p) {
-  delete [] ((C*)p) ; // { dg-error "" }
+  delete [] ((C*)p) ; // { dg-warning "problem|incomplete" }
 }
Index: testsuite/g++.dg/warn/Wdelete-incomplete-1.C
===================================================================
--- testsuite/g++.dg/warn/Wdelete-incomplete-1.C	(revision 0)
+++ testsuite/g++.dg/warn/Wdelete-incomplete-1.C	(working copy)
@@ -0,0 +1,7 @@
+// PR c++/43452
+
+class Foo;         // { dg-warning "forward" }
+int main() {
+   Foo* p;         // { dg-warning "incomplete" }
+   delete [] p;    // { dg-warning "problem" }
+}
Index: testsuite/g++.dg/warn/Wdelete-incomplete-2.C
===================================================================
--- testsuite/g++.dg/warn/Wdelete-incomplete-2.C	(revision 0)
+++ testsuite/g++.dg/warn/Wdelete-incomplete-2.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/43452
+// { dg-options -Wno-delete-incomplete }
+
+class Foo;
+int main() {
+   Foo* p;
+   delete [] p;
+}

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

* Re: [C++ Patch] PR 43452
  2013-09-09 10:38   ` Paolo Carlini
@ 2013-09-09 13:23     ` Jason Merrill
  0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2013-09-09 13:23 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

OK, thanks.

Jason

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

end of thread, other threads:[~2013-09-09 13:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-05 22:44 [C++ Patch] PR 43452 Paolo Carlini
2013-09-09  6:15 ` Jason Merrill
2013-09-09 10:38   ` Paolo Carlini
2013-09-09 13:23     ` 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).