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
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?
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
