2013/11/6 Richard Biener <richard.guent...@gmail.com>: > On Tue, Nov 5, 2013 at 3:08 PM, Ilya Enkovich <enkovich....@gmail.com> wrote: >> 2013/11/5 Richard Biener <richard.guent...@gmail.com>: >>> On Tue, Nov 5, 2013 at 2:37 PM, Ilya Enkovich <enkovich....@gmail.com> >>> wrote: >>>> 2013/11/5 Richard Biener <richard.guent...@gmail.com>: >>>>> On Tue, Nov 5, 2013 at 2:20 PM, Ilya Enkovich <enkovich....@gmail.com> >>>>> wrote: >>>>>> 2013/11/5 Richard Biener <richard.guent...@gmail.com>: >>>>>>> On Tue, Nov 5, 2013 at 2:02 PM, Ilya Enkovich <enkovich....@gmail.com> >>>>>>> wrote: >>>>>>>> 2013/11/4 Richard Biener <richard.guent...@gmail.com>: >>>>>>>>> On Thu, Oct 31, 2013 at 10:11 AM, Ilya Enkovich >>>>>>>>> <enkovich....@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.enkov...@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;