| Message ID | CACysShg6W+5BjqgmxqkHTmy0BPsXzabSsuuGw-P=i6qAh6sFXg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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 <aivchenk@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 <aivchenk@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
2017-04-04 18:34 GMT+03:00 Jeff Law <law@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 <aivchenk@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 <aivchenk@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
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.gnu@gmail.com>:
> 2017-04-04 18:34 GMT+03:00 Jeff Law <law@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 <aivchenk@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 <aivchenk@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
2017-05-11 0:05 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >> 2017-05-10 22:08 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>> Hi, >>>> >>>> Please share a patch you use. >>>> >>>> Thanks, >>>> Ilya >>>> >>>> 2017-05-09 18:39 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>>>> 2017-04-04 18:34 GMT+03:00 Jeff Law <law@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 <aivchenk@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 <aivchenk@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
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.gnu@gmail.com>: > 2017-05-11 0:05 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>> 2017-05-10 22:08 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>>> Hi, >>>>> >>>>> Please share a patch you use. >>>>> >>>>> Thanks, >>>>> Ilya >>>>> >>>>> 2017-05-09 18:39 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>>>>> 2017-04-04 18:34 GMT+03:00 Jeff Law <law@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 <aivchenk@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 <aivchenk@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
2017-06-09 15:21 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >> 2017-05-11 0:05 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>> 2017-05-10 22:08 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>>>> Hi, >>>>>> >>>>>> Please share a patch you use. >>>>>> >>>>>> Thanks, >>>>>> Ilya >>>>>> >>>>>> 2017-05-09 18:39 GMT+03:00 Alexander Ivchenko <aivchenk@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.gnu@gmail.com>: >>>>>>>> 2017-04-04 18:34 GMT+03:00 Jeff Law <law@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 <aivchenk@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 <aivchenk@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
diff --git a/gcc/testsuite/gcc.target/i386/mpx/PR79987.c b/gcc/testsuite/gcc.target/i386/mpx/PR79987.c new file mode 100644 index 0000000..14a32f3 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/mpx/PR79987.c @@ -0,0 +1,5 @@ +/* { dg-do compile } */ +/* { dg-options "-fcheck-pointer-bounds -mmpx" } */ + +extern void foo; +void *bar = &foo; diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c index b1ff218..8a16025 100644 --- a/gcc/tree-chkp.c +++ b/gcc/tree-chkp.c @@ -3146,6 +3146,11 @@ chkp_get_bounds_for_decl_addr (tree decl) && !flag_chkp_incomplete_type) return chkp_get_zero_bounds (); + /* For void variables use zero bounds as well - we cannot + possibly get the size of the void. */ + if (VOID_TYPE_P (TREE_TYPE (decl))) + return chkp_get_zero_bounds (); + if (flag_chkp_use_static_bounds && VAR_P (decl) && (TREE_STATIC (decl)
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 <aivchenk@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 <aivchenk@gmail.com> * gcc.target/i386/mpx/PR79987.c: New test.