Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Nov 5, 2013 at 3:08 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Nov 5, 2013 at 2:37 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>> On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>> 2013/11/5 Richard Biener <richard.guenther@gmail.com>:
>>>>>> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>> 2013/11/4 Richard Biener <richard.guenther@gmail.com>:
>>>>>>>> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> This patch adds support Pointer Bounds Checker into c-family and LTO front-ends.  The main purpose of changes in front-end is to register all statically initialized objects for checker.  We also need to register such objects created by compiler.
>>>>>>>
>>>>>>> LTO is quite specific front-end.  For LTO Pointer Bounds Checker
>>>>>>> support macro just means it allows instrumented code as input because
>>>>>>> all instrumentation is performed before code is streamed out for LTO.
>>>>>>
>>>>>> But your patch doesn't even make use of the langhook...
>>>>>
>>>>> Use of langhook is in previous patch
>>>>> (http://gcc.gnu.org/ml/gcc-patches/2013-10/msg02408.html). Here is
>>>>> it's part:
>>>>>
>>>>> diff --git a/gcc/toplev.c b/gcc/toplev.c
>>>>> index db269b7..0eaf081 100644
>>>>> --- a/gcc/toplev.c
>>>>> +++ b/gcc/toplev.c
>>>>> @@ -1282,6 +1282,15 @@ process_options (void)
>>>>>            "and -ftree-loop-linear)");
>>>>>  #endif
>>>>>
>>>>> +  if (flag_check_pointer_bounds)
>>>>> +    {
>>>>> +      if (targetm.chkp_bound_mode () == VOIDmode)
>>>>> +       error ("-fcheck-pointers is not supported for this target");
>>>>> +
>>>>> +      if (!lang_hooks.chkp_supported)
>>>>> +       flag_check_pointer_bounds = 0;
>>>>> +    }
>>>>> +
>>>>>    /* One region RA really helps to decrease the code size.  */
>>>>>    if (flag_ira_region == IRA_REGION_AUTODETECT)
>>>>>      flag_ira_region
>>>>>
>>>>> If we try to use -fcheck-pointers -flto for some unsupported language,
>>>>> code will not be instrumented.
>>>>
>>>> What's the reason to have unsupported languages?
>>>
>>> For some languages (e.g. Java) Pointer Bounds Checker does not make
>>> sense at all. Others may require additional support in front-end. The
>>> primary target is C family where solved problem is more critical.
>>
>> What does break if you "enable" it for Java or other "unsupported"
>> languages?  That is, if LTO is able to handle a mixed Java and
>> C binary then why can Java alone not handle it?
>
> In such case checker will produce useless overhead in Java code.
> Resulting code will probably report some bound violations because Java
> FE may generate code which seems wrong for Pointer Bounds Checker.

So it's only an issue that if you use it that it may trip over Java FE bugs?
Not a good reason to have a langhook - you can use the existing
post_options langhook for disallowing this?

Thanks,
Richard.

> Ilya
>
>>
>> Richard.
>>
>>> Ilya
>>>>
>>>> Richard.
>>>>
>>>>> Ilya
>>>>>
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>>> Ilya
>>>>>>>
>>>>>>>>
>>>>>>>> You define CHKP as supported in LTO.  That means it has to be supported
>>>>>>>> for all languages and thus you should drop the langhook.
>>>>>>>>
>>>>>>>> Richard.
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Ilya
>>>>>>>>> --
>>>>>>>>>
>>>>>>>>> gcc/
>>>>>>>>>
>>>>>>>>> 2013-10-29  Ilya Enkovich  <ilya.enkovich@intel.com>
>>>>>>>>>
>>>>>>>>>         * c/c-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>>>>>>>>         * cp/cp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>>>>>>>>         * objc/objc-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>>>>>>>>         * objcp/objcp-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>>>>>>>>         * lto/lto-lang.c (LANG_HOOKS_CHKP_SUPPORTED): New.
>>>>>>>>>         * c/c-parser.c (c_parser_declaration_or_fndef): Register statically
>>>>>>>>>         initialized decls in Pointer Bounds Checker.
>>>>>>>>>         * cp/decl.c (cp_finish_decl): Likewise.
>>>>>>>>>         * gimplify.c (gimplify_init_constructor): Likewise.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> diff --git a/gcc/c/c-lang.c b/gcc/c/c-lang.c
>>>>>>>>> index 614c46d..a32bc6b 100644
>>>>>>>>> --- a/gcc/c/c-lang.c
>>>>>>>>> +++ b/gcc/c/c-lang.c
>>>>>>>>> @@ -43,6 +43,8 @@ enum c_language_kind c_language = clk_c;
>>>>>>>>>  #define LANG_HOOKS_INIT c_objc_common_init
>>>>>>>>>  #undef LANG_HOOKS_INIT_TS
>>>>>>>>>  #define LANG_HOOKS_INIT_TS c_common_init_ts
>>>>>>>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>>>>>>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>>>>>>>
>>>>>>>>>  /* Each front end provides its own lang hook initializer.  */
>>>>>>>>>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>>>>>>>>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>>>>>>>>> index 9ccae3b..65d83c8 100644
>>>>>>>>> --- a/gcc/c/c-parser.c
>>>>>>>>> +++ b/gcc/c/c-parser.c
>>>>>>>>> @@ -1682,6 +1682,12 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
>>>>>>>>>                   maybe_warn_string_init (TREE_TYPE (d), init);
>>>>>>>>>                   finish_decl (d, init_loc, init.value,
>>>>>>>>>                                init.original_type, asm_name);
>>>>>>>>> +
>>>>>>>>> +                 /* Register all decls with initializers in Pointer
>>>>>>>>> +                    Bounds Checker to generate required static bounds
>>>>>>>>> +                    initializers.  */
>>>>>>>>> +                 if (DECL_INITIAL (d) != error_mark_node)
>>>>>>>>> +                   chkp_register_var_initializer (d);
>>>>>>>>>                 }
>>>>>>>>>             }
>>>>>>>>>           else
>>>>>>>>> diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
>>>>>>>>> index a7fa8e4..6d138bd 100644
>>>>>>>>> --- a/gcc/cp/cp-lang.c
>>>>>>>>> +++ b/gcc/cp/cp-lang.c
>>>>>>>>> @@ -81,6 +81,8 @@ static tree get_template_argument_pack_elems_folded (const_tree);
>>>>>>>>>  #define LANG_HOOKS_EH_PERSONALITY cp_eh_personality
>>>>>>>>>  #undef LANG_HOOKS_EH_RUNTIME_TYPE
>>>>>>>>>  #define LANG_HOOKS_EH_RUNTIME_TYPE build_eh_type_type
>>>>>>>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>>>>>>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>>>>>>>
>>>>>>>>>  /* Each front end provides its own lang hook initializer.  */
>>>>>>>>>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>>>>>>>>> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
>>>>>>>>> index 1e92f2a..db40e75 100644
>>>>>>>>> --- a/gcc/cp/decl.c
>>>>>>>>> +++ b/gcc/cp/decl.c
>>>>>>>>> @@ -6379,6 +6379,12 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
>>>>>>>>>              the class specifier.  */
>>>>>>>>>           if (!DECL_EXTERNAL (decl))
>>>>>>>>>             var_definition_p = true;
>>>>>>>>> +
>>>>>>>>> +         /* If var has initilizer then we need to register it in
>>>>>>>>> +            Pointer Bounds Checker to generate static bounds initilizer
>>>>>>>>> +            if required.  */
>>>>>>>>> +         if (DECL_INITIAL (decl) && DECL_INITIAL (decl) != error_mark_node)
>>>>>>>>> +           chkp_register_var_initializer (decl);
>>>>>>>>>         }
>>>>>>>>>        /* If the variable has an array type, lay out the type, even if
>>>>>>>>>          there is no initializer.  It is valid to index through the
>>>>>>>>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
>>>>>>>>> index 4f52c27..503450f 100644
>>>>>>>>> --- a/gcc/gimplify.c
>>>>>>>>> +++ b/gcc/gimplify.c
>>>>>>>>> @@ -4111,6 +4111,11 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>>>>>>>>
>>>>>>>>>                 walk_tree (&ctor, force_labels_r, NULL, NULL);
>>>>>>>>>                 ctor = tree_output_constant_def (ctor);
>>>>>>>>> +
>>>>>>>>> +               /* We need to register created constant object to
>>>>>>>>> +                  initialize bounds for pointers in it.  */
>>>>>>>>> +               chkp_register_var_initializer (ctor);
>>>>>>>>> +
>>>>>>>>>                 if (!useless_type_conversion_p (type, TREE_TYPE (ctor)))
>>>>>>>>>                   ctor = build1 (VIEW_CONVERT_EXPR, type, ctor);
>>>>>>>>>                 TREE_OPERAND (*expr_p, 1) = ctor;
>>>>>>>>> diff --git a/gcc/lto/lto-lang.c b/gcc/lto/lto-lang.c
>>>>>>>>> index 0fa0fc9..b6073d9 100644
>>>>>>>>> --- a/gcc/lto/lto-lang.c
>>>>>>>>> +++ b/gcc/lto/lto-lang.c
>>>>>>>>> @@ -1278,6 +1278,8 @@ static void lto_init_ts (void)
>>>>>>>>>
>>>>>>>>>  #undef LANG_HOOKS_INIT_TS
>>>>>>>>>  #define LANG_HOOKS_INIT_TS lto_init_ts
>>>>>>>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>>>>>>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>>>>>>>
>>>>>>>>>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>>>>>>>>>
>>>>>>>>> diff --git a/gcc/objc/objc-lang.c b/gcc/objc/objc-lang.c
>>>>>>>>> index bc0008b..5e7e43b 100644
>>>>>>>>> --- a/gcc/objc/objc-lang.c
>>>>>>>>> +++ b/gcc/objc/objc-lang.c
>>>>>>>>> @@ -49,6 +49,8 @@ enum c_language_kind c_language = clk_objc;
>>>>>>>>>  #define LANG_HOOKS_GIMPLIFY_EXPR objc_gimplify_expr
>>>>>>>>>  #undef LANG_HOOKS_INIT_TS
>>>>>>>>>  #define LANG_HOOKS_INIT_TS objc_common_init_ts
>>>>>>>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>>>>>>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>>>>>>>
>>>>>>>>>  /* Each front end provides its own lang hook initializer.  */
>>>>>>>>>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
>>>>>>>>> diff --git a/gcc/objcp/objcp-lang.c b/gcc/objcp/objcp-lang.c
>>>>>>>>> index f9b126f..0bb80eb 100644
>>>>>>>>> --- a/gcc/objcp/objcp-lang.c
>>>>>>>>> +++ b/gcc/objcp/objcp-lang.c
>>>>>>>>> @@ -46,6 +46,8 @@ static void objcxx_init_ts (void);
>>>>>>>>>  #define LANG_HOOKS_GIMPLIFY_EXPR objc_gimplify_expr
>>>>>>>>>  #undef LANG_HOOKS_INIT_TS
>>>>>>>>>  #define LANG_HOOKS_INIT_TS objcxx_init_ts
>>>>>>>>> +#undef LANG_HOOKS_CHKP_SUPPORTED
>>>>>>>>> +#define LANG_HOOKS_CHKP_SUPPORTED true
>>>>>>>>>
>>>>>>>>>  /* Each front end provides its own lang hook initializer.  */
>>>>>>>>>  struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;