This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- From: Ilya Enkovich <enkovich dot gnu at gmail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 6 Nov 2013 14:55:16 +0400
- Subject: Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Authentication-results: sourceware.org; auth=none
- References: <20131031091116 dot GD54327 at msticlxl57 dot ims dot intel dot com> <CAFiYyc3kcBnrjfZMNLL9d=obJZg9giu+47iBB13Vr=px+K=WKA at mail dot gmail dot com> <CAMbmDYa265-Jbk2Uq3-7uKu+mAHrHagQ4iRRRnt7ip6ejhx6eg at mail dot gmail dot com> <CAFiYyc0-TcYs0MjpmyR0Cho+3OG1WdX9xDNnm9KuXioQ1jk2=w at mail dot gmail dot com> <CAMbmDYaOzYjcud8L3fvsj94mWpvNrFTQGHR=p5akvFPrD-NYDA at mail dot gmail dot com> <CAFiYyc2xd7wynB9JRPF3KWY6EhXg-=KUg=g5BTxOjzO1pF3ovQ at mail dot gmail dot com> <CAMbmDYZSpcnCxufY59_QCL_Zp8-=+fQEZvi_tJCDKE=M=Hwqfg at mail dot gmail dot com> <CAFiYyc0RGepPUTU36HCvLrFFM_Y1eGae9kuAV8tn=MZnTRLwzA at mail dot gmail dot com> <CAMbmDYaL=q4D5qfXENhaH5OjdsMznwiyWURWLDw+8unyxP-0oQ at mail dot gmail dot com> <CAFiYyc2owq4pow_h0uep6vw9wUoay5CrLVEVpAuq0yQSmgiW7w at mail dot gmail dot com>
2013/11/6 Richard Biener <richard.guenther@gmail.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?
The issue is that users do not get what expect. I do not want someone
having mixed codes get instrumentation for his Java/Fortran/Ada
functions which slows them down and does nothing useful. What is the
point to allow checks of pointer bounds for language with no pointers?
If I use post_options, I need to change hooks of languages that do not
care about checker to disable it. Now I have it disabled by default.
Using post_options will also require empty redefinition of
post_options in C languages with empty body which may be confusing.
If you do not like this hook, I can move all Pointer Bounds Checker
flags into c.opt and remove the hook. Should it be OK?
Thanks,
Ilya
>
> 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;
- References:
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support
- Re: [PATCH, MPX, 2/X] Pointers Checker [8/25] Languages support