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] |
On 12/7/18 1:06 AM, Richard Biener wrote:
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).
I don't understand what you're asking me to do differently here: - callee = gimple_call_fndecl (call); - if (!callee) + tree calltype; + if (tree callfn = gimple_call_fndecl (call)) + calltype = TREE_TYPE (callfn); + else + calltype = gimple_call_fntype (call); ^^^^^^^^^^^^^^^^^^ Can you show the change you're looking for? (The change I had made originally was in response to this same request you made in Bugzilla, which Jakub then suggested may not be robust enough.) Btw., it should be straightforward to ask "give me the attribute if this is a call to an alloc_size-kind of a function?" (or one with whatever other attribute of interest). Since it appears to be anything but straightforward, we should consider providing an API to make it so. Maybe something like: tree gimple_call_attribute (gcall *, tree attribute); Martin
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.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |