From patchwork Thu Dec 6 20:21:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1009003 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-491827-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="K2dFQ8rX"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="MiPgmpUe"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 439n8B3j9Gz9s0n for ; Fri, 7 Dec 2018 07:22:16 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; q=dns; s= default; b=YYSKOiizjwjXTvgDi+BiQVIX9XDOZqSvrt4/ad7XU7+ptRkC5Budg 1mgBwJrMEA+41iw3/fNXjyqkZr/BT8uTxpxPnNevJmZSlEkVc6RArdgEBWfoBCdb /weURHBcRjiac9izfbfnU0WiHiWOFEeADGgL6bJiYlbR6bl+qapxz0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :subject:to:message-id:date:mime-version:content-type; s= default; bh=FkNPBx3eiWOnX6ztBUYKstQb9aA=; b=K2dFQ8rXLXoZnCf/ZeVk AZCKlT5ZXG0e74PBeCTqxRTvpLv3WDYLOT3ZmBq1cMf39iJdtBCwJBUSW7MWvpJD OGN7COiTH00yTFIJT+oX5IGLGrdFteRzgW9FEEYvUnsn35isza8j8WxnPFA10QWA PDgbIAsc4Tl/1BRP6ZzFVWY= Received: (qmail 37705 invoked by alias); 6 Dec 2018 20:22:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 37688 invoked by uid 89); 6 Dec 2018 20:22:08 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.1 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=is_gimple_call, sk:tree_in, 4276, exceed X-HELO: mail-qt1-f193.google.com Received: from mail-qt1-f193.google.com (HELO mail-qt1-f193.google.com) (209.85.160.193) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 06 Dec 2018 20:22:03 +0000 Received: by mail-qt1-f193.google.com with SMTP id n32so2011387qte.11 for ; Thu, 06 Dec 2018 12:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:subject:to:message-id:date:user-agent:mime-version :content-language; bh=RpxT/u8zOuafYbkwt5gf2OsiC6KvsSye0lpN6C+dTXQ=; b=MiPgmpUeSHFLaH+tR4xI4pFiSnP6jt2CALZCILug4e1L/wtMDkHNSvHzHFQ9G61yUA que00YskT3ILMCaRv/y3i61zQr+wXW+is0Bt0koxD5/uc9Auc756gUEvnio3ay0iSk1k fzGk2c7jhBaCAW6eUzHIq31XLHT1OrLsAeTvPKWuDVJ6Y6/KBQlg0ZVH1+vOH/Kj3MOS jn7dZlejE3VDAqK7SzFbU2EQMVk8YdVA2rrApQBaKYfQDBhfBei/+uemutpeBFky/9qz XVjhVv77YR31cGB1eKIklIF+W/ZVUzmzZ6GM7UjQpk4ZbHUWtuL6gOx+Ut6dWFcHLkJQ l3ng== Received: from localhost.localdomain (97-118-99-160.hlrn.qwest.net. [97.118.99.160]) by smtp.gmail.com with ESMTPSA id v42sm892474qth.32.2018.12.06.12.21.59 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Dec 2018 12:21:59 -0800 (PST) From: Martin Sebor Subject: [PATCH] handle function pointers in __builtin_object_size (PR 88372) To: Gcc Patch List Message-ID: Date: Thu, 6 Dec 2018 13:21:58 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 X-IsSubscribed: yes 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. Tested on x86_64-redhat-linux. Martin PR tree-optimization/88372 - alloc_size attribute is ignored on function pointers gcc/ChangeLog: PR tree-optimization/88372 * calls.c (maybe_warn_alloc_args_overflow): Handle function pointers. * tree-object-size.c (alloc_object_size): Same. Simplify. * doc/extend.texi (Object Size Checking): Update. (Other Builtins): Add __builtin_object_size. gcc/testsuite/ChangeLog: PR tree-optimization/88372 * gcc.dg/Walloc-size-larger-than-18.c: New test. * gcc.dg/builtin-object-size-19.c: Same. Index: gcc/calls.c =================================================================== --- gcc/calls.c (revision 266862) +++ gcc/calls.c (working copy) @@ -1342,9 +1342,10 @@ get_size_range (tree exp, tree range[2], bool allo /* Diagnose a call EXP to function FN decorated with attribute alloc_size whose argument numbers given by IDX with values given by ARGS exceed the maximum object size or cause an unsigned oveflow (wrapping) when - multiplied. When ARGS[0] is null the function does nothing. ARGS[1] - may be null for functions like malloc, and non-null for those like - calloc that are decorated with a two-argument attribute alloc_size. */ + multiplied. FN is null when EXP is a call via a function pointer. + When ARGS[0] is null the function does nothing. ARGS[1] may be null + for functions like malloc, and non-null for those like calloc that + are decorated with a two-argument attribute alloc_size. */ void maybe_warn_alloc_args_overflow (tree fn, tree exp, tree args[2], int idx[2]) @@ -1357,6 +1358,8 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, location_t loc = EXPR_LOCATION (exp); + tree fntype = fn ? TREE_TYPE (fn) : TREE_TYPE (TREE_TYPE (exp)); + built_in_function fncode = fn ? DECL_FUNCTION_CODE (fn) : BUILT_IN_NONE; bool warned = false; /* Validate each argument individually. */ @@ -1382,11 +1385,11 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, friends. Also avoid issuing the warning for calls to function named "alloca". */ - if ((DECL_FUNCTION_CODE (fn) == BUILT_IN_ALLOCA + if ((fncode == BUILT_IN_ALLOCA && IDENTIFIER_LENGTH (DECL_NAME (fn)) != 6) - || (DECL_FUNCTION_CODE (fn) != BUILT_IN_ALLOCA + || (fncode != BUILT_IN_ALLOCA && !lookup_attribute ("returns_nonnull", - TYPE_ATTRIBUTES (TREE_TYPE (fn))))) + TYPE_ATTRIBUTES (fntype)))) warned = warning_at (loc, OPT_Walloc_zero, "%Kargument %i value is zero", exp, idx[i] + 1); @@ -1398,6 +1401,7 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, size overflow. There's no good way to detect C++98 here so avoid diagnosing these calls for all C++ modes. */ if (i == 0 + && fn && !args[1] && lang_GNU_CXX () && DECL_IS_OPERATOR_NEW (fn) @@ -1481,7 +1485,7 @@ maybe_warn_alloc_args_overflow (tree fn, tree exp, } } - if (warned) + if (warned && fn) { location_t fnloc = DECL_SOURCE_LOCATION (fn); @@ -1933,14 +1937,13 @@ initialize_argument_information (int num_actuals A bitmap_obstack_release (NULL); - /* Extract attribute alloc_size and if set, store the indices of - the corresponding arguments in ALLOC_IDX, and then the actual - argument(s) at those indices in ALLOC_ARGS. */ + /* Extract attribute alloc_size from the type of the called expression + (which could be a function or a function pointer) and if set, store + the indices of the corresponding arguments in ALLOC_IDX, and then + the actual argument(s) at those indices in ALLOC_ARGS. */ int alloc_idx[2] = { -1, -1 }; - if (tree alloc_size - = (fndecl ? lookup_attribute ("alloc_size", - TYPE_ATTRIBUTES (TREE_TYPE (fndecl))) - : NULL_TREE)) + if (tree alloc_size = lookup_attribute ("alloc_size", + TYPE_ATTRIBUTES (fntype))) { tree args = TREE_VALUE (alloc_size); alloc_idx[0] = TREE_INT_CST_LOW (TREE_VALUE (args)) - 1; Index: gcc/tree-object-size.c =================================================================== --- gcc/tree-object-size.c (revision 266862) +++ gcc/tree-object-size.c (working copy) @@ -401,25 +401,23 @@ addr_object_size (struct object_size_info *osi, co /* Compute __builtin_object_size for CALL, which is a GIMPLE_CALL. - Handles various allocation calls. OBJECT_SIZE_TYPE is the second - argument from __builtin_object_size. If unknown, return - unknown[object_size_type]. */ + Handles calls to functions declared with attribute alloc_size. + OBJECT_SIZE_TYPE is the second argument from __builtin_object_size. + If unknown, return unknown[object_size_type]. */ static unsigned HOST_WIDE_INT alloc_object_size (const gcall *call, int object_size_type) { - tree callee, bytes = NULL_TREE; - tree alloc_size; - int arg1 = -1, arg2 = -1; - gcc_assert (is_gimple_call (call)); - callee = gimple_call_fndecl (call); - if (!callee) + tree calltype = gimple_call_fntype (call); + if (!calltype) return unknown[object_size_type]; - alloc_size = lookup_attribute ("alloc_size", - TYPE_ATTRIBUTES (TREE_TYPE (callee))); + /* Set to positions of alloc_size arguments. */ + int arg1 = -1, arg2 = -1; + tree alloc_size = lookup_attribute ("alloc_size", + TYPE_ATTRIBUTES (calltype)); if (alloc_size && TREE_VALUE (alloc_size)) { tree p = TREE_VALUE (alloc_size); @@ -429,19 +427,6 @@ alloc_object_size (const gcall *call, int object_s arg2 = TREE_INT_CST_LOW (TREE_VALUE (TREE_CHAIN (p)))-1; } - if (DECL_BUILT_IN_CLASS (callee) == BUILT_IN_NORMAL) - switch (DECL_FUNCTION_CODE (callee)) - { - case BUILT_IN_CALLOC: - arg2 = 1; - /* fall through */ - case BUILT_IN_MALLOC: - CASE_BUILT_IN_ALLOCA: - arg1 = 0; - default: - break; - } - if (arg1 < 0 || arg1 >= (int)gimple_call_num_args (call) || TREE_CODE (gimple_call_arg (call, arg1)) != INTEGER_CST || (arg2 >= 0 @@ -449,6 +434,7 @@ alloc_object_size (const gcall *call, int object_s || TREE_CODE (gimple_call_arg (call, arg2)) != INTEGER_CST))) return unknown[object_size_type]; + tree bytes = NULL_TREE; if (arg2 >= 0) bytes = size_binop (MULT_EXPR, fold_convert (sizetype, gimple_call_arg (call, arg1)), Index: gcc/doc/extend.texi =================================================================== --- gcc/doc/extend.texi (revision 266862) +++ gcc/doc/extend.texi (working copy) @@ -11124,7 +11124,10 @@ a limited extent, they can be used without optimiz @deftypefn {Built-in Function} {size_t} __builtin_object_size (const void * @var{ptr}, int @var{type}) is a built-in construct that returns a constant number of bytes from @var{ptr} to the end of the object @var{ptr} pointer points to -(if known at compile time). @code{__builtin_object_size} never evaluates +(if known at compile time). To determine the sizes of dynamically allocated +objects the function relies on the allocation functions called to obtain +the storage to be declared with the @code{alloc_size} attribute (@xref{Common +Function Attributes}). @code{__builtin_object_size} never evaluates its arguments for side effects. If there are any side effects in them, it returns @code{(size_t) -1} for @var{type} 0 or 1 and @code{(size_t) 0} for @var{type} 2 or 3. If there are multiple objects @var{ptr} can @@ -11249,6 +11252,7 @@ is called and the @var{flag} argument passed to it @findex __builtin_islessequal @findex __builtin_islessgreater @findex __builtin_isunordered +@findex __builtin_object_size @findex __builtin_powi @findex __builtin_powif @findex __builtin_powil @@ -12492,6 +12496,10 @@ is evaluated if it includes side effects but no ot and GCC does not issue a warning. @end deftypefn +@deftypefn {Built-in Function}{size_t} __builtin_object_size (const void * @var{ptr}, int @var{type}) +Returns the size of an object pointed to by @var{ptr}. @xref{Object Size Checking} for a detailed description of the function. +@end deftypefn + @deftypefn {Built-in Function} double __builtin_huge_val (void) Returns a positive infinity, if supported by the floating-point format, else @code{DBL_MAX}. This function is suitable for implementing the Index: gcc/testsuite/gcc.dg/Walloc-size-larger-than-18.c =================================================================== --- gcc/testsuite/gcc.dg/Walloc-size-larger-than-18.c (nonexistent) +++ gcc/testsuite/gcc.dg/Walloc-size-larger-than-18.c (working copy) @@ -0,0 +1,93 @@ +/* PR tree-optimization/88372 - alloc_size attribute is ignored + on function pointers + Verify that calls via function pointers declared alloc_size + with zero or excessive size trigger either -Walloc-zero or + -Walloc-size-larger-than warnings. + { dg-do compile } + { dg-options "-O2 -Wall -Walloc-zero -ftrack-macro-expansion=0" } */ + +#define ATTR(...) __attribute__ ((__VA_ARGS__)) + +typedef __SIZE_TYPE__ size_t; + + +void sink (void*); + +#define T(call) sink (call) + +ATTR (alloc_size (1)) void* (*ai1)(int, int); +ATTR (alloc_size (2)) void* (*ai2)(int, int); +ATTR (alloc_size (1, 2)) void* (*ai1_2)(int, int); + +ATTR (alloc_size (1)) void* (*asz1)(size_t, size_t); +ATTR (alloc_size (2)) void* (*asz2)(size_t, size_t); +ATTR (alloc_size (1, 2)) void* (*asz1_2)(size_t, size_t); + + +void test_alloc_ptr_zero (void) +{ + T (asz1 (0, 0)); /* { dg-warning "argument 1 value is zero" } */ + T (asz1 (0, 1)); /* { dg-warning "argument 1 value is zero" } */ + T (asz1 (1, 0)); + T (asz1 (1, 1)); + + T (asz2 (0, 0)); /* { dg-warning "argument 2 value is zero" } */ + T (asz2 (0, 1)); + T (asz2 (1, 0)); /* { dg-warning "argument 2 value is zero" } */ + T (asz2 (1, 1)); + + T (asz1_2 (0, 0)); /* { dg-warning "argument \[12\] value is zero" } */ + T (asz1_2 (1, 0)); /* { dg-warning "argument 2 value is zero" } */ + T (asz1_2 (0, 1)); /* { dg-warning "argument 1 value is zero" } */ + T (asz1_2 (1, 1)); +} + + +void test_alloc_ptr_negative (int n) +{ + T (ai1 (-1, -1)); /* { dg-warning "argument 1 value .-1. is negative" } */ + T (ai1 (-2, 1)); /* { dg-warning "argument 1 value .-2. is negative" } */ + T (ai1 ( 1, -1)); + T (ai1 ( 1, 1)); + + T (ai2 (-1, -3)); /* { dg-warning "argument 2 value .-3. is negative" } */ + T (ai2 (-1, 1)); + T (ai2 ( 1, -4)); /* { dg-warning "argument 2 value .-4. is negative" } */ + T (ai2 ( 1, 1)); + + T (ai1_2 (-5, -6)); /* { dg-warning "argument \[12\] value .-\[56\]. is negative" } */ + T (ai1_2 ( 1, -7)); /* { dg-warning "argument 2 value .-7. is negative" } */ + T (ai1_2 (-8, 1)); /* { dg-warning "argument 1 value .-8. is negative" } */ + T (ai1_2 ( 1, 1)); + + if (n > -1) + n = -1; + + /* Also verify a simple range. */ + T (ai1_2 ( 1, n)); /* { dg-warning "argument 2 range \\\[-\[0-9\]+, -1] is negative" } */ + T (ai1_2 ( n, 1)); /* { dg-warning "argument 1 range \\\[-\[0-9\]+, -1] is negative" } */ +} + +void test_alloc_ptr_too_big (void) +{ + size_t x = (__SIZE_MAX__ >> 1) + 1; + size_t y = __SIZE_MAX__ / 5; + + T (asz1 (x, x)); /* { dg-warning "argument 1 value .\[0-9\]+. exceeds" } */ + T (asz1 (x, 1)); /* { dg-warning "argument 1 value .\[0-9\]+. exceeds" } */ + T (asz1 (1, x)); + T (asz1 (1, 1)); + + T (asz2 (x, x)); /* { dg-warning "argument 2 value .\[0-9\]+. exceeds" } */ + T (asz2 (x, 1)); + T (asz2 (1, x)); /* { dg-warning "argument 2 value .\[0-9\]+. exceeds" } */ + T (asz2 (1, 1)); + + T (asz1_2 (x, x)); /* { dg-warning "argument \[12\] value .\[0-9\]+. exceeds" } */ + T (asz1_2 (y, 3)); /* { dg-warning "product .\[0-9\]+ \\\* 3. of arguments 1 and 2 exceeds" } */ + T (asz1_2 (y, y)); /* { dg-warning "product .\[0-9\]+ \\\* \[0-9\]+. of arguments 1 and 2 exceeds" } */ + T (asz1_2 (1, x)); /* { dg-warning "argument 2 value .\[0-9\]+. exceeds" } */ + T (asz1_2 (x, 1)); /* { dg-warning "argument 1 value .\[0-9\]+. exceeds" } */ + T (asz1_2 (1, 1)); + +} Index: gcc/testsuite/gcc.dg/builtin-object-size-19.c =================================================================== --- gcc/testsuite/gcc.dg/builtin-object-size-19.c (nonexistent) +++ gcc/testsuite/gcc.dg/builtin-object-size-19.c (working copy) @@ -0,0 +1,58 @@ +/* PR tree-optimization/88372 - alloc_size attribute is ignored + on function pointers { dg-do compile } + { dg-options "-O2 -fdump-tree-optimized" } */ + +#define ATTR(...) __attribute__ ((__VA_ARGS__)) +#define CONCAT(x, y) x ## y +#define CAT(x, y) CONCAT (x, y) +#define FAILNAME(name) CAT (call_ ## name ##_on_line_, __LINE__) + +#define FAIL(name) do { \ + extern void FAILNAME (name) (void); \ + FAILNAME (name)(); \ + } while (0) + +/* Macro to emit a call to function named + call_in_true_branch_not_eliminated_on_line_NNN() + for each call that's expected to be eliminated. The dg-final + scan-tree-dump-time directive at the bottom of the test verifies + that no such call appears in output. */ +#define ELIM(expr) \ + if (!(expr)) FAIL (in_true_branch_not_eliminated); else (void)0 + +void sink (void*); + +#define T1(alloc, n) do { \ + void *p = alloc; \ + sink (p); \ + ELIM (n == __builtin_object_size (p, 0)); \ + ELIM (n == __builtin_object_size (p, 1)); \ + ELIM (n == __builtin_object_size (p, 2)); \ + ELIM (n == __builtin_object_size (p, 3)); \ + } while (0) + + +ATTR (alloc_size (1)) void* (*alloc_1)(int, int); +ATTR (alloc_size (2)) void* (*alloc_2)(int, int); +ATTR (alloc_size (1, 2)) void* (*alloc_1_2)(int, int); + + +void test_alloc_ptr (void) +{ + T1 (alloc_1 (0, 0), 0); + T1 (alloc_1 (1, 0), 1); + T1 (alloc_1 (3, 0), 3); + T1 (alloc_1 (9, 5), 9); + + T1 (alloc_2 (0, 0), 0); + T1 (alloc_2 (1, 0), 0); + T1 (alloc_2 (0, 1), 1); + T1 (alloc_2 (9, 5), 5); + + T1 (alloc_1_2 (0, 0), 0); + T1 (alloc_1_2 (1, 0), 0); + T1 (alloc_1_2 (0, 1), 0); + T1 (alloc_1_2 (9, 5), 45); +} + +/* { dg-final { scan-tree-dump-not "not_eliminated" "optimized" } } */