2017-06-09 15:21 GMT+03:00 Alexander Ivchenko <aivch...@gmail.com>:
> Hi Ilya,
>
> I tried changing builtin call so it gets address of a decl instead of
> a decl, but it looked very unnatural and I hit some other problems
What is unnatural in passing a pointer to a function to get its bounds?
Please show your patch and describe problems you hit.
> implementing that. Keeping in mind the exposure of this problem, I
> think it is not worth it. I propose to reconsider the first and the
> simplest patch in this thread: just returning zero bounds for void
> vars. The standard does not specify that size anyways. What do you
> think?
I think declaring void var is similar to declaring static array with no
size and we should handle these cases similarly.
Thanks,
Ilya
>
> Alexander
>
>
>
> 2017-05-11 20:37 GMT+02:00 Ilya Enkovich <enkovich....@gmail.com>:
>> 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
