Re: [PATCH v3 1/4] c: runtime checking for assigment of VM types
Am Donnerstag, dem 18.07.2024 um 18:32 + schrieb Qing Zhao: > Hi, Martin, > > I briefly reviewed these 4 patches this week. > > Overall, I think that this is a nice new security feature to be added into > gcc. Thank you! (not only for security! it currently helps me with my numerics) > > At the same time, I have the following questions about the patches: > > 1. Does the name of the option mismatch the description of the option? > > -fvla-bounds > Emit run-time consistency checks for variably-modified types. > > Is “variably-modified type” equal to “via” (variable length array)? > > I see all other current options -Wvla-parameter, -Wvla, etc, the “vla”s all > are > for "variable length array" in the description. > > So, a little confused with the “variable-modified type” you used in the > documentation for > “vla”. > > I see that in GCC source code, the concept of “variable-modified type” > includes “via”. > but seems not limited to “vla”. Variably modified types are types that are a VLA or are derived from a VLA somehow, e.g. a pointer to a VLA. So I would say both terms are correct in this context because we check all bounds in a variably modified type but each bound belongs to an embedded VLA type. But anyway, I agree that the description would better if it would not refer to variably modified types. I also wonder about the name of the option too. Maybe there is a better name? > > 2. I feel that the documentation of the option need some simple examples to > show > the new feature’s capabilities to help users to use it. > > Currently. The doc of the option: > > +@item -fvla-bounds > > +This option is only available when compiling C code. If activated, > > +additional code is emitted that verifies at run time for assignments > > +involving variably-modified types that corresponding size expressions > > +evaluate to the same value. > > Providing multiple simple examples below will be helpful. Yes. > > 3. From my understanding, with -fvla-bounds, when there is any bound > mismatch > during runtime, the program will trap, but without any information on why such > trap happens. > > Is this correct? > > If so, I think that this might need to be documented to inform user the > current behavior, > then the users will have an accurate expectation on what might happen with > this > option. That makes sense. > > Later some error message might need to be issued at the same time when the > trap happens. > > > On Jul 15, 2024, at 03:19, Martin Uecker wrote: > > > > > > When checking compatibility of types during assignment, collect > > all pairs of types where the outermost bound needs to match at > > run-time. This list is then processed to add runtime checks for > > each bound. > > > > gcc/c-family: > > * c-opt (fvla-bounds): New flag. > > > > gcc/c: > > * c-typeck.cc (struct instrument_data): New structure. > > (comp_target_types_instr convert_for_assignment_instrument): New > > interfaces for existing functions. > > (struct comptypes_data): Add instrumentation. > > (comptypes_check_enum_int_intr): New interface. > > (comptypes_check_enum_int): Old interface (calls new). > > (comptypes_internal): Collect VLA types needed for UBSan. > > (comp_target_types_instr): New interface. > > (comp_target_types): Old interface (calls new). > > (function_types_compatible_p): No instrumentation for function > > arguments. > > (process_vm_constraints): New function. > > (convert_argument): Adapt. > > (convert_for_assignment_instrument): New interface. > > (convert_for_assignment): Instrument assignments. > > (c_instrument_vm_assign): Helper function. > > (process_vm_constraints): Helper function. > > “process_vm_constraints” repeated twice in the above. > > > thanks. Thank you! Martin > > Qing > > > > gcc/doc/: > > * invoke.texi (fvla-bounds): Document new flag. > > > > gcc/testsuite: > > * gcc.dg/vla-bounds-1.c: New test. > > * gcc.dg/vla-bounds-assign-1.c: New test. > > * gcc.dg/vla-bounds-assign-2.c: New test. > > * gcc.dg/vla-bounds-assign-3.c: New test. > > * gcc.dg/vla-bounds-assign-4.c: New test. > > * gcc.dg/vla-bounds-func-1.c: New test. > > * gcc.dg/vla-bounds-init-1.c: New test. > > * gcc.dg/vla-bounds-init-2.c: New test. > > * gcc.dg/vla-bounds-init-3.c: New test. > > * gcc.dg/vla-bounds-init-4.c: New test. > > * gcc.dg/vla-bounds-nest-1.c: New test. > > * gcc.dg/vla-bounds-nest-2.c: New test. > > * gcc.dg/vla-bounds-ret-1.c: New test. > > * gcc.dg/vla-bounds-ret-2.c: New test. > > --- > > gcc/c-family/c.opt | 4 + > > gcc/c/c-typeck.cc | 171 ++--- > > gcc/doc/invoke.texi | 9 ++ > > gcc/testsuite/gcc.dg/vla-bounds-1.c | 74 + > > gcc/testsuite/gcc.dg/vla-bounds-assign-1.c | 32 > > gcc/testsuite/gcc.dg/vla-bounds-assign-2.c | 30 > > gcc/testsuite/gcc.dg/vla-bounds-assign-3.c | 43 ++ > > gcc/testsuite/gcc.dg/vla-bounds-assign-4.c | 26 > > gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 54 +++ > > gcc/testsuite/g
Re: [PATCH v3 1/4] c: runtime checking for assigment of VM types
Hi, Martin, I briefly reviewed these 4 patches this week. Overall, I think that this is a nice new security feature to be added into gcc. At the same time, I have the following questions about the patches: 1. Does the name of the option mismatch the description of the option? -fvla-bounds Emit run-time consistency checks for variably-modified types. Is “variably-modified type” equal to “via” (variable length array)? I see all other current options -Wvla-parameter, -Wvla, etc, the “vla”s all are for "variable length array" in the description. So, a little confused with the “variable-modified type” you used in the documentation for “vla”. I see that in GCC source code, the concept of “variable-modified type” includes “via”. but seems not limited to “vla”. 2. I feel that the documentation of the option need some simple examples to show the new feature’s capabilities to help users to use it. Currently. The doc of the option: > +@item -fvla-bounds > +This option is only available when compiling C code. If activated, > +additional code is emitted that verifies at run time for assignments > +involving variably-modified types that corresponding size expressions > +evaluate to the same value. Providing multiple simple examples below will be helpful. 3. From my understanding, with -fvla-bounds, when there is any bound mismatch during runtime, the program will trap, but without any information on why such trap happens. Is this correct? If so, I think that this might need to be documented to inform user the current behavior, then the users will have an accurate expectation on what might happen with this option. Later some error message might need to be issued at the same time when the trap happens. > On Jul 15, 2024, at 03:19, Martin Uecker wrote: > > > When checking compatibility of types during assignment, collect > all pairs of types where the outermost bound needs to match at > run-time. This list is then processed to add runtime checks for > each bound. > > gcc/c-family: > * c-opt (fvla-bounds): New flag. > > gcc/c: > * c-typeck.cc (struct instrument_data): New structure. > (comp_target_types_instr convert_for_assignment_instrument): New > interfaces for existing functions. > (struct comptypes_data): Add instrumentation. > (comptypes_check_enum_int_intr): New interface. > (comptypes_check_enum_int): Old interface (calls new). > (comptypes_internal): Collect VLA types needed for UBSan. > (comp_target_types_instr): New interface. > (comp_target_types): Old interface (calls new). > (function_types_compatible_p): No instrumentation for function > arguments. > (process_vm_constraints): New function. > (convert_argument): Adapt. > (convert_for_assignment_instrument): New interface. > (convert_for_assignment): Instrument assignments. > (c_instrument_vm_assign): Helper function. > (process_vm_constraints): Helper function. “process_vm_constraints” repeated twice in the above. thanks. Qing > > gcc/doc/: > * invoke.texi (fvla-bounds): Document new flag. > > gcc/testsuite: > * gcc.dg/vla-bounds-1.c: New test. > * gcc.dg/vla-bounds-assign-1.c: New test. > * gcc.dg/vla-bounds-assign-2.c: New test. > * gcc.dg/vla-bounds-assign-3.c: New test. > * gcc.dg/vla-bounds-assign-4.c: New test. > * gcc.dg/vla-bounds-func-1.c: New test. > * gcc.dg/vla-bounds-init-1.c: New test. > * gcc.dg/vla-bounds-init-2.c: New test. > * gcc.dg/vla-bounds-init-3.c: New test. > * gcc.dg/vla-bounds-init-4.c: New test. > * gcc.dg/vla-bounds-nest-1.c: New test. > * gcc.dg/vla-bounds-nest-2.c: New test. > * gcc.dg/vla-bounds-ret-1.c: New test. > * gcc.dg/vla-bounds-ret-2.c: New test. > --- > gcc/c-family/c.opt | 4 + > gcc/c/c-typeck.cc | 171 ++--- > gcc/doc/invoke.texi | 9 ++ > gcc/testsuite/gcc.dg/vla-bounds-1.c | 74 + > gcc/testsuite/gcc.dg/vla-bounds-assign-1.c | 32 > gcc/testsuite/gcc.dg/vla-bounds-assign-2.c | 30 > gcc/testsuite/gcc.dg/vla-bounds-assign-3.c | 43 ++ > gcc/testsuite/gcc.dg/vla-bounds-assign-4.c | 26 > gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 54 +++ > gcc/testsuite/gcc.dg/vla-bounds-init-1.c | 26 > gcc/testsuite/gcc.dg/vla-bounds-init-2.c | 25 +++ > gcc/testsuite/gcc.dg/vla-bounds-init-3.c | 25 +++ > gcc/testsuite/gcc.dg/vla-bounds-init-4.c | 27 > gcc/testsuite/gcc.dg/vla-bounds-nest-1.c | 31 > gcc/testsuite/gcc.dg/vla-bounds-nest-2.c | 26 > gcc/testsuite/gcc.dg/vla-bounds-ret-1.c | 27 > gcc/testsuite/gcc.dg/vla-bounds-ret-2.c | 28 > 17 files changed, 639 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-1.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-1.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-2.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-3.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-4.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-1.c > create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-
Re: C runtime checking for assigment of VM types, v3
> On Jul 15, 2024, at 17:41, Martin Uecker wrote:
>
> Am Montag, dem 15.07.2024 um 21:26 + schrieb Qing Zhao:
>> Hi, Martin,
>> I didn’t see your v3 patches attached to the email, did you miss them?
>> (I really want to see them -:).
>
> Sorry, I should have CCed you. It was sent as a series of patches:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657254.html
Thanks. Will take a look at them.
>
>>
>>> On Jul 15, 2024, at 16:58, Martin Uecker wrote:
>>>
>>> Am Montag, dem 15.07.2024 um 13:05 -0700 schrieb Kees Cook:
On Mon, Jul 15, 2024 at 07:20:31PM +0200, Martin Uecker wrote:
> No, there are still two many missing pieces. The following
> works already
>
> int h(int n, int buf[n])
> {
> return __builtin_dynamic_object_size(buf, 1);
> }
Yeah, this is nice.
There are some interesting things happening around this general
idea. Clang has the rather limited attributes "pass_object_size" and
"pass_dynamic_object_size" that will work on function prototypes that
will inform a _bos or _bdos internally, but you can only choose _one_
type to apply to a given function parameter:
size_t h(char * const __attribute__((pass_dynamic_object_size(1))) buf)
{
return __builtin_dynamic_object_size(buf, 1);
}
https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
I have found it easier to just make wrapper macros, as that can allow
both size types:
size_t h(char *buf, const size_t p_max, const size_t p_min);
#define h(p) \
({ \
const size_t __p_max = __builtin_dynamic_object_size(p, 0); \
const size_t __p_min = __builtin_dynamic_object_size(p, 1); \
__h(p, __p_max, __p_min); \
})
But best is that it just gets handled automatically, which will be the
goals of the more generalized "counted_by" (and "sized_by") attributes
that will provide similar coverage as your example:
size_t h(int * __attribute__((sized_by(bytes))) buf, int bytes)
{
return __builtin_dynamic_object_size(buf, 1);
}
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
>>>
>>> Indeed, I already complained a lot to them about that design ;-)
Those attributes end up being similar to what you have
>>>
>>> Well, the syntax is from C99, we just have to make it work.
>>>
only the explicit
predeclaration isn't needed. i.e. to put "int n" in your example after
"buf", it needs predeclaration:
int h(int n; int buf[n], int n)
{
...
}
(But Clang doesn't appear to support predeclarations.)
>>>
>>> And isn't this a lot nicer? It also follows the exact same
>>> scoping rules of the language as everything else.
>>>
>>> In GCC the example above works also with this:
>>>
>>> int h(int n; char buf[n], int n)
>>> {
>>> return __builtin_dynamic_object_size(buf, 1);
>>> }
>>> https://godbolt.org/z/vfqoKaq7e
>>>
>>> and one can hide it with a macro if necessary
>>> I hope we will get the syntax with C2Y and also:
>>>
>>> struct { int n; char buf[.n]; };
>>
>> Yes, this is definitely great if we can get this into the C standard.
>> Do you have any idea on when that might be possible?
>
> Hard to tell, but it is being worked on. Implementation
> experience in GCC would help.
Do you mean that GCC will implement this as an extension first?
Qing
>
> Martin
>
>>>
>>> In any case, I hope we get proper language integration and
>>> not a soup of attributes (although the attributes are a step
>>> up from not having bounds checking at all.).
>>
>> agreed.
>>>
>>>
>>> The problem in GCC is that the code for the access attributes
>>> that are internally used also for parameters with variably
>>> modified types is rather complicated and fragile, which makes
>>> it harder than necessary to add the missing pieces. I will
>>> probably try to simply add the .ACCESS_WITH_SIZE builtin
>>> in the FE to function parameters just like for 'counted_by'.
>>> Maybe this is already sufficient to make it work.
>>
>> As we discussed previously for .ACCESS_WITH_SIZE, IIRC,
>> for those current available attributes, “access”, and “alloc_size”,
>> if we implement them with .ACCESS_WITH_SIZE, we can make
>> them more robust.
>>
>> I remembered that there were some PRs filed to those issues,
>> Will find those PRs.
>>
>> Maybe it’s time to resolve this PRs as well. I will take a look here.
>
>>>
>>> In general I have the vague idea that at any point where an
>>> array decays or is adjusted into a pointer and size is then
>>> lost from the type, we could insert this builtin to make
>>> the size discoverable by BDOS later. A seamless handover
>>> from types to BDOS magic...
>>
>> Yeah, sounds reasonable to me.
Re: C runtime checking for assigment of VM types, v3
Am Montag, dem 15.07.2024 um 21:26 + schrieb Qing Zhao:
> Hi, Martin,
> I didn’t see your v3 patches attached to the email, did you miss them?
> (I really want to see them -:).
Sorry, I should have CCed you. It was sent as a series of patches:
https://gcc.gnu.org/pipermail/gcc-patches/2024-July/657254.html
>
> > On Jul 15, 2024, at 16:58, Martin Uecker wrote:
> >
> > Am Montag, dem 15.07.2024 um 13:05 -0700 schrieb Kees Cook:
> > > On Mon, Jul 15, 2024 at 07:20:31PM +0200, Martin Uecker wrote:
> > > > No, there are still two many missing pieces. The following
> > > > works already
> > > >
> > > > int h(int n, int buf[n])
> > > > {
> > > >return __builtin_dynamic_object_size(buf, 1);
> > > > }
> > >
> > > Yeah, this is nice.
> > >
> > > There are some interesting things happening around this general
> > > idea. Clang has the rather limited attributes "pass_object_size" and
> > > "pass_dynamic_object_size" that will work on function prototypes that
> > > will inform a _bos or _bdos internally, but you can only choose _one_
> > > type to apply to a given function parameter:
> > >
> > > size_t h(char * const __attribute__((pass_dynamic_object_size(1))) buf)
> > > {
> > > return __builtin_dynamic_object_size(buf, 1);
> > > }
> > >
> > > https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
> > >
> > > I have found it easier to just make wrapper macros, as that can allow
> > > both size types:
> > >
> > > size_t h(char *buf, const size_t p_max, const size_t p_min);
> > >
> > > #define h(p) \
> > > ({ \
> > > const size_t __p_max = __builtin_dynamic_object_size(p, 0); \
> > > const size_t __p_min = __builtin_dynamic_object_size(p, 1); \
> > > __h(p, __p_max, __p_min); \
> > > })
> > >
> > >
> > > But best is that it just gets handled automatically, which will be the
> > > goals of the more generalized "counted_by" (and "sized_by") attributes
> > > that will provide similar coverage as your example:
> > >
> > > size_t h(int * __attribute__((sized_by(bytes))) buf, int bytes)
> > > {
> > > return __builtin_dynamic_object_size(buf, 1);
> > > }
> > >
> > > https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
> >
> > Indeed, I already complained a lot to them about that design ;-)
> > >
> > > Those attributes end up being similar to what you have
> >
> > Well, the syntax is from C99, we just have to make it work.
> >
> > > only the explicit
> > > predeclaration isn't needed. i.e. to put "int n" in your example after
> > > "buf", it needs predeclaration:
> > >
> > > int h(int n; int buf[n], int n)
> > > {
> > > ...
> > > }
> > >
> > > (But Clang doesn't appear to support predeclarations.)
> >
> > And isn't this a lot nicer? It also follows the exact same
> > scoping rules of the language as everything else.
> >
> > In GCC the example above works also with this:
> >
> > int h(int n; char buf[n], int n)
> > {
> > return __builtin_dynamic_object_size(buf, 1);
> > }
> > https://godbolt.org/z/vfqoKaq7e
> >
> > and one can hide it with a macro if necessary
> > I hope we will get the syntax with C2Y and also:
> >
> > struct { int n; char buf[.n]; };
>
> Yes, this is definitely great if we can get this into the C standard.
> Do you have any idea on when that might be possible?
Hard to tell, but it is being worked on. Implementation
experience in GCC would help.
Martin
> >
> > In any case, I hope we get proper language integration and
> > not a soup of attributes (although the attributes are a step
> > up from not having bounds checking at all.).
>
> agreed.
> >
> >
> > The problem in GCC is that the code for the access attributes
> > that are internally used also for parameters with variably
> > modified types is rather complicated and fragile, which makes
> > it harder than necessary to add the missing pieces. I will
> > probably try to simply add the .ACCESS_WITH_SIZE builtin
> > in the FE to function parameters just like for 'counted_by'.
> > Maybe this is already sufficient to make it work.
>
> As we discussed previously for .ACCESS_WITH_SIZE, IIRC,
> for those current available attributes, “access”, and “alloc_size”,
> if we implement them with .ACCESS_WITH_SIZE, we can make
> them more robust.
>
> I remembered that there were some PRs filed to those issues,
> Will find those PRs.
>
> Maybe it’s time to resolve this PRs as well. I will take a look here.
> >
> > In general I have the vague idea that at any point where an
> > array decays or is adjusted into a pointer and size is then
> > lost from the type, we could insert this builtin to make
> > the size discoverable by BDOS later. A seamless handover
> > from types to BDOS magic...
>
> Yeah, sounds reasonable to me.
>
Re: C runtime checking for assigment of VM types, v3
Hi, Martin,
I didn’t see your v3 patches attached to the email, did you miss them? (I
really want to see them -:).
> On Jul 15, 2024, at 16:58, Martin Uecker wrote:
>
> Am Montag, dem 15.07.2024 um 13:05 -0700 schrieb Kees Cook:
>> On Mon, Jul 15, 2024 at 07:20:31PM +0200, Martin Uecker wrote:
>>> No, there are still two many missing pieces. The following
>>> works already
>>>
>>> int h(int n, int buf[n])
>>> {
>>>return __builtin_dynamic_object_size(buf, 1);
>>> }
>>
>> Yeah, this is nice.
>>
>> There are some interesting things happening around this general
>> idea. Clang has the rather limited attributes "pass_object_size" and
>> "pass_dynamic_object_size" that will work on function prototypes that
>> will inform a _bos or _bdos internally, but you can only choose _one_
>> type to apply to a given function parameter:
>>
>> size_t h(char * const __attribute__((pass_dynamic_object_size(1))) buf)
>> {
>> return __builtin_dynamic_object_size(buf, 1);
>> }
>>
>> https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
>>
>> I have found it easier to just make wrapper macros, as that can allow
>> both size types:
>>
>> size_t h(char *buf, const size_t p_max, const size_t p_min);
>>
>> #define h(p) \
>> ({ \
>> const size_t __p_max = __builtin_dynamic_object_size(p, 0); \
>> const size_t __p_min = __builtin_dynamic_object_size(p, 1); \
>> __h(p, __p_max, __p_min); \
>> })
>>
>>
>> But best is that it just gets handled automatically, which will be the
>> goals of the more generalized "counted_by" (and "sized_by") attributes
>> that will provide similar coverage as your example:
>>
>> size_t h(int * __attribute__((sized_by(bytes))) buf, int bytes)
>> {
>> return __builtin_dynamic_object_size(buf, 1);
>> }
>>
>> https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
>
> Indeed, I already complained a lot to them about that design ;-)
>>
>> Those attributes end up being similar to what you have
>
> Well, the syntax is from C99, we just have to make it work.
>
>> only the explicit
>> predeclaration isn't needed. i.e. to put "int n" in your example after
>> "buf", it needs predeclaration:
>>
>> int h(int n; int buf[n], int n)
>> {
>> ...
>> }
>>
>> (But Clang doesn't appear to support predeclarations.)
>
> And isn't this a lot nicer? It also follows the exact same
> scoping rules of the language as everything else.
>
> In GCC the example above works also with this:
>
> int h(int n; char buf[n], int n)
> {
> return __builtin_dynamic_object_size(buf, 1);
> }
> https://godbolt.org/z/vfqoKaq7e
>
> and one can hide it with a macro if necessary
> I hope we will get the syntax with C2Y and also:
>
> struct { int n; char buf[.n]; };
Yes, this is definitely great if we can get this into the C standard.
Do you have any idea on when that might be possible?
>
> In any case, I hope we get proper language integration and
> not a soup of attributes (although the attributes are a step
> up from not having bounds checking at all.).
agreed.
>
>
> The problem in GCC is that the code for the access attributes
> that are internally used also for parameters with variably
> modified types is rather complicated and fragile, which makes
> it harder than necessary to add the missing pieces. I will
> probably try to simply add the .ACCESS_WITH_SIZE builtin
> in the FE to function parameters just like for 'counted_by'.
> Maybe this is already sufficient to make it work.
As we discussed previously for .ACCESS_WITH_SIZE, IIRC,
for those current available attributes, “access”, and “alloc_size”,
if we implement them with .ACCESS_WITH_SIZE, we can make
them more robust.
I remembered that there were some PRs filed to those issues,
Will find those PRs.
Maybe it’s time to resolve this PRs as well. I will take a look here.
>
> In general I have the vague idea that at any point where an
> array decays or is adjusted into a pointer and size is then
> lost from the type, we could insert this builtin to make
> the size discoverable by BDOS later. A seamless handover
> from types to BDOS magic...
Yeah, sounds reasonable to me.
Qing
>
> Martin
Re: C runtime checking for assigment of VM types, v3
Am Montag, dem 15.07.2024 um 13:05 -0700 schrieb Kees Cook:
> On Mon, Jul 15, 2024 at 07:20:31PM +0200, Martin Uecker wrote:
> > No, there are still two many missing pieces. The following
> > works already
> >
> > int h(int n, int buf[n])
> > {
> > return __builtin_dynamic_object_size(buf, 1);
> > }
>
> Yeah, this is nice.
>
> There are some interesting things happening around this general
> idea. Clang has the rather limited attributes "pass_object_size" and
> "pass_dynamic_object_size" that will work on function prototypes that
> will inform a _bos or _bdos internally, but you can only choose _one_
> type to apply to a given function parameter:
>
> size_t h(char * const __attribute__((pass_dynamic_object_size(1))) buf)
> {
> return __builtin_dynamic_object_size(buf, 1);
> }
>
> https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
>
> I have found it easier to just make wrapper macros, as that can allow
> both size types:
>
> size_t h(char *buf, const size_t p_max, const size_t p_min);
>
> #define h(p) \
> ({\
> const size_t __p_max = __builtin_dynamic_object_size(p, 0); \
> const size_t __p_min = __builtin_dynamic_object_size(p, 1); \
> __h(p, __p_max, __p_min); \
> })
>
>
> But best is that it just gets handled automatically, which will be the
> goals of the more generalized "counted_by" (and "sized_by") attributes
> that will provide similar coverage as your example:
>
> size_t h(int * __attribute__((sized_by(bytes))) buf, int bytes)
> {
> return __builtin_dynamic_object_size(buf, 1);
> }
>
> https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
Indeed, I already complained a lot to them about that design ;-)
>
> Those attributes end up being similar to what you have
Well, the syntax is from C99, we just have to make it work.
> only the explicit
> predeclaration isn't needed. i.e. to put "int n" in your example after
> "buf", it needs predeclaration:
>
> int h(int n; int buf[n], int n)
> {
> ...
> }
>
> (But Clang doesn't appear to support predeclarations.)
And isn't this a lot nicer? It also follows the exact same
scoping rules of the language as everything else.
In GCC the example above works also with this:
int h(int n; char buf[n], int n)
{
return __builtin_dynamic_object_size(buf, 1);
}
https://godbolt.org/z/vfqoKaq7e
and one can hide it with a macro if necessary
I hope we will get the syntax with C2Y and also:
struct { int n; char buf[.n]; };
In any case, I hope we get proper language integration and
not a soup of attributes (although the attributes are a step
up from not having bounds checking at all.).
The problem in GCC is that the code for the access attributes
that are internally used also for parameters with variably
modified types is rather complicated and fragile, which makes
it harder than necessary to add the missing pieces. I will
probably try to simply add the .ACCESS_WITH_SIZE builtin
in the FE to function parameters just like for 'counted_by'.
Maybe this is already sufficient to make it work.
In general I have the vague idea that at any point where an
array decays or is adjusted into a pointer and size is then
lost from the type, we could insert this builtin to make
the size discoverable by BDOS later. A seamless handover
from types to BDOS magic...
Martin
>
Re: C runtime checking for assigment of VM types, v3
On Mon, Jul 15, 2024 at 07:20:31PM +0200, Martin Uecker wrote:
> No, there are still two many missing pieces. The following
> works already
>
> int h(int n, int buf[n])
> {
> return __builtin_dynamic_object_size(buf, 1);
> }
Yeah, this is nice.
There are some interesting things happening around this general
idea. Clang has the rather limited attributes "pass_object_size" and
"pass_dynamic_object_size" that will work on function prototypes that
will inform a _bos or _bdos internally, but you can only choose _one_
type to apply to a given function parameter:
size_t h(char * const __attribute__((pass_dynamic_object_size(1))) buf)
{
return __builtin_dynamic_object_size(buf, 1);
}
https://clang.llvm.org/docs/AttributeReference.html#pass-object-size-pass-dynamic-object-size
I have found it easier to just make wrapper macros, as that can allow
both size types:
size_t h(char *buf, const size_t p_max, const size_t p_min);
#define h(p)\
({ \
const size_t __p_max = __builtin_dynamic_object_size(p, 0); \
const size_t __p_min = __builtin_dynamic_object_size(p, 1); \
__h(p, __p_max, __p_min); \
})
But best is that it just gets handled automatically, which will be the
goals of the more generalized "counted_by" (and "sized_by") attributes
that will provide similar coverage as your example:
size_t h(int * __attribute__((sized_by(bytes))) buf, int bytes)
{
return __builtin_dynamic_object_size(buf, 1);
}
https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/
Those attributes end up being similar to what you have only the explicit
predeclaration isn't needed. i.e. to put "int n" in your example after
"buf", it needs predeclaration:
int h(int n; int buf[n], int n)
{
...
}
(But Clang doesn't appear to support predeclarations.)
--
Kees Cook
Re: C runtime checking for assigment of VM types, v3
Am Montag, dem 15.07.2024 um 09:45 -0700 schrieb Kees Cook:
> On Mon, Jul 15, 2024 at 09:19:49AM +0200, Martin Uecker wrote:
> > The instrumentation is guarded by a new instrumentation flag -fvla-bounds,
> > but runtime overhead should generally be very low as most checks are
> > removed by the optimizer, e.g.
> >
> > void foo(int x, char (*buf)[x])
> > {
> > bar(x, buf);
> > }
> >
> > does not have any overhead with -O1 (we also might want to filter out
> > some obvious cases already in the FE). So I think this flag could be
> > a good addition to -fhardened after some testing. Maybe it could even
> > be activated by default.
>
> Just to clarify, but does any of this change the behavior of
> __builtin_object_size() or __builtin_dynamic_object_size() within
> functions that take array arguments?
>
> i.e. does this work now?
>
> void foo(int array[10])
> {
> global = __builtin_object_size(array, 1);
> }
>
> (Currently "global" will be set to SIZE_MAX, rather than 40.)
>
No, there are still two many missing pieces. The following
works already
int h(int n, int buf[n])
{
return __builtin_dynamic_object_size(buf, 1);
}
but this doesn't:
int h(int n, int (*buf)[n])
{
return __builtin_dynamic_object_size(buf, 1);
}
because there could be multiple buf[n] (the subobject
version of BDOS also does not, not sure whether it is
supposed to). But then also
int h(int n, int buf[1][n])
{
return __builtin_dynamic_object_size(buf, 1);
}
sadly doesn't work.
For the latter two my patch would ensure that the
size of the 'n' matches when a pointer is passed, which
if BDOS would work, would be useful. But we will get
there eventually
Martin
Re: C runtime checking for assigment of VM types, v3
On Mon, Jul 15, 2024 at 09:19:49AM +0200, Martin Uecker wrote:
> The instrumentation is guarded by a new instrumentation flag -fvla-bounds,
> but runtime overhead should generally be very low as most checks are
> removed by the optimizer, e.g.
>
> void foo(int x, char (*buf)[x])
> {
> bar(x, buf);
> }
>
> does not have any overhead with -O1 (we also might want to filter out
> some obvious cases already in the FE). So I think this flag could be
> a good addition to -fhardened after some testing. Maybe it could even
> be activated by default.
Just to clarify, but does any of this change the behavior of
__builtin_object_size() or __builtin_dynamic_object_size() within
functions that take array arguments?
i.e. does this work now?
void foo(int array[10])
{
global = __builtin_object_size(array, 1);
}
(Currently "global" will be set to SIZE_MAX, rather than 40.)
--
Kees Cook
Re: C runtime checking for assigment of VM types, v3
correct email.
Am Montag, dem 15.07.2024 um 09:19 +0200 schrieb Martin Uecker:
> This is the third revision for my patch series to check bounds
> consistency at run-time when assigning VM types. Relative
> to the last version, mostly the tests were simplified and some
> coding style issues fixed.
>
>
> It adds a new code instrumentation option that inserts
> run-time checks to ensure bounds are matching. This helps
> with bounds safe programming and also finds problems in
> numerical code, e.g. when bound are swapped in
> multi-dimensional arrays
>
> void foo(int x, int y, double m[x][y]);
>
> double m[10][20];
> foo(20, 10, m);
>
> where currently do not get a warning or check.
>
> (After updating this patch series and testing it, it
> found a bug in new code added recently to my BART
> project - a numerical toolbox for image reconstruction and
> machine learning for magnetic resonance imaging.)
>
>
>
> The patches in the series do:
>
> 1. Checks simple assignments.
>
> 2. In addition checks function calls under the assumption
> that size expressions are declared as in the definition.
> This assumption goes beyond what ISO C guarantees (and is
> one reason this is not part of the UB sanitizer, the other
> is that there is no library support of this use case) but
> any inconsistency would usually indicate a bug anyway and
> we warn already by default with -Wvla-parameter
> (this now becomes really useful)
>
> 3. Checks function calls via pointers when possible.
>
> 4. Adds a warning if a function calls can not be
> instrumented e.g. because size expressions do not simply
> references to other parameters or variables. Also adds
> documentation for the warning and about instrumentation
> of function calls.
>
>
> The code is fairly simple and FE only as during recursive type
> checking in the comptypes family of function we can simply collect
> all pairs of size expressions that are supposed to match. This
> list is then further processed to emit simple checking code.
>
> For functions the main complication is that we need to evalute
> size expressions in the caller. We therefor only add
> checking if all size expressions direcly refer to other
> declarations, and simply give up for anything more complex.
> This already covers the most important use cases though.
>
>
> The code is also useful infrastructure for future compile-time
> warnings, e.g. the simply example above should clearly be
> diagnosed already compile time.
>
> I haven't implemented this yet, but this should be simple to add
> by detecting obvious cases during processing of the list of size
> expressions.
>
> Other current limitations are:
>
> The outermost bounds for functions parameters are not checked because
> they are lost when the type is adjusted to a pointer. The right semantics
> of checking those are also less obvious.
>
> As mentioned above, for functions we only check very simple size
> expressions that directly refer to a parameter or argument. It would
> be useful to extend this to more complex expressions without side
> effects, such 'n + 1' or maybe even 'n + m'. This would then cover
> most use cases in numerical code.
>
> A bounds violation just causes a run-time trap without error message.
> This is sufficient for safety and debugging with a debugger, but one
> consider adding a short error message. (This would have to go into
> libgcc I guess).
>
>
>
> The instrumentation is guarded by a new instrumentation flag -fvla-bounds,
> but runtime overhead should generally be very low as most checks are
> removed by the optimizer, e.g.
>
> void foo(int x, char (*buf)[x])
> {
> bar(x, buf);
> }
>
> does not have any overhead with -O1 (we also might want to filter out
> some obvious cases already in the FE). So I think this flag could be
> a good addition to -fhardened after some testing. Maybe it could even
> be activated by default.
>
>
> Finally, I am unsure about the use if signals in the tests. Maybe this
> is not supported everywhere? Any recommendation is much appreciated.
>
>
> Each patch was bootstrapped and regression tested on x86_64.
>
>
> Martin
>
>
>
>
>
>
>
>
>
>
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
[PATCH v3 2/4] c: runtime checking for assigment of VM types
Support instrumentation of function arguments for functions
called via a declaration. We can support only simple size
expressions without side effects, because the run-time
instrumentation is done before the call, but the expressions
are evaluated in the callee.
gcc/c:
* c-typeck.cc (process_vm_constraints): Add support
for instrumenting function arguments.
(convert_arguments): Instrument function arguments.
(convert_for_assigmnent): Adapt.
gcc/testsuide/gcc.dg:
* vla-bounds-func-1.c: Update.
* vla-bounds-func-2.c: New test.
* vla-bounds-func-3.c: New test.
* vla-bounds-func-4.c: New test.
* vla-bounds-func-5.c: New test.
* vla-bounds-func-6.c: New test.
* vla-bounds-func-7.c: New test.
* vla-bounds-func-8.c: New test.
* vla-bounds-func-9.c: New test.
* vla-bounds-func-10.c: New test.
* vla-bounds-func-11.c: New test.
* vla-bounds-func-12.c: New test.
* vla-bounds-func-13.c: New test.
* vla-bounds-func-14.c: New test.
* vla-bounds-func-15.c: New test.
* vla-bounds-func-16.c: New test.
* vla-bounds-func-17.c: New test.
* vla-bounds-func-18.c: New test.
* vla-bounds-func-19.c: New test.
* vla-bounds-func-20.c: New test.
---
gcc/c/c-typeck.cc | 151 +++---
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 2 +-
gcc/testsuite/gcc.dg/vla-bounds-func-10.c | 74 +++
gcc/testsuite/gcc.dg/vla-bounds-func-11.c | 28
gcc/testsuite/gcc.dg/vla-bounds-func-12.c | 29 +
gcc/testsuite/gcc.dg/vla-bounds-func-13.c | 37 ++
gcc/testsuite/gcc.dg/vla-bounds-func-14.c | 15 +++
gcc/testsuite/gcc.dg/vla-bounds-func-15.c | 20 +++
gcc/testsuite/gcc.dg/vla-bounds-func-16.c | 18 +++
gcc/testsuite/gcc.dg/vla-bounds-func-17.c | 18 +++
gcc/testsuite/gcc.dg/vla-bounds-func-18.c | 20 +++
gcc/testsuite/gcc.dg/vla-bounds-func-19.c | 21 +++
gcc/testsuite/gcc.dg/vla-bounds-func-2.c | 73 +++
gcc/testsuite/gcc.dg/vla-bounds-func-20.c | 22
gcc/testsuite/gcc.dg/vla-bounds-func-3.c | 28
gcc/testsuite/gcc.dg/vla-bounds-func-4.c | 37 ++
gcc/testsuite/gcc.dg/vla-bounds-func-5.c | 40 ++
gcc/testsuite/gcc.dg/vla-bounds-func-6.c | 36 ++
gcc/testsuite/gcc.dg/vla-bounds-func-7.c | 39 ++
gcc/testsuite/gcc.dg/vla-bounds-func-8.c | 38 ++
gcc/testsuite/gcc.dg/vla-bounds-func-9.c | 39 ++
21 files changed, 769 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-10.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-11.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-12.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-13.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-14.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-15.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-16.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-17.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-18.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-19.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-2.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-20.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-3.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-4.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-5.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-6.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-7.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-8.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-9.c
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 59940a29b53..c0132a22e21 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1201,19 +1201,13 @@ comptypes_verify (tree type1, tree type2)
/* Instrument assignment of variably modified types. */
static tree
-c_instrument_vm_assign (location_t loc, tree a, tree b)
+c_instrument_vm_assign (location_t loc, tree a, tree b, tree as, tree bs)
{
gcc_assert (flag_vla_bounds);
gcc_assert (TREE_CODE (a) == ARRAY_TYPE);
gcc_assert (TREE_CODE (b) == ARRAY_TYPE);
- tree as = TYPE_MAX_VALUE (TYPE_DOMAIN (a));
- tree bs = TYPE_MAX_VALUE (TYPE_DOMAIN (b));
-
- as = fold_build2 (PLUS_EXPR, sizetype, as, size_one_node);
- bs = fold_build2 (PLUS_EXPR, sizetype, bs, size_one_node);
-
tree t = build2 (NE_EXPR, boolean_type_node, as, bs);
tree tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP), 0);
@@ -3772,7 +3766,8 @@ static tree
convert_argument (location_t ploc, tree function, tree fundecl,
tree type, tree origtype, tree val, tree valtype,
bool npc, tree rname, int parmnum, int argnum,
- bool excess_precision, int warnopt)
+ bool excess_precision, int warnopt,
+ struct instrument_data **instr_vec)
{
/* Formal parm type is specified by a function prototype. */
@@ -3932,7 +3927,7 @@ convert_argument (location_t ploc, tree function, tree
fundecl,
val, origtype, ic_argpass,
npc, fundecl, function,
parmnum + 1, warnopt,
- NULL);
+ instr_vec);
if (targetm.calls.promote_prototypes (fundec
[PATCH v3 3/4] c: runtime checking for assigment of VM types
Support instrumentation of functions called via pointers. To do so,
record the declaration with the parameter types, so that it can be
retrieved later.
gcc/c:
c-decl.cc (get_parm_info): Record function declaration
for arguments.
c-typeck.cc (process_vm_constraints): Instrument functions
called via pointers.
gcc/testsuide/gcc.dg:
* vla-bounds-func-1.c: Add warning.
* vla-bounds-fnptr-1.c: New test.
* vla-bounds-fnptr-2.c: New test.
* vla-bounds-fnptr-3.c: New test.
---
gcc/c/c-decl.cc | 4
gcc/c/c-typeck.cc | 14 ++--
gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c | 24
gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c | 27 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c | 25 +
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 2 +-
6 files changed, 93 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 97f1d346835..d328e3bd5ac 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -8628,6 +8628,10 @@ get_parm_info (bool ellipsis, tree expr)
declared types. The back end may override this later. */
DECL_ARG_TYPE (decl) = type;
types = tree_cons (0, type, types);
+
+ /* Record the decl for use for VLA bounds checking. */
+ if (flag_vla_bounds)
+ TREE_PURPOSE (types) = decl;
}
break;
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index c0132a22e21..6ffa0c24e0c 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -3958,9 +3958,19 @@ process_vm_constraints (location_t location,
}
else
{
- /* Functions called via pointers are not yet supported. */
- return void_node;
+ while (TREE_CODE (function) != FUNCTION_TYPE)
+ function = TREE_TYPE (function);
+
+ args = TREE_PURPOSE (TYPE_ARG_TYPES (function));
+
+ if (!args)
+ {
+ /* FIXME: this can happen when forming composite types for the
+ conditional operator. */
+ return void_node;
+ }
}
+ gcc_assert (TREE_CODE (args) == PARM_DECL);
}
for (struct instrument_data* d = *instr_vec; d; d = d->next)
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
new file mode 100644
index 000..61ff0dff1db
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
@@ -0,0 +1,24 @@
+/* { dg-do run } */
+/* { dg-options "-fvla-bounds" } */
+
+#include
+#include
+
+static void handler(int) { exit(0); }
+
+void foo1(void (*p)(int n, char (*a)[n]))
+{
+ char A0[3];
+ (*p)(3, &A0);
+ (*p)(4, &A0); // 4 != 3
+ abort();
+}
+
+void b0(int n, char (*a)[n]) { }
+
+int main()
+{
+ signal(SIGILL, handler);
+
+ foo1(&b0);
+}
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
new file mode 100644
index 000..0c01d4592ed
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* { dg-options "-fvla-bounds" } */
+
+#include
+#include
+
+static void handler(int) { exit(0); }
+
+int n;
+
+void foo2(void (*p)(int n, char (*a)[n]))
+{
+ n = 4;
+ char A0[3];
+ (*p)(3, &A0);
+ (*p)(4, &A0);
+ abort();
+}
+
+void b1(int n0, char (*a)[n]) { }
+
+int main()
+{
+ signal(SIGILL, handler);
+
+ foo2(&b1); // we should diagnose mismatch
+}
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c
b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c
new file mode 100644
index 000..c239216cdfc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c
@@ -0,0 +1,25 @@
+/* { dg-do run } */
+/* { dg-options "-fvla-bounds" } */
+
+#include
+#include
+
+static void handler(int) { exit(0); }
+
+int n;
+
+void foo3(void (*p)(int n0, char (*a)[n]))
+{
+ n = 4;
+ char A0[3];
+ (*p)(3, &A0); // 4 != 3
+ abort();
+}
+
+void b1(int n0, char (*a)[n]) { }
+
+int main()
+{
+ signal(SIGILL, handler);
+ foo3(&b1);
+}
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
index 378c6073688..36072a372a3 100644
--- a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
+++ b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
@@ -30,7 +30,7 @@ void f(void)
int u = 3; int v = 4;
char a[u][v];
- (1 ? f1 : f2)(u, v, a);
+ (1 ? f1 : f2)(u, v, a); /* "Function call not instrumented." */
}
/* size expression in parameter */
--
2.39.2
[PATCH v3 4/4] c: runtime checking for assigment of VM types
Add warning for the case when a function call can not be instrumented
and add documentation for instrumentation of function calls.
gcc/c-family/:
* c.opt (Wvla-parameter-missing-check): Add warning.
gcc/c/:
* c-typeck.cc (process_vm_constraints): Add warning.
gcc/doc/:
* invoke.texi (Wvla-parameter-missing-check): Document warning.
(flag_vla_bounds): Update.
gcc/testsuite/:
* gcc.dg/vla-bounds-func-1.c: Add warning.
---
gcc/c-family/c.opt | 5 +
gcc/c/c-typeck.cc | 4
gcc/doc/invoke.texi | 17 ++---
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 6 +++---
4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 786cd4ce52a..7070ff767bf 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1542,6 +1542,11 @@ Wvla-parameter
C ObjC C++ ObjC++ Var(warn_vla_parameter) Warning LangEnabledBy(C ObjC C++
ObjC++,Wall)
Warn about mismatched declarations of VLA parameters.
+Wvla-parameter-missing-check
+C ObjC Var(warn_vla_parameter_check) Warning Init(0)
+When using run-time checking of VLA bounds, warn about function calls which
+could not be instrumented.
+
Wvolatile
C++ ObjC++ Var(warn_volatile) Warning
Warn about deprecated uses of volatile qualifier.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 6ffa0c24e0c..3f380761efd 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -3967,6 +3967,8 @@ process_vm_constraints (location_t location,
{
/* FIXME: this can happen when forming composite types for the
conditional operator. */
+ warning_at (location, OPT_Wvla_parameter_missing_check,
+ "Function call not instrumented");
return void_node;
}
}
@@ -4050,6 +4052,8 @@ process_vm_constraints (location_t location,
also not instrument any of the others because it may have
side effects affecting them. (We could restart and instrument
only the ones with integer constants.) */
+ warning_at (location, OPT_Wvla_parameter_missing_check,
+ "Function call not instrumented");
return void_node;
}
cont:
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8d6d68e4f27..510d29735a7 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10699,6 +10699,13 @@ void g (int n)
involving ordinary array arguments.
+@opindex Wvla-parameter-missing-check
+@item -Wvla-parameter-missing-check
+Warn when function calls can not be instrumented with the use of
+@option{-fvla-bounds} to detect inconsistencies between the bounds of
+VLA parameters at runtime.
+
+
@opindex Wvolatile-register-var
@opindex Wno-volatile-register-var
@item -Wvolatile-register-var
@@ -20836,9 +20843,13 @@ The @var{string} should be different for every file
you compile.
@item -fvla-bounds
This option is only available when compiling C code. If activated,
additional code is emitted that verifies at run time for assignments
-involving variably-modified types that corresponding size expressions
-evaluate to the same value.
-
+and function calls involving variably-modified types that corresponding
+size expressions evaluate to the same value. Note that for function
+calls the visible declarations needs to have a size expression that
+matches the size expression in the definition. A mismatch seen by the
+the compiler is diagnosed by @option{-Wvla-parameter}). In same cases,
+a function call can not be instrumented. This can be diagnosed by
+@option{-Wvla-parameter-missing-check}.
@opindex save-temps
@item -save-temps
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
index 36072a372a3..b7a994a80b7 100644
--- a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
+++ b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-fvla-bounds" } */
+/* { dg-options "-fvla-bounds -Wvla-parameter-missing-check" } */
// make sure we do not ICE on any of these
@@ -30,7 +30,7 @@ void f(void)
int u = 3; int v = 4;
char a[u][v];
- (1 ? f1 : f2)(u, v, a); /* "Function call not instrumented." */
+ (1 ? f1 : f2)(u, v, a); /* { dg-warning "Function call" "not instrumented." }
*/
}
/* size expression in parameter */
@@ -50,5 +50,5 @@ int c(int u, char (*a)[u]) { }
int d(void)
{
char a[3];
- c(3, &a); /* "Function call not instrumented." */
+ c(3, &a); /* { dg-warning "Function call" "not instrumented." } */
}
--
2.39.2
[PATCH v3 1/4] c: runtime checking for assigment of VM types
When checking compatibility of types during assignment, collect all pairs of types where the outermost bound needs to match at run-time. This list is then processed to add runtime checks for each bound. gcc/c-family: * c-opt (fvla-bounds): New flag. gcc/c: * c-typeck.cc (struct instrument_data): New structure. (comp_target_types_instr convert_for_assignment_instrument): New interfaces for existing functions. (struct comptypes_data): Add instrumentation. (comptypes_check_enum_int_intr): New interface. (comptypes_check_enum_int): Old interface (calls new). (comptypes_internal): Collect VLA types needed for UBSan. (comp_target_types_instr): New interface. (comp_target_types): Old interface (calls new). (function_types_compatible_p): No instrumentation for function arguments. (process_vm_constraints): New function. (convert_argument): Adapt. (convert_for_assignment_instrument): New interface. (convert_for_assignment): Instrument assignments. (c_instrument_vm_assign): Helper function. (process_vm_constraints): Helper function. gcc/doc/: * invoke.texi (fvla-bounds): Document new flag. gcc/testsuite: * gcc.dg/vla-bounds-1.c: New test. * gcc.dg/vla-bounds-assign-1.c: New test. * gcc.dg/vla-bounds-assign-2.c: New test. * gcc.dg/vla-bounds-assign-3.c: New test. * gcc.dg/vla-bounds-assign-4.c: New test. * gcc.dg/vla-bounds-func-1.c: New test. * gcc.dg/vla-bounds-init-1.c: New test. * gcc.dg/vla-bounds-init-2.c: New test. * gcc.dg/vla-bounds-init-3.c: New test. * gcc.dg/vla-bounds-init-4.c: New test. * gcc.dg/vla-bounds-nest-1.c: New test. * gcc.dg/vla-bounds-nest-2.c: New test. * gcc.dg/vla-bounds-ret-1.c: New test. * gcc.dg/vla-bounds-ret-2.c: New test. --- gcc/c-family/c.opt | 4 + gcc/c/c-typeck.cc | 171 ++--- gcc/doc/invoke.texi | 9 ++ gcc/testsuite/gcc.dg/vla-bounds-1.c | 74 + gcc/testsuite/gcc.dg/vla-bounds-assign-1.c | 32 gcc/testsuite/gcc.dg/vla-bounds-assign-2.c | 30 gcc/testsuite/gcc.dg/vla-bounds-assign-3.c | 43 ++ gcc/testsuite/gcc.dg/vla-bounds-assign-4.c | 26 gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 54 +++ gcc/testsuite/gcc.dg/vla-bounds-init-1.c | 26 gcc/testsuite/gcc.dg/vla-bounds-init-2.c | 25 +++ gcc/testsuite/gcc.dg/vla-bounds-init-3.c | 25 +++ gcc/testsuite/gcc.dg/vla-bounds-init-4.c | 27 gcc/testsuite/gcc.dg/vla-bounds-nest-1.c | 31 gcc/testsuite/gcc.dg/vla-bounds-nest-2.c | 26 gcc/testsuite/gcc.dg/vla-bounds-ret-1.c | 27 gcc/testsuite/gcc.dg/vla-bounds-ret-2.c | 28 17 files changed, 639 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-3.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-4.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-3.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-4.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-nest-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-nest-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-ret-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-ret-2.c diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index a52682d835c..786cd4ce52a 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -2324,6 +2324,10 @@ fvisibility-ms-compat C++ ObjC++ Var(flag_visibility_ms_compat) Changes visibility to match Microsoft Visual Studio by default. +fvla-bounds +C Var(flag_vla_bounds) +Emit run-time consistency checks for variably-modified types. + fvtable-gc C++ ObjC++ WarnRemoved No longer supported. diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 7e0f01ed22b..59940a29b53 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -97,11 +97,12 @@ static tree qualify_type (tree, tree); struct comptypes_data; static bool tagged_types_tu_compatible_p (const_tree, const_tree, struct comptypes_data *); -static bool comp_target_types (location_t, tree, tree); static bool function_types_compatible_p (const_tree, const_tree, struct comptypes_data *); static bool type_lists_compatible_p (const_tree, const_tree, struct comptypes_data *); +static bool comp_target_types_instr (location_t, tree, tree, + struct instrument_data **); static int convert_arguments (location_t, vec, tree, vec *, vec *, tree, tree); @@ -109,6 +110,9 @@ static tree pointer_diff (location_t, tree, tree, tree *); static tree convert_for_assignment (location_t, location_t, tree, tree, tree, enum impl_conv, bool, tree, tree, int, int = 0); +static tree convert_for_assignment_instrument (location_t, location_t, tree, tree, tree, + enum impl_conv, bool, tree, tree, int, int, + struct
C runtime checking for assigment of VM types, v3
This is the third revision for my patch series to check bounds
consistency at run-time when assigning VM types. Relative
to the last version, mostly the tests were simplified and some
coding style issues fixed.
It adds a new code instrumentation option that inserts
run-time checks to ensure bounds are matching. This helps
with bounds safe programming and also finds problems in
numerical code, e.g. when bound are swapped in
multi-dimensional arrays
void foo(int x, int y, double m[x][y]);
double m[10][20];
foo(20, 10, m);
where currently do not get a warning or check.
(After updating this patch series and testing it, it
found a bug in new code added recently to my BART
project - a numerical toolbox for image reconstruction and
machine learning for magnetic resonance imaging.)
The patches in the series do:
1. Checks simple assignments.
2. In addition checks function calls under the assumption
that size expressions are declared as in the definition.
This assumption goes beyond what ISO C guarantees (and is
one reason this is not part of the UB sanitizer, the other
is that there is no library support of this use case) but
any inconsistency would usually indicate a bug anyway and
we warn already by default with -Wvla-parameter
(this now becomes really useful)
3. Checks function calls via pointers when possible.
4. Adds a warning if a function calls can not be
instrumented e.g. because size expressions do not simply
references to other parameters or variables. Also adds
documentation for the warning and about instrumentation
of function calls.
The code is fairly simple and FE only as during recursive type
checking in the comptypes family of function we can simply collect
all pairs of size expressions that are supposed to match. This
list is then further processed to emit simple checking code.
For functions the main complication is that we need to evalute
size expressions in the caller. We therefor only add
checking if all size expressions direcly refer to other
declarations, and simply give up for anything more complex.
This already covers the most important use cases though.
The code is also useful infrastructure for future compile-time
warnings, e.g. the simply example above should clearly be
diagnosed already compile time.
I haven't implemented this yet, but this should be simple to add
by detecting obvious cases during processing of the list of size
expressions.
Other current limitations are:
The outermost bounds for functions parameters are not checked because
they are lost when the type is adjusted to a pointer. The right semantics
of checking those are also less obvious.
As mentioned above, for functions we only check very simple size
expressions that directly refer to a parameter or argument. It would
be useful to extend this to more complex expressions without side
effects, such 'n + 1' or maybe even 'n + m'. This would then cover
most use cases in numerical code.
A bounds violation just causes a run-time trap without error message.
This is sufficient for safety and debugging with a debugger, but one
consider adding a short error message. (This would have to go into
libgcc I guess).
The instrumentation is guarded by a new instrumentation flag -fvla-bounds,
but runtime overhead should generally be very low as most checks are
removed by the optimizer, e.g.
void foo(int x, char (*buf)[x])
{
bar(x, buf);
}
does not have any overhead with -O1 (we also might want to filter out
some obvious cases already in the FE). So I think this flag could be
a good addition to -fhardened after some testing. Maybe it could even
be activated by default.
Finally, I am unsure about the use if signals in the tests. Maybe this
is not supported everywhere? Any recommendation is much appreciated.
Each patch was bootstrapped and regression tested on x86_64.
Martin
[PATCH 4/4] c: runtime checking for assigment of VM types 4/4
Add warning for the case when a function call can not be instrumened.
gcc/c-family/:
* c.opt (Wvla-parameter-missing-check): Add warning.
gcc/c/:
* c-typeck.cc (process_vm_constraints): Add warning.
gcc/doc/:
* invoke.texi (Wvla-parameter-missing-check): Document warning.
(flag_vla_bounds): Update.
gcc/testsuite/:
* gcc.dg/vla-bounds-func-1.c: Add warning.
---
gcc/c-family/c.opt | 5 +
gcc/c/c-typeck.cc| 4
gcc/doc/invoke.texi | 11 ---
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 6 +++---
4 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 29bc0956181..bd45ba577bd 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1485,6 +1485,11 @@ Wvla-parameter
C ObjC C++ ObjC++ Var(warn_vla_parameter) Warning LangEnabledBy(C ObjC C++
ObjC++,Wall)
Warn about mismatched declarations of VLA parameters.
+Wvla-parameter-missing-check
+C ObjC Var(warn_vla_parameter_check) Warning Init(0)
+When using run-time checking of VLA bounds, warn about function calls which
+could not be instrumented.
+
Wvolatile
C++ ObjC++ Var(warn_volatile) Warning
Warn about deprecated uses of volatile qualifier.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 1200abc2f4a..a4fb0a6b527 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -3481,6 +3481,8 @@ process_vm_constraints (location_t location,
{
/* FIXME: this can happen when forming composite types for the
conditional operator. */
+ warning_at (location, OPT_Wvla_parameter_missing_check,
+ "Function call not instrumented");
return void_node;
}
}
@@ -3564,6 +3566,8 @@ process_vm_constraints (location_t location,
also not instrument any of the others because it may have
side effects affecting them. (We could restart and instrument
only the ones with integer constants.) */
+ warning_at (location, OPT_Wvla_parameter_missing_check,
+ "Function call not instrumented");
return void_node;
}
cont:
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index c94ca59086b..6f4bbd43919 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -10269,6 +10269,7 @@ void g (int n)
@option{-Warray-parameter} option triggers warnings for similar problems
involving ordinary array arguments.
+
@opindex Wvla-parameter-missing-check
@item -Wvla-parameter-missing-check
Warn when function calls can not be instrumented with the use of
@@ -20063,9 +20064,13 @@ The @var{string} should be different for every file
you compile.
@item -fvla-bounds
This option is only available when compiling C code. If activated,
additional code is emitted that verifies at run time for assignments
-involving variably-modified types that corresponding size expressions
-evaluate to the same value.
-
+and function calls involving variably-modified types that corresponding
+size expressions evaluate to the same value. Note that for function
+calls the visible declarations needs to have a size expression that
+matches the size expression in the definition. A mismatch seen by the
+the compiler is diagnosed by @option{-Wvla-parameter}). In same cases,
+a function call can not be instrumented. This can be diagnosed by
+@option{-Wvla-parameter-missing-check}.
@opindex save-temps
@item -save-temps
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
index 72dba39107b..205e5174185 100644
--- a/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
+++ b/gcc/testsuite/gcc.dg/vla-bounds-func-1.c
@@ -1,5 +1,5 @@
/* { dg-do compile } */
-/* { dg-options "-fvla-bounds" } */
+/* { dg-options "-fvla-bounds -Wvla-parameter-missing-check" } */
// make sure we do not ICE on any of these
@@ -31,7 +31,7 @@ void f(void)
int u = 3; int v = 4;
char a[u][v];
- (1 ? f1 : f2)(u, v, a); /* "Function call not instrumented." */
+ (1 ? f1 : f2)(u, v, a); /* { dg-warning "Function call" "not
instrumented." } */
}
/* size expression in parameter */
@@ -51,6 +51,6 @@ int c(int u, char (*a)[u]) { }
int d(void)
{
char a[3];
- c(3, &a); /* "Function call not instrumented." */
+ c(3, &a); /* { dg-warning "Function call" "not
instrumented." } */
}
--
2.39.2
[PATCH 3/4] c: runtime checking for assigment of VM types 3/4
Support instrumentation of functions called via pointers. To do so,
record the declaration with the parameter types, so that it can be
retrieved later.
gcc/c:
c-decl.cc (get_parm_info): Record function declaration
for arguments.
c-typeck.cc (process_vm_constraints): Instrument functions
called via pointers.
gcc/testsuide/gcc.dg:
* vla-bounds-func-1.c: Add warning.
* vla-bounds-fnptr.c: New test.
* vla-bounds-fnptr-1.c: New test.
* vla-bounds-fnptr-2.c: New test.
* vla-bounds-fnptr-3.c: New test.
* vla-bounds-fnptr-4.c: New test.
* vla-bounds-fnptr-5.c: New test.
---
gcc/c/c-decl.cc | 4 ++
gcc/c/c-typeck.cc | 14 +++-
gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr-4.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr-5.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-fnptr.c | 78 +++
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 2 +-
9 files changed, 485 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-3.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-4.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr-5.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-fnptr.c
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 64d3a941cb9..84a30f7476a 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -8549,6 +8549,10 @@ get_parm_info (bool ellipsis, tree expr)
declared types. The back end may override this later. */
DECL_ARG_TYPE (decl) = type;
types = tree_cons (0, type, types);
+
+ /* Record the decl for use for VLA bounds checking. */
+ if (flag_vla_bounds)
+ TREE_PURPOSE (types) = decl;
}
break;
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index b65fc450940..1200abc2f4a 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -3472,9 +3472,19 @@ process_vm_constraints (location_t location,
}
else
{
- /* Functions called via pointers are not yet supported. */
- return void_node;
+ while (FUNCTION_TYPE != TREE_CODE (function))
+ function = TREE_TYPE (function);
+
+ args = TREE_PURPOSE (TYPE_ARG_TYPES (function));
+
+ if (!args)
+ {
+ /* FIXME: this can happen when forming composite types for the
+conditional operator. */
+ return void_node;
+ }
}
+ gcc_assert (PARM_DECL == TREE_CODE (args));
}
for (struct instrument_data* d = *instr_vec; d; d = d->next)
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
new file mode 100644
index 000..b9af87f6338
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-1.c
@@ -0,0 +1,78 @@
+/* { dg-do run } */
+/* { dg-options "-fvla-bounds" } */
+
+#include
+#include
+
+static void handler(int) { exit(0); }
+
+#define TRY(...) __VA_ARGS__ __builtin_abort();
+#define ERROR(...)
+
+
+
+void foo1(void (*p)(int n, char (*a)[n]))
+{
+ char A0[3];
+ (*p)(3, &A0);
+TRY( (*p)(4, &A0); ) // 4 != 3
+}
+
+void b0(int n, char (*a)[n]) { }
+
+
+int n;
+
+void foo2(void (*p)(int n, char (*a)[n]))
+{
+ n = 4;
+ char A0[3];
+ (*p)(3, &A0);
+ERROR( (*p)(4, &A0); ) // 4 != 3
+}
+
+void foo3(void (*p)(int n0, char (*a)[n]))
+{
+ n = 4;
+ char A0[3];
+ERROR( (*p)(3, &A0); ) // 4 != 3
+ERROR( (*p)(4, &A0); ) // 4 != 3
+}
+
+void foo4(void (*p)(int n, char (*a)[n]))
+{
+ n = 3;
+ char A0[3];
+ (*p)(3, &A0);
+ERROR( (*p)(4, &A0); ) // 4 != 3
+}
+
+
+void foo5(void (*p)(int n0, char (*a)[n]))
+{
+ n = 3;
+ char A0[3];
+ (*p)(3, &A0);
+ (*p)(4, &A0);
+}
+
+
+void b1(int n0, char (*a)[n]) { }
+
+
+
+int main()
+{
+ signal(SIGILL, handler);
+
+ foo1(&b0);
+
+ foo2(&b1);
+ foo3(&b1); // we should diagnose mismatch and run-time discrepancies
+
+ foo4(&b1);
+ foo5(&b1); // we should diagnose mismatch and run-time discrepancies
+}
+
+
+
diff --git a/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
new file mode 100644
index 000..4ec326af06c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vla-bounds-fnptr-2.c
@@ -0,0 +1,78 @@
+/* { dg-do run } */
+/* { dg-options "-fvla-bounds" } */
+
+#include
+#include
+
+static void handler(int) { exit(0); }
+
+#define TRY(...) __VA_ARGS__ __builtin_abort();
+#define ERROR(...)
+
+
+
+void foo1(voi
[PATCH 2/4] c: runtime checking for assigment of VM types 2/4
Support instrumentation of function arguments for functions
called via a declaration. We can support only simple size
expressions without side effects, because the run-time
instrumentation is done before the call, but the expressions
are evaluated in the callee.
gcc/c:
* c-typeck.cc (process_vm_constraints): Add support
for instrumenting function arguments.
(convert_arguments): Instrument function arguments.
(convert_for_assigmnent): Adapt.
gcc/testsuide/gcc.dg:
* vla-bounds-func-1.c: Update.
* vla-bounds-func-2.c: New test.
* vla-bounds-func-3.c: New test.
* vla-bounds-func-4.c: New test.
* vla-bounds-func-5.c: New test.
* vla-bounds-func-6.c: New test.
* vla-bounds-func-7.c: New test.
* vla-bounds-func-8.c: New test.
* vla-bounds-func-9.c: New test.
* vla-bounds-func-10.c: New test.
* vla-bounds-func-11.c: New test.
* vla-bounds-func-12.c: New test.
* vla-bounds-func-13.c: New test.
* vla-bounds-func-14.c: New test.
* vla-bounds-func-15.c: New test.
* vla-bounds-func-16.c: New test.
* vla-bounds-func-17.c: New test.
* vla-bounds-func-18.c: New test.
* vla-bounds-func-19.c: New test.
* vla-bounds-func-20.c: New test.
---
gcc/c/c-typeck.cc | 151 +++---
gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 4 +-
gcc/testsuite/gcc.dg/vla-bounds-func-10.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-11.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-12.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-13.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-14.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-15.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-16.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-17.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-18.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-19.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-2.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-20.c | 45 +++
gcc/testsuite/gcc.dg/vla-bounds-func-3.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-4.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-5.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-6.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-7.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-8.c | 99 ++
gcc/testsuite/gcc.dg/vla-bounds-func-9.c | 99 ++
21 files changed, 1641 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-10.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-11.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-12.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-13.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-14.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-15.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-16.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-17.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-18.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-19.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-2.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-20.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-3.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-4.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-5.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-6.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-7.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-8.c
create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-9.c
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index cb5887b6255..b65fc450940 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1067,19 +1067,13 @@ common_type (tree t1, tree t2)
/* Instrument assignment of variably modified types. */
static tree
-c_instrument_vm_assign (location_t loc, tree a, tree b)
+c_instrument_vm_assign (location_t loc, tree a, tree b, tree as, tree bs)
{
gcc_assert (flag_vla_bounds);
gcc_assert (TREE_CODE (a) == ARRAY_TYPE);
gcc_assert (TREE_CODE (b) == ARRAY_TYPE);
- tree as = TYPE_MAX_VALUE (TYPE_DOMAIN (a));
- tree bs = TYPE_MAX_VALUE (TYPE_DOMAIN (b));
-
- as = fold_build2 (PLUS_EXPR, sizetype, as, size_one_node);
- bs = fold_build2 (PLUS_EXPR, sizetype, bs, size_one_node);
-
tree t = build2 (NE_EXPR, boolean_type_node, as, bs);
tree tt = build_call_expr_loc (loc, builtin_decl_explicit (BUILT_IN_TRAP),
0);
@@ -3286,7 +3280,8 @@ static tree
convert_argument (location_t ploc, tree function, tree fundecl,
tree type, tree origtype, tree val, tree valtype,
bool npc, tree rname, int parmnum, int argnum,
- boo
[PATCH 1/4] c: runtime checking for assigment of VM types 1/4
When checking compatibility of types during assignment, collect all pairs of types where the outermost bound needs to match at run-time. This list is then processed to add runtime checks for each bound. gcc/c-family: * c-opt (fvla-bounds): New flag. gcc/c: * c-typeck.cc (struct instrument_data): New structure. (comp_target_types_instr convert_for_assignment_instrument): New interfaces for existing functions. (struct comptypes_data): Add instrumentation. (comptypes_check_enum_int_intr): New interface. (comptypes_check_enum_int): Old interface (calls new). (comptypes_internal): Collect VLA types needed for UBSan. (comp_target_types_instr): New interface. (comp_target_types): Old interface (calls new). (function_types_compatible_p): No instrumentation for function arguments. (process_vm_constraints): New function. (convert_argument): Adapt. (convert_for_assignment_instrument): New interface. (convert_for_assignment): Instrument assignments. (c_instrument_vm_assign): Helper function. (process_vm_constraints): Helper function. gcc/doc/: * invoke.texi (fvla-bounds): Document new flag. gcc/testsuite: * gcc.dg/vla-bounds-1.c: New test. * gcc.dg/vla-bounds-assign-1.c: New test. * gcc.dg/vla-bounds-assign-2.c: New test. * gcc.dg/vla-bounds-assign-3.c: New test. * gcc.dg/vla-bounds-assign-4.c: New test. * gcc.dg/vla-bounds-func-1.c: New test. * gcc.dg/vla-bounds-init-1.c: New test. * gcc.dg/vla-bounds-init-2.c: New test. * gcc.dg/vla-bounds-init-3.c: New test. * gcc.dg/vla-bounds-init-4.c: New test. * gcc.dg/vla-bounds-nest-1.c: New test. * gcc.dg/vla-bounds-nest-2.c: New test. * gcc.dg/vla-bounds-ret-1.c: New test. * gcc.dg/vla-bounds-ret-2.c: New test. --- gcc/c-family/c.opt | 4 + gcc/c/c-typeck.cc | 171 ++--- gcc/doc/invoke.texi| 15 ++ gcc/testsuite/gcc.dg/vla-bounds-1.c| 85 ++ gcc/testsuite/gcc.dg/vla-bounds-assign-1.c | 126 +++ gcc/testsuite/gcc.dg/vla-bounds-assign-2.c | 126 +++ gcc/testsuite/gcc.dg/vla-bounds-assign-3.c | 126 +++ gcc/testsuite/gcc.dg/vla-bounds-assign-4.c | 133 gcc/testsuite/gcc.dg/vla-bounds-func-1.c | 56 +++ gcc/testsuite/gcc.dg/vla-bounds-init-1.c | 125 +++ gcc/testsuite/gcc.dg/vla-bounds-init-2.c | 125 +++ gcc/testsuite/gcc.dg/vla-bounds-init-3.c | 126 +++ gcc/testsuite/gcc.dg/vla-bounds-init-4.c | 125 +++ gcc/testsuite/gcc.dg/vla-bounds-nest-1.c | 39 + gcc/testsuite/gcc.dg/vla-bounds-nest-2.c | 33 gcc/testsuite/gcc.dg/vla-bounds-ret-1.c| 132 gcc/testsuite/gcc.dg/vla-bounds-ret-2.c| 133 17 files changed, 1661 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-3.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-assign-4.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-func-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-3.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-init-4.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-nest-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-nest-2.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-ret-1.c create mode 100644 gcc/testsuite/gcc.dg/vla-bounds-ret-2.c diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index b10c6057cd1..29bc0956181 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -2280,6 +2280,10 @@ fvisibility-ms-compat C++ ObjC++ Var(flag_visibility_ms_compat) Changes visibility to match Microsoft Visual Studio by default. +fvla-bounds +C Var(flag_vla_bounds) +Emit run-time consistency checks for variably-modified types. + fvtable-gc C++ ObjC++ WarnRemoved No longer supported. diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc index 1dbb4471a88..cb5887b6255 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -93,11 +93,13 @@ static tree qualify_type (tree, tree); struct comptypes_data; static bool tagged_types_tu_compatible_p (const_tree, const_tree, struct comptypes_data *); -static bool comp_target_types (location_t, tree, tree); static bool function_types_compatible_p (const_tree, const_tree, struct comptypes_data *); static bool type_lists_compatible_p (const_tree, const_tree,
C runtime checking for assigment of VM types
This is another revised series for checking for bounds consistency when assigning VM types. Based on feedback, I disentangled this from UBSan for a three reasons: - I think it makes sense as a stand-alone feature similar to other run-time instrumentation features GCC already has. - Not all checks are strictly speaking for UB, i.e. it triggers for strictly conforming code which has inconsistent bounds. For this feature, it makes sense to assume that bounds are correct (and GCC warns about inconsistently declared bounds by default already for a while). - So far, there is no upstream support in libubsan which we could use. Bootstrapped and regression tested on x86_64. Martin
