Re: [CHKP, PATCH] Fix instrumented indirect calls with propagated pointers

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

On Wed, Mar 25, 2015 at 9:50 AM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
> 2015-03-24 17:40 GMT+03:00 Richard Biener <richard.guenther@gmail.com>:
>> On Tue, Mar 24, 2015 at 3:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> On Tue, Mar 24, 2015 at 12:22:27PM +0300, Ilya Enkovich wrote:
>>>
>>> The question is what you want to do in the LTO case for the different cases,
>>> in particular a TU compiled with -fcheck-pointer-bounds and LTO link without
>>> that, or TU compiled without -fcheck-pointer-bounds and LTO link with it.
>>> It could be handled as LTO incompatible option, where lto1 would error out
>>> if you try to mix -fcheck-pointer-bounds with -fno-check-pointer-bounds
>>> code, or e.g. similar to var-tracking, you could consider adjusting the IL
>>> upon LTO reading if if some TU has been built with -fcheck-pointer-bounds
>>> and the LTO link is -fno-check-pointer-bounds.  Dunno what will happen
>>> with -fno-check-pointer-bounds TUs LTO linked with -fcheck-pointer-bounds.
>>> Or another possibility is to or in -fcheck-pointer-bounds from all TUs.
>>>
>>>> Maybe replace attribute usage with a new flag in tree_decl_with_vis structure?
>>>
>>> Depends, might be better to stick it into cgraph_node instead, depends on
>>> whether you are querying it already early in the FEs or just during GIMPLE
>>> when the cgraph node should be created too.
>>
>> I also wonder why it is necessary to execute pass_chkp_instrumentation_passes
>> when mpx is not active.
>>
>> That is, can we guard that properly in
>>
>> void
>> pass_manager::execute_early_local_passes ()
>> {
>>   execute_pass_list (cfun, pass_build_ssa_passes_1->sub);
>>   execute_pass_list (cfun, pass_chkp_instrumentation_passes_1->sub);
>>   execute_pass_list (cfun, pass_local_optimization_passes_1->sub);
>> }
>
> I'm worried about new functions generated in LTO. But with re-created
> flag_check_pointer_bounds it should be safe to guard it.
>
>>
>> (why's that so oddly wrapped?)
>>
>> class pass_chkp_instrumentation_passes
>>
>> also has no gate that guards with flag_mpx or so.
>>
>> That would save a IL walk over all functions (fixup_cfg) and a cgraph
>> edge rebuild.
>
> Right. Will fix it.

I am already testing

Index: gcc/passes.c
===================================================================
--- gcc/passes.c        (revision 221633)
+++ gcc/passes.c        (working copy)
@@ -156,7 +156,8 @@ void
 pass_manager::execute_early_local_passes ()
 {
   execute_pass_list (cfun, pass_build_ssa_passes_1->sub);
-  execute_pass_list (cfun, pass_chkp_instrumentation_passes_1->sub);
+  if (flag_check_pointer_bounds)
+    execute_pass_list (cfun, pass_chkp_instrumentation_passes_1->sub);
   execute_pass_list (cfun, pass_local_optimization_passes_1->sub);
 }

@@ -424,7 +425,8 @@ public:
   virtual bool gate (function *)
     {
       /* Don't bother doing anything if the program has errors.  */
-      return (!seen_error () && !in_lto_p);
+      return (flag_check_pointer_bounds
+             && !seen_error () && !in_lto_p);
     }

 }; // class pass_chkp_instrumentation_passes


Richard.

> Thanks,
> Ilya
>
>>
>> Richard.
>>
>>>         Jakub