2017-05-11 0:05 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>:
> Didn't quite get your point. I though that the idea is to hit this code:
>
>       bounds = chkp_generate_extern_var_bounds (decl);
> ... to get the builtin_sizeof call on this variable and hence to get
> the size relocation. What exactly do you mean by reproducing the
> problem? Currently the compiler just ICE on the testcase :)

There are various ways to build bounds. On of them is used by default
and causes ICE. With your patch another way is chosen causing another
ICE. But you can get this another ICE by simply using compiler flag.
Obviously both ways should be fixed.

You don't have to hit chkp_generate_extern_var_bounds to use size
relocation. If you create static bounds var then it is initialized later
in chkp_output_static_bounds and size relocations can appear there
either.

Thanks
Ilya

>
> Alexander
>
> 2017-05-10 22:51 GMT+02:00 Ilya Enkovich <enkovich....@gmail.com>:
>> 2017-05-10 22:08 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>:
>>> Hi,
>>>
>>> something like that:
>>>
>>> diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
>>> index b1ff218..be06d71 100644
>>> --- a/gcc/tree-chkp.c
>>> +++ b/gcc/tree-chkp.c
>>> @@ -3148,6 +3148,7 @@ chkp_get_bounds_for_decl_addr (tree decl)
>>>
>>>    if (flag_chkp_use_static_bounds
>>>        && VAR_P (decl)
>>> +      && !VOID_TYPE_P (TREE_TYPE (decl))
>>>        && (TREE_STATIC (decl)
>>>               || DECL_EXTERNAL (decl)
>>>               || TREE_PUBLIC (decl))
>>> @@ -3162,10 +3163,11 @@ chkp_get_bounds_for_decl_addr (tree decl)
>>>        gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
>>>      }
>>>    else if (!DECL_SIZE (decl)
>>> -      || (chkp_variable_size_type (TREE_TYPE (decl))
>>> -         && (TREE_STATIC (decl)
>>> -             || DECL_EXTERNAL (decl)
>>> -             || TREE_PUBLIC (decl))))
>>> +          || VOID_TYPE_P (TREE_TYPE (decl))
>>> +          || (chkp_variable_size_type (TREE_TYPE (decl))
>>> +              && (TREE_STATIC (decl)
>>> +                  || DECL_EXTERNAL (decl)
>>> +                  || TREE_PUBLIC (decl))))
>>>      {
>>>        gcc_assert (VAR_P (decl));
>>>        bounds = chkp_generate_extern_var_bounds (decl);
>>
>> Looking one more time into this issue I now think that bounds generation
>> already uses size relocation and this is not a part to fix.
>>
>> BTW this patch just restricts static bounds creation for void vars which
>> is not what you need. I think you should be able to reproduce the problem
>> without any patch by just using -fno-chkp-use-static-bounds.
>>
>> Looks like gimple doesn't allow void vars as call arguments. I don't know
>> what is the best way to handle this restriction. The simplest thing a can
>> think of is to change builtin call so it gets address of a decl instead of
>> a decl.
>>
>> Thanks,
>> Ilya
>>
>>>
>>> Thanks,
>>> Alexander
>>>
>>> 2017-05-10 20:55 GMT+02:00 Ilya Enkovich <enkovich....@gmail.com>:
>>>> Hi,
>>>>
>>>> Please share a patch you use.
>>>>
>>>> Thanks,
>>>> Ilya
>>>>
>>>> 2017-05-09 18:39 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>:
>>>>> If we use chkp_generate_extern_var_bounds for void variable just as
>>>>> for arrays with unknown size, we will create the following gimple seq:
>>>>>
>>>>> # VUSE <.MEM>
>>>>> __size_tmp.0 = __builtin_ia32_sizeof (foo);
>>>>> __size_tmp.1_3 = __size_tmp.0;
>>>>>
>>>>> However, this will fail in verify_gimple_call:
>>>>>
>>>>> tree arg = gimple_call_arg (stmt, i);
>>>>> if ((is_gimple_reg_type (TREE_TYPE (arg))
>>>>>      && !is_gimple_val (arg))
>>>>>     || (!is_gimple_reg_type (TREE_TYPE (arg))
>>>>>         && !is_gimple_lvalue (arg)))
>>>>>   {
>>>>>     error ("invalid argument to gimple call");
>>>>>     debug_generic_expr (arg);
>>>>>     return true;
>>>>>   }
>>>>> ..here the TREE_TYPE(arg)==void. Any ideas for a good workaround ?
>>>>>
>>>>> Alexander
>>>>>
>>>>>
>>>>>
>>>>> 2017-04-08 21:59 GMT+02:00 Ilya Enkovich <enkovich....@gmail.com>:
>>>>>> 2017-04-04 18:34 GMT+03:00 Jeff Law <l...@redhat.com>:
>>>>>>> On 04/04/2017 09:07 AM, Alexander Ivchenko wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> When creating static bounds for foo below we end up with:
>>>>>>>>
>>>>>>>> *((unsigned long *) &__chkp_bounds_of_foo + 8) =
>>>>>>>> ~(__builtin_ia32_sizeof (foo) + ((long unsigned int) &foo +
>>>>>>>> 18446744073709551615));
>>>>>>>>
>>>>>>>> This fails in gimplify_function_tree with gcc_assert (!VOID_TYPE_P
>>>>>>>> (TREE_TYPE (*expr_p)));
>>>>>>>>
>>>>>>>> Is it OK?
>>>>>>>>
>>>>>>>> gcc/ChangeLog:
>>>>>>>>
>>>>>>>> 2017-04-04  Alexander Ivchenko  <aivch...@gmail.com>
>>>>>>>>
>>>>>>>>         * tree-chkp.c (chkp_get_bounds_for_decl_addr):
>>>>>>>> assigning zero bounds to void variables
>>>>>>>>
>>>>>>>>
>>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>>
>>>>>>>> 2017-04-04  Alexander Ivchenko  <aivch...@gmail.com>
>>>>>>>>
>>>>>>>>         * gcc.target/i386/mpx/PR79987.c: New test.
>>>>>>>
>>>>>>> I've put this (and other CHKP fixes) in the queue for gcc-8 as AFAICT 
>>>>>>> it's
>>>>>>> not a regression.
>>>>>>>
>>>>>>> Jeff
>>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> If we delay it for GCC8 anyway then I think we may fix it in a better 
>>>>>> way. If we
>>>>>> cannot detect size of a variable then size relocations may be used. It 
>>>>>> is done
>>>>>> already for arrays with unknown size and also can be done for void vars.
>>>>>>
>>>>>> Thanks,
>>>>>> Ilya

Reply via email to