This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] ubsan: remove bogus check for flexible array members


On Wed, Feb 25, 2015 at 10:01:07PM -0800, Martin Uecker wrote:
> this patch removes a bogus check for flexible array members
> which prevents array references to be instrumented in some
> interesting cases. Arrays accessed through pointers are now
> instrumented correctly.
> 
> The check was unnecessary because flexible arrays are not 
> instrumented anyway because of
> TYPE_MAX_VALUE (domain) == NULL_TREE. 

No, it is not bogus nor unnecessary.
This isn't about just real flexible arrays, but similar constructs,
C++ doesn't have flexible array members, nor C89, so people use the
GNU extension of struct S { ... ; char a[0]; } instead, or
use char a[1]; as the last member and still expect to be able to access
s->a[i] for i > 0 say on heap allocations etc.

I think we were talking at some point about having a strict-bounds or
something similar that would accept only real flexible array members and
nothing else and be more pedantic, at the disadvantage of diagnosing tons
of real-world code that is supported by GCC.

Perhaps if the reference is just an array, not a struct containing
flexible-array-member-like array, we could consider it strict always,
but that is certainly not what your patch does.

>         * c-family/c-ubsan.c (ubsan_instrument_bounds): Remove

c-family/ shouldn't be in the ChangeLog entry, instead that part should go
into c-family/ChangeLog.
> --- a/gcc/c-family/c-ubsan.c
> +++ b/gcc/c-family/c-ubsan.c
> @@ -293,6 +293,7 @@ ubsan_instrument_bounds (location_t loc, tree array, tree *index,
>    tree type = TREE_TYPE (array);
>    tree domain = TYPE_DOMAIN (type);
>  
> +  /* This also takes care of flexilbe array members */

Typo.

> +#ifdef __cplusplus
> +extern "C" void* malloc(unsigned long x);
> +#else
> +extern void* malloc(unsigned long x);
> +#endif

Why are you declaring malloc (wrongly), when it isn't used?
Malloc argument is size_t aka __SIZE_TYPE__, not unsigned long.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]