On Thu, 6 Dec 2018, Martin Sebor wrote:
> On 12/6/18 2:26 PM, Jakub Jelinek wrote:
> > On Thu, Dec 06, 2018 at 01:21:58PM -0700, Martin Sebor wrote:
> > > Bug 88372 - alloc_size attribute is ignored on function pointers
> > > points out that even though the alloc_size attribute is accepted
> > > on function pointers it doesn't have any effect on Object Size
> > > Checking. The reporter, who is implementing the feature in Clang,
> > > wants to know if by exposing it under the same name they won't be
> > > causing incompatibilities with GCC.
> > >
> > > I don't think it's intentional that GCC doesn't take advantage of
> > > the attribute for Object Size Checking, and certainly not to detect
> > > the same kinds of issues as with other allocation functions (such
> > > as excessive or negative size arguments). Rather, it's almost
> > > certainly an oversight since GCC does make use of function pointer
> > > attributes in other contexts (e.g., attributes alloc_align and
> > > noreturn).
> > >
> > > As an oversight, I think it's fair to consider it a bug rather
> > > than a request for an enhancement. Since not handling
> > > the attribute in Object Size Checking has adverse security
> > > implications, I also think this bug should be addressed in GCC
> > > 9. With that, I submit the attached patch to resolve both
> > > aspects of the problem.
> >
> > This is because alloc_object_size has been written before we had attributes
> > like alloc_size. The only thing I'm unsure about is whether we should
> > prefer gimple_call_fntype or TREE_TYPE (gimple_call_fndecl ()) if it is a
> > direct call or if we should try to look for alloc_size attribute on both
> > of those if they are different types. E.g. if somebody does
> >
> > #include <stdlib.h>
> >
> > typedef void *(*allocfn) (size_t);
> >
> > static inline void *
> > foo (allocfn fn, size_t sz)
> > {
> > return fn (sz);
> > }
> >
> > static inline void *
> > bar (size_t sz)
> > {
> > return foo (malloc, sz);
> > }
> >
> > then I think this patch would no longer treat it as malloc.
> >
> > As this is security relevant, I'd probably look for alloc_size
> > attribute in both gimple_call_fntype and, if gimple_call_fndecl is non-NULL,
> > its TREE_TYPE.
>
> Thanks for the test case! I wondered if using fntype would
> always work but couldn't think of when it wouldn't. I've
> adjusted the function to use both and added the test case.
>
> While thinking about this it occurred to me that alloc_size
> is only documented as a function attribute but not one that
> applies to pointers or types. I added documentation for
> these uses to the Common Type and Common Variable sections.
Please always _only_ use gimple_call_fntype when the decl
isn't visible. As elsewhere the type of the function
pointer doesn't have any semantic meaning (it could be a
wrong one).
Richard.
> Martin
>
> PS Other function attributes that also apply to types and
> variables are only documented in the function section. They
> should also be mentioned in the other sections. Which, if
> done in the established style, will result in duplicating
> a lot of text in three places. I think that suggests that
> we might want to think about structuring these sections of
> the manual differently to avoid the duplication.
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
