diff mbox

[RFC] Add alloc_size attribute to the default operator new and operator new[] (take 2)

Message ID 20110803182634.GI2687@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Aug. 3, 2011, 6:26 p.m. UTC 
On Wed, Aug 03, 2011 at 02:31:17PM +0200, Jakub Jelinek wrote:
> 2011-08-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/49905
> 	* decl.c (cxx_init_decl_processing): Add alloc_size (1) attribute
> 	for operator new and operator new [].

Unfortunately the patch caused a bunch of regressions, apparently
attribs.c initializes itself only on the call to decl_attributes, if there
was none, the hash table wasn't initialized and type hashing would crash
when seeing a type with TYPE_ATTRIBUTES.

This version passed bootstrap/regtest on i686-linux.

As for the properties which the middle-end would like to assume or not
from the operator new/operator new[]:
1) for alloc_size it is whether the returned pointer has exactly the
requested bytes defined, i.e. can't return a buffer where only fewer bytes
are valid and it is invalid to access bytes beyond those that were requested
2) aliasing - is the returned buffer guaranteed not to alias any other
object the program may validly access?
3) side-effects - currently for malloc we assume it has no visible
side-effects other than allocating the memory (i.e. malloc internals are
treated as black box), I guess for user supplied operator new/operator
new[] we shouldn't assume it doesn't have other side-effects (thus e.g. we
shouldn't optimize it away, etc.)
Richard, any other properties we are interested in?

2011-08-03  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/49905
	* tree.h (init_attributes): New prototype.
	* attribs.c (init_attributes): No longer static.

	* decl.c (cxx_init_decl_processing): Add alloc_size (1) attribute
	for operator new and operator new [].  Call init_attributes.

	* g++.dg/ext/builtin-object-size3.C: New test.



	Jakub

Comments

Jason Merrill Aug. 3, 2011, 7:53 p.m. UTC | #1 
On 08/03/2011 02:26 PM, Jakub Jelinek wrote:
> As for the properties which the middle-end would like to assume or not
> from the operator new/operator new[]:
> 1) for alloc_size it is whether the returned pointer has exactly the
> requested bytes defined, i.e. can't return a buffer where only fewer bytes
> are valid and it is invalid to access bytes beyond those that were requested

"If it is successful, it shall return the address of the start of a 
block of storage whose length in bytes shall be at least as large as
the requested size."

I suppose this leaves room for a user operator new to allocate some 
extra space at the end and other code to take advantage of that, though 
I would be surprised if anyone actually did that.

> 2) aliasing - is the returned buffer guaranteed not to alias any other
> object the program may validly access?

Not currently.

> 3) side-effects - currently for malloc we assume it has no visible
> side-effects other than allocating the memory (i.e. malloc internals are
> treated as black box), I guess for user supplied operator new/operator
> new[] we shouldn't assume it doesn't have other side-effects (thus e.g. we
> shouldn't optimize it away, etc.)

We have no guarantees about what side-effects a user-supplied operator 
new might have.

Jason
diff mbox

Patch

--- gcc/tree.h.jj	2011-08-02 18:08:11.000000000 +0200
+++ gcc/tree.h	2011-08-03 18:41:07.691420159 +0200
@@ -5547,6 +5547,8 @@  extern bool must_pass_in_stack_var_size_
 
 extern const struct attribute_spec *lookup_attribute_spec (const_tree);
 
+extern void init_attributes (void);
+
 /* Process the attributes listed in ATTRIBUTES and install them in *NODE,
    which is either a DECL (including a TYPE_DECL) or a TYPE.  If a DECL,
    it should be modified in place; if a TYPE, a copy should be created
--- gcc/attribs.c.jj	2011-06-22 12:32:22.000000000 +0200
+++ gcc/attribs.c	2011-08-03 17:57:43.977558394 +0200
@@ -34,8 +34,6 @@  along with GCC; see the file COPYING3.  
 #include "hashtab.h"
 #include "plugin.h"
 
-static void init_attributes (void);
-
 /* Table of the tables of attributes (common, language, format, machine)
    searched.  */
 static const struct attribute_spec *attribute_tables[4];
@@ -107,12 +105,15 @@  eq_attr (const void *p, const void *q)
 /* Initialize attribute tables, and make some sanity checks
    if --enable-checking.  */
 
-static void
+void
 init_attributes (void)
 {
   size_t i;
   int k;
 
+  if (attributes_initialized)
+    return;
+
   attribute_tables[0] = lang_hooks.common_attribute_table;
   attribute_tables[1] = lang_hooks.attribute_table;
   attribute_tables[2] = lang_hooks.format_attribute_table;
--- gcc/cp/decl.c.jj	2011-07-22 22:00:43.970484172 +0200
+++ gcc/cp/decl.c	2011-08-03 17:59:51.130796523 +0200
@@ -3629,6 +3629,7 @@  cxx_init_decl_processing (void)
   current_lang_name = lang_name_cplusplus;
 
   {
+    tree newattrs;
     tree newtype, deltype;
     tree ptr_ftype_sizetype;
     tree new_eh_spec;
@@ -3656,7 +3657,13 @@  cxx_init_decl_processing (void)
     else
       new_eh_spec = noexcept_false_spec;
 
-    newtype = build_exception_variant (ptr_ftype_sizetype, new_eh_spec);
+    /* Ensure attribs.c is initialized.  */
+    init_attributes ();
+    newattrs
+      = build_tree_list (get_identifier ("alloc_size"),
+			 build_tree_list (NULL_TREE, integer_one_node));
+    newtype = cp_build_type_attribute_variant (ptr_ftype_sizetype, newattrs);
+    newtype = build_exception_variant (newtype, new_eh_spec);
     deltype = build_exception_variant (void_ftype_ptr, empty_except_spec);
     push_cp_library_fn (NEW_EXPR, newtype);
     push_cp_library_fn (VEC_NEW_EXPR, newtype);
--- gcc/testsuite/g++.dg/ext/builtin-object-size3.C.jj	2011-08-03 16:08:07.732671633 +0200
+++ gcc/testsuite/g++.dg/ext/builtin-object-size3.C	2011-08-03 16:08:07.732671633 +0200
@@ -0,0 +1,26 @@ 
+// { dg-do compile }
+// { dg-options "-O2" }
+
+void baz (int *, int *);
+
+#define MEMCPY(d,s,l) __builtin___memcpy_chk (d, s, l, __builtin_object_size (d, 0))
+
+int
+foo ()
+{
+  int *p = new int;
+  int *q = new int[4];
+  MEMCPY (p, "abcdefghijklmnopqrstuvwxyz", sizeof (int));
+  MEMCPY (q, "abcdefghijklmnopqrstuvwxyz", 4 * sizeof (int));
+  baz (p, q);
+}
+
+int
+bar ()
+{
+  int *p = new int;
+  int *q = new int[4];
+  MEMCPY (p, "abcdefghijklmnopqrstuvwxyz", sizeof (int) + 1);		// { dg-warning "will always overflow destination buffer" }
+  MEMCPY (q, "abcdefghijklmnopqrstuvwxyz", 4 * sizeof (int) + 1);	// { dg-warning "will always overflow destination buffer" }
+  baz (p, q);
+}