[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #8 from Andi Kleen  ---
__builtin_constant_p does not cover variable range information, which is what
we're looking for here to prevent security bugs.

Also in my experience these explicit expressions tend to be somewhat fragile
and is not well specified.  It has to assume that the optimizer does specific
operations which are nowhere guaranteed.

An explicit builtin could be much tighter defined.

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #7 from Alexander Monakov  ---
This sounds like a separate problem that is solvable via __builtin_constant_p? 
For example:

void link_error(void) __attribute__((error("size check failed")));

if (__builtin_constant_p(size) && size-0ul >= MAX_ALLOC_SIZE)
  link_error();

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #6 from Andi Kleen  ---
In the kernel there is also an upper limit on allocations.

Perhaps just a generic assert builtin that:
- uses value range information
- uses constant propagation
- is a nop when the compiler doesn't have either of this available
- otherwise warns at build time

__builtin_compile_assert(size >= 0 && size < MAX_ALLOC_SIZE);

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[amonakov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

Alexander Monakov  changed:

   What|Removed |Added

 CC||amonakov at gcc dot gnu.org

--- Comment #5 from Alexander Monakov  ---
Inline wrappers won't work because the second instance of pass_object_sizes
that is supposed to fully fold it is scheduled after IPA passes, and thus
information tied only to the attributes of inlined wrappers is lost.

I think it might be better to introduce __builtin_assume_object_size that can
convey such information directly in the IR.  User code then can do simply:

#define alloc(a, b) __builtin_assume_object_size(alloc(a, b), a+b)

and GCC itself can properly emit such builtin for existing functions bearing
the attribute, i.e. instead of 

var = malloc(size);

have in gimple explicitly:

tmp = malloc(size);
var = __builtin_assume_object_size(tmp, size);

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #4 from Andi Kleen  ---
I tested it now and the inline trick doesn't work. Here's a test case

extern void *do_alloc(int a, int b);

static inline __attribute__((alloc_size(1))) void check_alloc_size(int size)
{
}

static inline void *alloc(int a, int b)
{
check_alloc_size(a + b);
return do_alloc(a, b);
}

void func(void)
{
alloc(-1, 0);
}

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #3 from Andi Kleen  ---
Hmm, that trick may work for the shift too. Let me try.

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

Richard Biener  changed:

   What|Removed |Added

   Severity|normal  |enhancement

--- Comment #2 from Richard Biener  ---
For size_a + size_b can't you simply use an inline wrapper?

[Bug c/80378] Extend alloc_size attribute for better Linux kernel checking

[andi-gcc at firstfloor dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80378

--- Comment #1 from Andi Kleen  ---
Small correction: argument 4 would need to be a constant for shifted by.