diff mbox

[MPX,2/X] Pointers Checker [4/25] Constructors

Message ID 20131021121018.GD37888@msticlxl57.ims.intel.com
State New
Headers show

Commit Message

Ilya Enkovich Oct. 21, 2013, 12:10 p.m. UTC
Hi,

This patch introduces two new contructor types supported by cgraph_build_static_cdtor.

'B' type is used to initialize static objects (bounds) created by Pointers Checker. The difference of this type from the regular constructor is that 'B' constructor is never instrumented by Pointers Checker.

'P' type is used by Pointers Checker to generate constructors to initialize bounds of statically initialized pointers. Pointers Checker remove all stores from such constructors after instrumentation.

Since 'P' type constructors are created for statically initialized objects, we need to avoid creation of such objects during its gimplification. New restriction was added to gimplify_init_constructor.

Bootstrapped and checked on linux-x86_64.

Thanks,
Ilya
--

gcc/

2013-10-21  Ilya Enkovich  <ilya.enkovich@intel.com>

	* ipa.c (cgraph_build_static_cdtor_1): Support contructors
	with "chkp ctor" and "bnd_legacy" attributes.
	* gimplify.c (gimplify_init_constructor): Avoid infinite
	loop during gimplification of bounds initializer.

Comments

Jeff Law Oct. 25, 2013, 5:55 a.m. UTC | #1
On 10/21/13 06:10, Ilya Enkovich wrote:
> Hi,
>
> This patch introduces two new contructor types supported by cgraph_build_static_cdtor.
>
> 'B' type is used to initialize static objects (bounds) created by Pointers Checker. The difference of this type from the regular constructor is that 'B' constructor is never instrumented by Pointers Checker.
>
> 'P' type is used by Pointers Checker to generate constructors to initialize bounds of statically initialized pointers. Pointers Checker remove all stores from such constructors after instrumentation.
>
> Since 'P' type constructors are created for statically initialized objects, we need to avoid creation of such objects during its gimplification. New restriction was added to gimplify_init_constructor.
>
> Bootstrapped and checked on linux-x86_64.
>
> Thanks,
> Ilya
> --
>
> gcc/
>
> 2013-10-21  Ilya Enkovich  <ilya.enkovich@intel.com>
>
> 	* ipa.c (cgraph_build_static_cdtor_1): Support contructors
> 	with "chkp ctor" and "bnd_legacy" attributes.
> 	* gimplify.c (gimplify_init_constructor): Avoid infinite
> 	loop during gimplification of bounds initializer.
This is OK.

As a side note, it seems awfully strange to be passing in the type of 
ctor/dtor in a char varaible.  I'd look favorably upon changing that to 
an enum where the enum values describe the cases they handle.  The 
existing code seems so, umm, 80s/90s style.  Obviously not something 
that's required of you to move this patch forward.

jeff
Ilya Enkovich Oct. 30, 2013, 10:40 a.m. UTC | #2
On 24 Oct 23:55, Jeff Law wrote:
> On 10/21/13 06:10, Ilya Enkovich wrote:
> >Hi,
> >
> >This patch introduces two new contructor types supported by cgraph_build_static_cdtor.
> >
> >'B' type is used to initialize static objects (bounds) created by Pointers Checker. The difference of this type from the regular constructor is that 'B' constructor is never instrumented by Pointers Checker.
> >
> >'P' type is used by Pointers Checker to generate constructors to initialize bounds of statically initialized pointers. Pointers Checker remove all stores from such constructors after instrumentation.
> >
> >Since 'P' type constructors are created for statically initialized objects, we need to avoid creation of such objects during its gimplification. New restriction was added to gimplify_init_constructor.
> >
> >Bootstrapped and checked on linux-x86_64.
> >
> >Thanks,
> >Ilya
> >--
> >
> >gcc/
> >
> >2013-10-21  Ilya Enkovich  <ilya.enkovich@intel.com>
> >
> >	* ipa.c (cgraph_build_static_cdtor_1): Support contructors
> >	with "chkp ctor" and "bnd_legacy" attributes.
> >	* gimplify.c (gimplify_init_constructor): Avoid infinite
> >	loop during gimplification of bounds initializer.
> This is OK.
> 
> As a side note, it seems awfully strange to be passing in the type
> of ctor/dtor in a char varaible.  I'd look favorably upon changing
> that to an enum where the enum values describe the cases they
> handle.  The existing code seems so, umm, 80s/90s style.  Obviously
> not something that's required of you to move this patch forward.

Thanks! Installed to trunk.

Ilya
> 
> jeff
>
diff mbox

Patch

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 86bda77..7c350fd 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4033,10 +4033,19 @@  gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	   individual element initialization.  Also don't do this for small
 	   all-zero initializers (which aren't big enough to merit
 	   clearing), and don't try to make bitwise copies of
-	   TREE_ADDRESSABLE types.  */
+	   TREE_ADDRESSABLE types.
+
+	   We cannot apply such transformation when compiling chkp static
+	   initializer because creation of initializer image in the memory
+	   will require static initialization of bounds for it.  It should
+	   result in another gimplification of similar initializer and we
+	   may fall into infinite loop.  */
 	if (valid_const_initializer
 	    && !(cleared || num_nonzero_elements == 0)
-	    && !TREE_ADDRESSABLE (type))
+	    && !TREE_ADDRESSABLE (type)
+	    && (!current_function_decl
+		|| !lookup_attribute ("chkp ctor",
+				      DECL_ATTRIBUTES (current_function_decl))))
 	  {
 	    HOST_WIDE_INT size = int_size_in_bytes (type);
 	    unsigned int align;
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 92343fb2..36cc621 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -1259,9 +1259,11 @@  make_pass_ipa_whole_program_visibility (gcc::context *ctxt)
 }
 
 /* Generate and emit a static constructor or destructor.  WHICH must
-   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
-   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor. 
+   be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+   (for chp static vars constructor) or 'B' (for chkp static bounds
+   constructor).  BODY is a STATEMENT_LIST containing GENERIC
+   statements.  PRIORITY is the initialization priority for this
+   constructor or destructor.
 
    FINAL specify whether the externally visible name for collect2 should
    be produced. */
@@ -1320,6 +1322,20 @@  cgraph_build_static_cdtor_1 (char which, tree body, int priority, bool final)
       DECL_STATIC_CONSTRUCTOR (decl) = 1;
       decl_init_priority_insert (decl, priority);
       break;
+    case 'P':
+      DECL_STATIC_CONSTRUCTOR (decl) = 1;
+      DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("chkp ctor"),
+					  NULL,
+					  NULL_TREE);
+      decl_init_priority_insert (decl, priority);
+      break;
+    case 'B':
+      DECL_STATIC_CONSTRUCTOR (decl) = 1;
+      DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("bnd_legacy"),
+					  NULL,
+					  NULL_TREE);
+      decl_init_priority_insert (decl, priority);
+      break;
     case 'D':
       DECL_STATIC_DESTRUCTOR (decl) = 1;
       decl_fini_priority_insert (decl, priority);
@@ -1337,9 +1353,11 @@  cgraph_build_static_cdtor_1 (char which, tree body, int priority, bool final)
 }
 
 /* Generate and emit a static constructor or destructor.  WHICH must
-   be one of 'I' (for a constructor) or 'D' (for a destructor).  BODY
-   is a STATEMENT_LIST containing GENERIC statements.  PRIORITY is the
-   initialization priority for this constructor or destructor.  */
+   be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+   (for chkp static vars constructor) or 'B' (for chkp static bounds
+   constructor).  BODY is a STATEMENT_LIST containing GENERIC
+   statements.  PRIORITY is the initialization priority for this
+   constructor or destructor.  */
 
 void
 cgraph_build_static_cdtor (char which, tree body, int priority)