From patchwork Wed Nov 6 18:00:36 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1190592 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-512636-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="Yy5QDW7+"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="Hh360VLy"; 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 477Z8s1q7tz9s7T for ; Thu, 7 Nov 2019 05:01:15 +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=GhI5IwREdShscwJohIr3HO3Y3679UWxSCI8yWPSOoaY8Vw8rRAXNF 2tyvaKPker5CwwjqMIUIlDIsxpAn6ci9WeSLV606IqQ7n2S8zKFTBE05nGopBHRt pWwFyXV333nQOVTI8nIJNpTFdEmi/18PRucjJ2HyyosnbXRsp3MzII= 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=TKqZWMLFJ/igPIwqLghRsLUFgRQ=; b=Yy5QDW7+oOH4435pGS/d CL/HbelCKXeFh9d/c4JgyKnCM81GF/Tgc4QoS5NhZP5vcLihgpzwJ2CW0fVXnyTc ruKLILklxNPmvJBN+a2ZHZeqLD02Ur853/V52b4usU50p7CiIon5330aSzQvHAN8 yTrX+6+/Wh6VnmL+railAko= Received: (qmail 77662 invoked by alias); 6 Nov 2019 18:01:00 -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 77603 invoked by uid 89); 6 Nov 2019 18:00:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-14.1 required=5.0 tests=AWL, 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.1 spammy=31513, sk:tree_fi, sprintf X-HELO: mail-yw1-f51.google.com Received: from mail-yw1-f51.google.com (HELO mail-yw1-f51.google.com) (209.85.161.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Nov 2019 18:00:42 +0000 Received: by mail-yw1-f51.google.com with SMTP id k127so9884225ywc.6 for ; Wed, 06 Nov 2019 10:00:42 -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=fsM5LLG1+A+p+7uLIjNEvrkJZbc2+cGm65iK8wF7bQo=; b=Hh360VLyuw13S1lwSYJWKBg7FvH8nJWZkMjsRAXNpMluPWTgiH8G1/3RTs+oI8+uKs 00L5zeanU5YNFwsrz4p/F3LQuVp+iI5CGZNHOmwl4mkbddIB0Q8TBqN6jMxkTUyRXF6r 5XMKwshbeaWLj8naz6rQfGkF1be/fqM3t78++beJXW/x13pFIkyld7sh6akZD3h0GF4p JEAnsd3UBzWUNqpLntNqznknRzlft3hJeG7emLLfFw+2VxTnFXc8G1Nupc63hyPY0KHX dJs/JkP8B5Ggu9SG9whvMrBgKVZV1WNOyEfx/xv5P+1xFpH4MZnaW1WIYC098asoI1ZY KEBw== Received: from [192.168.0.41] (97-118-98-145.hlrn.qwest.net. [97.118.98.145]) by smtp.gmail.com with ESMTPSA id z16sm14460739ywj.93.2019.11.06.10.00.37 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 Nov 2019 10:00:38 -0800 (PST) From: Martin Sebor Subject: [PATCH] include size and offset in -Wstringop-overflow To: gcc-patches Message-ID: <9d3210bf-e1bd-81ac-b1a0-5c66c8ec6db7@gmail.com> Date: Wed, 6 Nov 2019 11:00:36 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 X-IsSubscribed: yes The -Wstringop-overflow warnings for single-byte and multi-byte stores mention the amount of data being stored and the amount of space remaining in the destination, such as: warning: writing 4 bytes into a region of size 0 [-Wstringop-overflow=] 123 | *p = 0; | ~~~^~~ note: destination object declared here 45 | char b[N]; | ^ A warning like this can take some time to analyze. First, the size of the destination isn't mentioned and may not be easy to tell from the sources. In the note above, when N's value is the result of some non-trivial computation, chasing it down may be a small project in and of itself. Second, it's also not clear why the region size is zero. It could be because the offset is exactly N, or because it's negative, or because it's in some range greater than N. Mentioning both the size of the destination object and the offset makes the existing messages clearer, are will become essential when GCC starts diagnosing overflow into allocated buffers (as my follow-on patch does). The attached patch enhances -Wstringop-overflow to do this by letting compute_objsize return the offset to its caller, doing something similar in get_stridx, and adding a new function to the strlen pass to issue this enhanced warning (eventually, I'd like the function to replace the -Wstringop-overflow handler in builtins.c). With the change, the note above might read something like: note: at offset 11 to object ‘b’ with size 8 declared here 45 | char b[N]; | ^ Tested on x86_64-linux. Martin gcc/ChangeLog: * builtins.c (compute_objsize): Add an argument and set it to offset into destination. * builtins.h (compute_objsize): Add an argument. * tree-object-size.c (addr_object_size): Add an argument and set it to offset into destination. (compute_builtin_object_size): Same. * tree-object-size.h (compute_builtin_object_size): Add an argument. * tree-ssa-strlen.c (get_addr_stridx): Add an argument and set it to offset into destination. (maybe_warn_overflow): New function. (handle_store): Call maybe_warn_overflow to issue warnings. gcc/testsuite/ChangeLog: * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected messages. * g++.dg/warn/Wstringop-overflow-3.C: Same. * gcc.dg/Wstringop-overflow-17.c: Same. Index: gcc/builtins.c =================================================================== --- gcc/builtins.c (revision 277886) +++ gcc/builtins.c (working copy) @@ -3571,23 +3571,29 @@ check_access (tree exp, tree, tree, tree dstwrite, a non-constant offset in some range the returned value represents the largest size given the smallest non-negative offset in the range. If nonnull, set *PDECL to the decl of the referenced - subobject if it can be determined, or to null otherwise. + subobject if it can be determined, or to null otherwise. Likewise, + when POFF is nonnull *POFF is set to the offset into *PDECL. The function is intended for diagnostics and should not be used to influence code generation or optimization. */ tree -compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */) +compute_objsize (tree dest, int ostype, tree *pdecl /* = NULL */, + tree *poff /* = NULL */) { - tree dummy = NULL_TREE; + tree dummy_decl = NULL_TREE; if (!pdecl) - pdecl = &dummy; + pdecl = &dummy_decl; + tree dummy_off = size_zero_node; + if (!poff) + poff = &dummy_off; + unsigned HOST_WIDE_INT size; /* Only the two least significant bits are meaningful. */ ostype &= 3; - if (compute_builtin_object_size (dest, ostype, &size, pdecl)) + if (compute_builtin_object_size (dest, ostype, &size, pdecl, poff)) return build_int_cst (sizetype, size); if (TREE_CODE (dest) == SSA_NAME) @@ -3608,7 +3614,7 @@ tree tree off = gimple_assign_rhs2 (stmt); if (TREE_CODE (off) == INTEGER_CST) { - if (tree size = compute_objsize (dest, ostype, pdecl)) + if (tree size = compute_objsize (dest, ostype, pdecl, poff)) { wide_int wioff = wi::to_wide (off); wide_int wisiz = wi::to_wide (size); @@ -3619,10 +3625,16 @@ tree if (wi::sign_mask (wioff)) ; else if (wi::ltu_p (wioff, wisiz)) - return wide_int_to_tree (TREE_TYPE (size), - wi::sub (wisiz, wioff)); + { + *poff = size_binop (PLUS_EXPR, *poff, off); + return wide_int_to_tree (TREE_TYPE (size), + wi::sub (wisiz, wioff)); + } else - return size_zero_node; + { + *poff = size_binop (PLUS_EXPR, *poff, off); + return size_zero_node; + } } } else if (TREE_CODE (off) == SSA_NAME @@ -3644,10 +3656,18 @@ tree || wi::sign_mask (max)) ; else if (wi::ltu_p (min, wisiz)) - return wide_int_to_tree (TREE_TYPE (size), - wi::sub (wisiz, min)); + { + *poff = size_binop (PLUS_EXPR, *poff, + wide_int_to_tree (sizetype, min)); + return wide_int_to_tree (TREE_TYPE (size), + wi::sub (wisiz, min)); + } else - return size_zero_node; + { + *poff = size_binop (PLUS_EXPR, *poff, + wide_int_to_tree (sizetype, min)); + return size_zero_node; + } } } } @@ -3666,17 +3686,21 @@ tree { tree ref = TREE_OPERAND (dest, 0); tree off = TREE_OPERAND (dest, 1); - if (tree size = compute_objsize (ref, ostype, pdecl)) + if (tree size = compute_objsize (ref, ostype, pdecl, poff)) { + *poff = size_binop (PLUS_EXPR, *poff, fold_convert (sizetype, off)); + /* If the declaration of the destination object is known to have zero size, return zero. */ if (integer_zerop (size)) - return integer_zero_node; + return size_zero_node; - if (TREE_CODE (off) != INTEGER_CST - || TREE_CODE (size) != INTEGER_CST) + if (TREE_CODE (size) != INTEGER_CST + || TREE_CODE (off) != INTEGER_CST) return NULL_TREE; + off = fold_convert (ptrdiff_type_node, off); + if (TREE_CODE (dest) == ARRAY_REF) { tree eltype = TREE_TYPE (dest); @@ -3687,9 +3711,19 @@ tree return NULL_TREE; } + if (tree_int_cst_sgn (off) < 0) + { + /* Add the negative offset to the size and return the smaller + of the difference and zero. */ + tree szdiff = fold_build2 (PLUS_EXPR, size_type_node, size, off); + if (tree_int_cst_lt (szdiff, size)) + return szdiff; + return size_zero_node; + } + if (tree_int_cst_lt (off, size)) return fold_build2 (MINUS_EXPR, size_type_node, size, off); - return integer_zero_node; + return size_zero_node; } return NULL_TREE; Index: gcc/builtins.h =================================================================== --- gcc/builtins.h (revision 277886) +++ gcc/builtins.h (working copy) @@ -134,7 +134,7 @@ extern tree fold_call_stmt (gcall *, bool); extern void set_builtin_user_assembler_name (tree decl, const char *asmspec); extern bool is_simple_builtin (tree); extern bool is_inexpensive_builtin (tree); -extern tree compute_objsize (tree, int, tree * = NULL); +extern tree compute_objsize (tree, int, tree * = NULL, tree * = NULL); extern bool readonly_data_expr (tree exp); extern bool init_target_chars (void); Index: gcc/testsuite/c-c++-common/Wstringop-overflow-2.c =================================================================== --- gcc/testsuite/c-c++-common/Wstringop-overflow-2.c (revision 277886) +++ gcc/testsuite/c-c++-common/Wstringop-overflow-2.c (working copy) @@ -10,7 +10,7 @@ void sink (void*); struct Ax { char n; - char a[]; // { dg-message "destination object declared here" } + char a[]; // { dg-message "declared here" } }; // Verify warning for a definition with no initializer. @@ -91,7 +91,7 @@ void gaxx (void) struct A0 { char n; - char a[0]; // { dg-message "destination object declared here" } + char a[0]; // { dg-message "declared here" } }; // Verify warning for a definition with no initializer. @@ -158,7 +158,7 @@ void ga0x (void) struct A1 { char n; - char a[1]; // { dg-message "destination object declared here" } + char a[1]; // { dg-message "declared here" } }; // Verify warning for a definition with no initializer. @@ -256,7 +256,7 @@ void ga1x (void) struct A1i { char n; - char a[1]; // { dg-message "destination object declared here" } + char a[1]; // { dg-message "declared here" } char x; }; Index: gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C =================================================================== --- gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C (revision 277886) +++ gcc/testsuite/g++.dg/warn/Wstringop-overflow-3.C (working copy) @@ -3,6 +3,8 @@ { dg-do compile } { dg-options "-O2 -Wall -Wno-array-bounds" } */ +#define NOIPA __attribute__ ((noipa)) + void sink (void*); // Exercise flexible array members. @@ -10,13 +12,13 @@ void sink (void*); struct Ax { char n; - char a[]; // { dg-message "destination object declared here" } + char a[]; // { dg-message "at offset \[0-2\] to object 'Ax::a' declared here" } }; // Verify warning for a definition with no initializer. Ax ax_; -void gax_ () +NOIPA void gax_ () { ax_.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } ax_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -27,7 +29,7 @@ Ax ax_; // initialize the flexible array member. Ax ax0 = { 0 }; -void gax0 () +NOIPA void gax0 () { ax0.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } ax0.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -38,7 +40,7 @@ Ax ax0 = { 0 }; // initializes the flexible array member to empty. Ax ax0_ = { 0, { } }; -void gax0_ () +NOIPA void gax0_ () { ax0_.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } ax0_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -49,7 +51,7 @@ Ax ax0_ = { 0, { } }; // an initializer. Ax ax1 = { 1, { 0 } }; -void gax1 () +NOIPA void gax1 () { ax1.a[0] = 0; ax1.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -58,7 +60,7 @@ Ax ax1 = { 1, { 0 } }; Ax ax2 = { 2, { 1, 0 } }; -void gax2 () +NOIPA void gax2 () { ax2.a[0] = 0; ax2.a[1] = 0; @@ -67,7 +69,7 @@ Ax ax2 = { 2, { 1, 0 } }; // Verify no warning for an unknown struct object. -void gaxp (Ax *p) +NOIPA void gaxp (Ax *p) { p->a[0] = 0; p->a[3] = 0; @@ -79,7 +81,7 @@ Ax ax2 = { 2, { 1, 0 } }; // initialized to any number of elements. extern Ax axx; -void gaxx () +NOIPA void gaxx () { axx.a[0] = 0; axx.a[3] = 0; @@ -91,13 +93,13 @@ extern Ax axx; struct A0 { char n; - char a[0]; // { dg-message "destination object declared here" } + char a[0]; // { dg-message "at offset \[0-2\] to object 'A0::a' with size 0 declared here" } }; // Verify warning for a definition with no initializer. A0 a0_; -void ga0_ () +NOIPA void ga0_ () { a0_.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } a0_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -108,7 +110,7 @@ A0 a0_; // initialize the flexible array member. A0 a00 = { 0 }; -void ga00 () +NOIPA void ga00 () { a00.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } a00.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -119,7 +121,7 @@ A0 a00 = { 0 }; // initializes the flexible array member to empty. A0 a00_ = { 0, { } }; -void ga00_ () +NOIPA void ga00_ () { a00_.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } a00_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -133,7 +135,7 @@ A0 a00_ = { 0, { } }; // Verify no warning for an unknown struct object. -void ga0p (A0 *p) +NOIPA void ga0p (A0 *p) { p->a[0] = 0; p->a[3] = 0; @@ -145,7 +147,7 @@ A0 a00_ = { 0, { } }; // flexible array member) may not be initialized. extern A0 a0x; -void ga0x () +NOIPA void ga0x () { a0x.a[0] = 0; // { dg-warning "\\\[-Wstringop-overflow" } a0x.a[3] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -158,13 +160,13 @@ extern A0 a0x; struct A1 { char n; - char a[1]; // { dg-message "destination object declared here" } + char a[1]; // { dg-message "at offset \[1-9\] to object 'A1::a' with size 1 declared here" } }; // Verify warning for a definition with no initializer. A1 a1_; -void ga1_ () +NOIPA void ga1_ () { a1_.a[0] = 0; a1_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -175,7 +177,7 @@ A1 a1_; // initialize the one-element array member. A1 a1__ = { 0 }; -void ga1__ () +NOIPA void ga1__ () { a1__.a[0] = 0; a1__.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -186,7 +188,7 @@ A1 a1__ = { 0 }; // initializes the one-element array member to empty. A1 a1_0 = { 0, { } }; -void ga1_0_ () +NOIPA void ga1_0_ () { a1_0.a[0] = 0; a1_0.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -197,7 +199,7 @@ A1 a1_0 = { 0, { } }; // initializes the one-element array member. A1 a1_1 = { 0, { 1 } }; -void ga1_1 () +NOIPA void ga1_1 () { a1_1.a[0] = 0; a1_1.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -206,7 +208,7 @@ A1 a1_1 = { 0, { 1 } }; // Verify no warning for an unknown struct object. -void ga1p (A1 *p) +NOIPA void ga1p (A1 *p) { p->a[0] = 0; p->a[3] = 0; @@ -219,7 +221,7 @@ A1 a1_1 = { 0, { 1 } }; // a single element. extern A1 a1x; -void ga1x () +NOIPA void ga1x () { a1x.a[0] = 0; a1x.a[3] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -232,7 +234,7 @@ extern A1 a1x; struct A1i { char n; - char a[1]; // { dg-message "destination object declared here" } + char a[1]; // { dg-message "at offset \[1-9\] to object 'A1i::a' with size 1 declared here" } char x; }; @@ -239,7 +241,7 @@ struct A1i // Verify warning for a definition with no initializer. A1i a1i_; -void ga1i_ () +NOIPA void ga1i_ () { a1i_.a[0] = 0; a1i_.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -250,7 +252,7 @@ A1i a1i_; // initialize the one-element array member. A1i a1i__ = { 0 }; -void ga1i__ () +NOIPA void ga1i__ () { a1i__.a[0] = 0; a1i__.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -261,7 +263,7 @@ A1i a1i__ = { 0 }; // initializes the one-element array member to empty. A1 a1i_0 = { 0, { } }; -void ga1i_0_ () +NOIPA void ga1i_0_ () { a1i_0.a[0] = 0; a1i_0.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -272,7 +274,7 @@ A1 a1i_0 = { 0, { } }; // initializes the one-element array member. A1 a1i_1 = { 0, { 1 } }; -void ga1i_1 () +NOIPA void ga1i_1 () { a1i_1.a[0] = 0; a1i_1.a[1] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -281,7 +283,7 @@ A1 a1i_1 = { 0, { 1 } }; // Verify no warning for an unknown struct object. -void ga1ip (A1i *p) +NOIPA void ga1ip (A1i *p) { p->a[0] = 0; p->a[3] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -292,7 +294,7 @@ A1 a1i_1 = { 0, { 1 } }; // Verify no warning for an extern struct object. extern A1i a1ix; -void ga1ix () +NOIPA void ga1ix () { a1ix.a[0] = 0; a1ix.a[3] = 0; // { dg-warning "\\\[-Wstringop-overflow" } @@ -305,7 +307,7 @@ extern A1i a1ix; struct Bx { char n; - char a[]; // { dg-message "destination object declared here" } + char a[]; // { dg-message "at offset 0 to object 'Bx::a' declared here" } // Verify the warning for a constant. Bx () { a[0] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } @@ -315,13 +317,13 @@ struct Bx Bx (int i) { a[i] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } }; -void gbx (void) +NOIPA void gbx (void) { struct Bx bx; sink (&bx); } -void gbxi (int i) +NOIPA void gbxi (int i) { struct Bx bxi (i); sink (&bxi); @@ -330,13 +332,13 @@ struct Bx struct B0 { char n; - char a[0]; // { dg-message "destination object declared here" } + char a[0]; // { dg-message "at offset 0 to object 'B0::a' with size 0 declared here" } B0 () { a[0] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } }; -void gb0 (void) +NOIPA void gb0 (void) { struct B0 b0; sink (&b0); @@ -346,12 +348,12 @@ struct B0 struct B1 { char n; - char a[1]; // { dg-message "destination object declared here" } + char a[1]; // { dg-message "at offset 1 to object 'B1::a' with size 1 declared here" } B1 () { a[1] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } }; -void gb1 (void) +NOIPA void gb1 (void) { struct B1 b1; sink (&b1); @@ -360,12 +362,12 @@ struct B1 struct B123 { - char a[123]; // { dg-message "destination object declared here" } + char a[123]; // { dg-message "at offset 123 to object 'B123::a' with size 123 declared here" } B123 () { a[123] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } }; -void gb123 (void) +NOIPA void gb123 (void) { struct B123 b123; sink (&b123); @@ -374,12 +376,12 @@ struct B123 struct B234 { - char a[234]; // { dg-message "destination object declared here" } + char a[234]; // { dg-message "at offset 234 to object 'B234::a' with size 234 declared here" } B234 (int i) { a[i] = 0; } // { dg-warning "\\\[-Wstringop-overflow" } }; -void g234 (void) +NOIPA void g234 (void) { struct B234 b234 (234); sink (&b234); Index: gcc/testsuite/gcc.dg/Wstringop-overflow-17.c =================================================================== --- gcc/testsuite/gcc.dg/Wstringop-overflow-17.c (revision 277886) +++ gcc/testsuite/gcc.dg/Wstringop-overflow-17.c (working copy) @@ -13,7 +13,7 @@ void sink (void*); void call_copy_n (const char *s) { - char a[3]; // { dg-message "destination object declared here" } + char a[3]; // { dg-message "declared here" } copy_n (a, "1234567", 7); sink (a); } Index: gcc/tree-object-size.c =================================================================== --- gcc/tree-object-size.c (revision 277886) +++ gcc/tree-object-size.c (working copy) @@ -55,7 +55,7 @@ static const unsigned HOST_WIDE_INT unknown[4] = { static tree compute_object_offset (const_tree, const_tree); static bool addr_object_size (struct object_size_info *, const_tree, int, unsigned HOST_WIDE_INT *, - tree * = NULL); + tree * = NULL, tree * = NULL); static unsigned HOST_WIDE_INT alloc_object_size (const gcall *, int); static tree pass_through_call (const gcall *); static void collect_object_sizes_for (struct object_size_info *, tree); @@ -174,13 +174,15 @@ compute_object_offset (const_tree expr, const_tree static bool addr_object_size (struct object_size_info *osi, const_tree ptr, int object_size_type, unsigned HOST_WIDE_INT *psize, - tree *pdecl /* = NULL */) + tree *pdecl /* = NULL */, tree *poff /* = NULL */) { tree pt_var, pt_var_size = NULL_TREE, var_size, bytes; - tree dummy; + tree dummy_decl, dummy_off = size_zero_node; if (!pdecl) - pdecl = &dummy; + pdecl = &dummy_decl; + if (!poff) + poff = &dummy_off; gcc_assert (TREE_CODE (ptr) == ADDR_EXPR); @@ -201,7 +203,7 @@ addr_object_size (struct object_size_info *osi, co || TREE_CODE (TREE_OPERAND (pt_var, 0)) != SSA_NAME) { compute_builtin_object_size (TREE_OPERAND (pt_var, 0), - object_size_type & ~1, &sz, pdecl); + object_size_type & ~1, &sz, pdecl, poff); } else { @@ -376,6 +378,7 @@ addr_object_size (struct object_size_info *osi, co bytes = size_zero_node; else bytes = size_binop (MINUS_EXPR, var_size, bytes); + *poff = bytes; } if (var != pt_var && pt_var_size @@ -390,6 +393,7 @@ addr_object_size (struct object_size_info *osi, co bytes2 = size_zero_node; else bytes2 = size_binop (MINUS_EXPR, pt_var_size, bytes2); + *poff = size_binop (PLUS_EXPR, *poff, bytes2); bytes = size_binop (MIN_EXPR, bytes, bytes2); } } @@ -496,10 +500,16 @@ pass_through_call (const gcall *call) bool compute_builtin_object_size (tree ptr, int object_size_type, unsigned HOST_WIDE_INT *psize, - tree *pdecl /* = NULL */) + tree *pdecl /* = NULL */, tree *poff /* = NULL */) { gcc_assert (object_size_type >= 0 && object_size_type <= 3); + tree dummy_decl, dummy_off = size_zero_node; + if (!pdecl) + pdecl = &dummy_decl; + if (!poff) + poff = &dummy_off; + /* Set to unknown and overwrite just before returning if the size could be determined. */ *psize = unknown[object_size_type]; @@ -508,7 +518,7 @@ compute_builtin_object_size (tree ptr, int object_ init_offset_limit (); if (TREE_CODE (ptr) == ADDR_EXPR) - return addr_object_size (NULL, ptr, object_size_type, psize, pdecl); + return addr_object_size (NULL, ptr, object_size_type, psize, pdecl, poff); if (TREE_CODE (ptr) != SSA_NAME || !POINTER_TYPE_P (TREE_TYPE (ptr))) @@ -533,11 +543,12 @@ compute_builtin_object_size (tree ptr, int object_ if (tree_fits_shwi_p (offset) && compute_builtin_object_size (ptr, object_size_type, - psize, pdecl)) + psize, pdecl, poff)) { /* Return zero when the offset is out of bounds. */ unsigned HOST_WIDE_INT off = tree_to_shwi (offset); *psize = off < *psize ? *psize - off : 0; + *poff = offset; return true; } } Index: gcc/tree-object-size.h =================================================================== --- gcc/tree-object-size.h (revision 277886) +++ gcc/tree-object-size.h (working copy) @@ -23,6 +23,6 @@ along with GCC; see the file COPYING3. If not see extern void init_object_sizes (void); extern void fini_object_sizes (void); extern bool compute_builtin_object_size (tree, int, unsigned HOST_WIDE_INT *, - tree * = NULL); + tree * = NULL, tree * = NULL); #endif // GCC_TREE_OBJECT_SIZE_H Index: gcc/tree-ssa-strlen.c =================================================================== --- gcc/tree-ssa-strlen.c (revision 277886) +++ gcc/tree-ssa-strlen.c (working copy) @@ -189,6 +189,52 @@ struct laststmt_struct static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, tree); static void handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *); +/* Sets MINMAX to either the constant value or the range VAL is in + and returns true on success. */ + +static bool +get_range (tree val, wide_int minmax[2], const vr_values *rvals = NULL) +{ + if (tree_fits_uhwi_p (val)) + { + minmax[0] = minmax[1] = wi::to_wide (val); + return true; + } + + if (TREE_CODE (val) != SSA_NAME) + return false; + + if (rvals) + { + gimple *def = SSA_NAME_DEF_STMT (val); + if (gimple_assign_single_p (def) + && gimple_assign_rhs_code (def) == INTEGER_CST) + { + /* get_value_range returns [0, N] for constant assignments. */ + val = gimple_assign_rhs1 (def); + minmax[0] = minmax[1] = wi::to_wide (val); + return true; + } + + const value_range *vr + = (CONST_CAST (class vr_values *, rvals)->get_value_range (val)); + value_range_kind rng = vr->kind (); + if (rng != VR_RANGE || !range_int_cst_p (vr)) + return false; + + minmax[0] = wi::to_wide (vr->min ()); + minmax[1] = wi::to_wide (vr->max ()); + return true; + } + + value_range_kind rng = get_range_info (val, minmax, minmax + 1); + if (rng == VR_RANGE) + return true; + + // FIXME: handle anti-ranges? + return false; +} + /* Return: * +1 if SI is known to start with more than OFF nonzero characters. @@ -334,11 +380,18 @@ get_addr_stridx (tree exp, tree ptr, unsigned HOST return 0; } -/* Return string index for EXP. */ +/* Returns string index for EXP. When EXP is an SSA_NAME that refers + to a known strinfo with an offset and OFFRNG is non-null, sets + both elements of the OFFRNG array to the range of the offset and + returns the index of the known strinfo. In this case the result + must not be used in for functions that modify the string. */ static int -get_stridx (tree exp) +get_stridx (tree exp, wide_int offrng[2] = NULL) { + if (offrng) + offrng[0] = offrng[1] = wi::zero (TYPE_PRECISION (sizetype)); + if (TREE_CODE (exp) == SSA_NAME) { if (ssa_ver_to_stridx[SSA_NAME_VERSION (exp)]) @@ -345,6 +398,7 @@ static int return ssa_ver_to_stridx[SSA_NAME_VERSION (exp)]; tree e = exp; + int last_idx = 0; HOST_WIDE_INT offset = 0; /* Follow a chain of at most 5 assignments. */ for (int i = 0; i < 5; i++) @@ -351,7 +405,7 @@ static int { gimple *def_stmt = SSA_NAME_DEF_STMT (e); if (!is_gimple_assign (def_stmt)) - return 0; + return last_idx; tree_code rhs_code = gimple_assign_rhs_code (def_stmt); tree ptr, off; @@ -403,25 +457,69 @@ static int else return 0; - if (TREE_CODE (ptr) != SSA_NAME - || !tree_fits_shwi_p (off)) + if (TREE_CODE (ptr) != SSA_NAME) return 0; + + if (!tree_fits_shwi_p (off)) + { + if (int idx = ssa_ver_to_stridx[SSA_NAME_VERSION (ptr)]) + if (offrng) + { + /* Only when requested by setting OFFRNG to non-null, + return the index corresponding to the SSA_NAME. + Do this irrespective of the whether the offset + is known. */ + if (get_range (off, offrng)) + { + /* When the offset range is known, increment it + it by the constant offset computed in prior + iterations and store it in the OFFRNG array. */ + offrng[0] += offset; + offrng[1] += offset; + } + else + { + /* When the offset range cannot be determined + store [0, SIZE_MAX] and let the caller decide + if the offset matters. */ + offrng[1] = wi::to_wide (TYPE_MAX_VALUE (sizetype)); + offrng[0] = wi::zero (offrng[1].get_precision ()); + } + return idx; + } + return 0; + } + HOST_WIDE_INT this_off = tree_to_shwi (off); + if (offrng) + { + offrng[0] += wi::shwi (this_off, offrng->get_precision ()); + offrng[1] += offrng[0]; + } + if (this_off < 0) - return 0; + return last_idx; + offset = (unsigned HOST_WIDE_INT) offset + this_off; if (offset < 0) - return 0; - if (ssa_ver_to_stridx[SSA_NAME_VERSION (ptr)]) + return last_idx; + + if (int idx = ssa_ver_to_stridx[SSA_NAME_VERSION (ptr)]) { - strinfo *si - = get_strinfo (ssa_ver_to_stridx[SSA_NAME_VERSION (ptr)]); - if (si && compare_nonzero_chars (si, offset) >= 0) - return get_stridx_plus_constant (si, offset, exp); + strinfo *si = get_strinfo (idx); + if (si) + { + if (compare_nonzero_chars (si, offset) >= 0) + return get_stridx_plus_constant (si, offset, exp); + + if (offrng) + last_idx = idx; + } } e = ptr; } - return 0; + + return last_idx; } if (TREE_CODE (exp) == ADDR_EXPR) @@ -1763,6 +1861,279 @@ maybe_set_strlen_range (tree lhs, tree src, tree b return set_strlen_range (lhs, min, max, bound); } +/* Diagnose buffer overflow by a STMT writing LEN + PLUS_ONE bytes, + into an object designated by the LHS of STMT otherise. */ + +static void +maybe_warn_overflow (gimple *stmt, tree len, + const vr_values *rvals = NULL, + strinfo *si = NULL, bool plus_one = false) +{ + if (!len || gimple_no_warning_p (stmt)) + return; + + tree writefn = NULL_TREE; + tree destdecl = NULL_TREE; + tree destsize = NULL_TREE; + tree dest = NULL_TREE; + + /* The offset into the destination object set by compute_objsize + but already reflected in DESTSIZE. */ + tree destoff = NULL_TREE; + + if (is_gimple_assign (stmt)) + { + dest = gimple_assign_lhs (stmt); + if (TREE_NO_WARNING (dest)) + return; + + /* For assignments try to determine the size of the destination + first. Set DESTOFF to the the offset on success. */ + tree off = size_zero_node; + destsize = compute_objsize (dest, 1, &destdecl, &off); + if (destsize) + destoff = off; + } + else if (is_gimple_call (stmt)) + { + writefn = gimple_call_fndecl (stmt); + dest = gimple_call_arg (stmt, 0); + } + + /* The offset into the destination object computed below and not + reflected in DESTSIZE. Either DESTOFF is set above or OFFRNG + below. */ + wide_int offrng[2]; + offrng[0] = wi::zero (TYPE_PRECISION (sizetype)); + offrng[1] = offrng[0]; + + if (!destsize && !si && dest) + { + /* For both assignments and calls, if no destination STRINFO was + provided, try to get it from the DEST. */ + tree ref = dest; + tree off = NULL_TREE; + if (TREE_CODE (ref) == ARRAY_REF) + { + /* Handle stores to VLAs (represented as + ARRAY_REF (MEM_REF (vlaptr, 0), N]. */ + off = TREE_OPERAND (ref, 1); + ref = TREE_OPERAND (ref, 0); + } + + if (TREE_CODE (ref) == MEM_REF) + { + tree mem_off = TREE_OPERAND (ref, 1); + if (off) + { + if (!integer_zerop (mem_off)) + return; + } + else + off = mem_off; + ref = TREE_OPERAND (ref, 0); + } + + if (int idx = get_stridx (ref, offrng)) + { + si = get_strinfo (idx); + if (off && TREE_CODE (off) == INTEGER_CST) + { + wide_int wioff = wi::to_wide (off, offrng->get_precision ()); + offrng[0] += wioff; + offrng[1] += wioff; + } + } + else + return; + } + + /* Return early if the DESTSIZE size expression is the same as LEN + and the offset into the destination is zero. This might happen + in the case of a pair of malloc and memset calls to allocate + an object and clear it as if by calloc. */ + if (destsize == len && !plus_one && offrng[0] == 0 && offrng[0] == offrng[1]) + return; + + wide_int lenrng[2]; + if (!get_range (len, lenrng, rvals)) + return; + + if (plus_one) + { + lenrng[0] += 1; + lenrng[1] += 1; + } + + /* Compute the range of sizes of the destination object. The range + is constant for declared objects but may be a range for allocated + objects. */ + wide_int sizrng[2]; + if (!destsize || !get_range (destsize, sizrng, rvals)) + { + /* On failure, rather than bailing outright, use the maximum range + so that overflow in allocated objects whose size depends on + the strlen of the source can still be diagnosed below. */ + sizrng[0] = wi::zero (lenrng->get_precision ()); + sizrng[1] = wi::to_wide (TYPE_MAX_VALUE (ptrdiff_type_node)); + } + + /* The size of the remaining space in the destination computed as + the size of the latter minus the offset into it. */ + wide_int spcrng[2] = { sizrng[0], sizrng[1] }; + if (wi::sign_mask (offrng[0])) + { + /* FIXME: Handle negative offsets into allocated objects. */ + if (destdecl) + spcrng[0] = spcrng[1] = wi::zero (spcrng->get_precision ()); + else + return; + } + else + { + spcrng[0] -= wi::ltu_p (offrng[0], spcrng[0]) ? offrng[0] : spcrng[0]; + spcrng[1] -= wi::ltu_p (offrng[0], spcrng[1]) ? offrng[0] : spcrng[1]; + } + + if (wi::leu_p (lenrng[0], spcrng[0])) + return; + + if (lenrng[0] == spcrng[1] + && (len != destsize + || !si || !is_strlen_related_p (si->ptr, len))) + return; + + location_t loc = gimple_nonartificial_location (stmt); + if (loc == UNKNOWN_LOCATION && dest && EXPR_HAS_LOCATION (dest)) + loc = tree_nonartificial_location (dest); + loc = expansion_point_location_if_in_system_header (loc); + + bool warned = false; + if (wi::leu_p (lenrng[0], spcrng[1])) + { + if (len != destsize + && (!si || !is_strlen_related_p (si->ptr, len))) + return; + + warned = (writefn + ? warning_at (loc, OPT_Wstringop_overflow_, + "%G%qD writing one too many bytes into a region " + "of a size that depends on %", + stmt, writefn) + : warning_at (loc, OPT_Wstringop_overflow_, + "%Gwriting one too many bytes into a region " + "of a size that depends on %", + stmt)); + } + else if (lenrng[0] == lenrng[1]) + { + if (spcrng[0] == spcrng[1]) + warned = (writefn + ? warning_n (loc, OPT_Wstringop_overflow_, + lenrng[0].to_uhwi (), + "%G%qD writing %wu byte into a region " + "of size %wu", + "%G%qD writing %wu bytes into a region " + "of size %wu", + stmt, writefn, lenrng[0].to_uhwi (), + spcrng[0].to_uhwi ()) + : warning_n (loc, OPT_Wstringop_overflow_, + lenrng[0].to_uhwi (), + "%Gwriting %wu byte into a region " + "of size %wu", + "%Gwriting %wu bytes into a region " + "of size %wu", + stmt, lenrng[0].to_uhwi (), + spcrng[0].to_uhwi ())); + else + warned = (writefn + ? warning_n (loc, OPT_Wstringop_overflow_, + lenrng[0].to_uhwi (), + "%G%qD writing %wu byte into a region " + "of size between %wu and %wu", + "%G%qD writing %wu bytes into a region " + "of size between %wu and %wu", + stmt, writefn, lenrng[0].to_uhwi (), + spcrng[0].to_uhwi (), spcrng[1].to_uhwi ()) + : warning_n (loc, OPT_Wstringop_overflow_, + lenrng[0].to_uhwi (), + "%Gwriting %wu byte into a region " + "of size between %wu and %wu", + "%Gwriting %wu bytes into a region " + "of size between %wu and %wu", + stmt, lenrng[0].to_uhwi (), + spcrng[0].to_uhwi (), spcrng[1].to_uhwi ())); + } + else if (spcrng[0] == spcrng[1]) + warned = (writefn + ? warning_at (loc, OPT_Wstringop_overflow_, + "%G%qD writing between %wu and %wu bytes " + "into a region of size %wu", + stmt, writefn, lenrng[0].to_uhwi (), + lenrng[1].to_uhwi (), + spcrng[0].to_uhwi ()) + : warning_at (loc, OPT_Wstringop_overflow_, + "%Gwriting between %wu and %wu bytes " + "into a region of size %wu", + stmt, lenrng[0].to_uhwi (), + lenrng[1].to_uhwi (), + spcrng[0].to_uhwi ())); + else + warned = (writefn + ? warning_at (loc, OPT_Wstringop_overflow_, + "%G%qD writing between %wu and %wu bytes " + "into a region of size between %wu and %wu", + stmt, writefn, lenrng[0].to_uhwi (), + lenrng[1].to_uhwi (), + spcrng[0].to_uhwi (), spcrng[1].to_uhwi ()) + : warning_at (loc, OPT_Wstringop_overflow_, + "%Gwriting between %wu and %wu bytes " + "into a region of size between %wu and %wu", + stmt, lenrng[0].to_uhwi (), + lenrng[1].to_uhwi (), + spcrng[0].to_uhwi (), spcrng[1].to_uhwi ())); + + if (!warned) + return; + + /* If DESTOFF is not null, use it to format the offset value/range. */ + if (destoff) + get_range (destoff, offrng); + + /* Format the offset to keep the number of inform calls from growing + out of control. */ + char offstr[64]; + if (offrng[0] == offrng[1]) + sprintf (offstr, "%lli", (long long) offrng[0].to_shwi ()); + else + sprintf (offstr, "[%lli, %lli]", + (long long) offrng[0].to_shwi (), (long long) offrng[1].to_shwi ()); + + if (destdecl) + { + if (tree size = DECL_SIZE_UNIT (destdecl)) + inform (DECL_SOURCE_LOCATION (destdecl), + "at offset %s to object %qD with size %E declared here", + offstr, destdecl, size); + else + inform (DECL_SOURCE_LOCATION (destdecl), + "at offset %s to object %qD declared here", + offstr, destdecl); + return; + } +} + +/* Convenience wrapper for the above. */ + +static inline void +maybe_warn_overflow (gimple *stmt, unsigned HOST_WIDE_INT len, + const vr_values *rvals = NULL, + strinfo *si = NULL, bool plus_one = false) +{ + maybe_warn_overflow (stmt, build_int_cst (size_type_node, len), rvals, + si, plus_one); +} + /* Handle a strlen call. If strlen of the argument is known, replace the strlen call with the known value, otherwise remember that strlen of the argument is stored in the lhs SSA_NAME. */ @@ -4323,6 +4694,13 @@ handle_store (gimple_stmt_iterator *gsi, bool *zer else if (si == NULL || compare_nonzero_chars (si, offset, rvals) < 0) { *zero_write = initializer_zerop (rhs); + + bool dummy; + unsigned lenrange[] = { UINT_MAX, 0, 0 }; + if (count_nonzero_bytes (rhs, lenrange, &dummy, &dummy, &dummy, + rvals)) + maybe_warn_overflow (stmt, lenrange[2], rvals); + return true; } } @@ -4360,36 +4738,7 @@ handle_store (gimple_stmt_iterator *gsi, bool *zer rhs_minlen = lenrange[0]; storing_nonzero_p = lenrange[1] > 0; *zero_write = storing_all_zeros_p; - - /* Avoid issuing multiple warnings for the same LHS or statement. - For example, -Warray-bounds may have already been issued for - an out-of-bounds subscript. */ - if (!TREE_NO_WARNING (lhs) && !gimple_no_warning_p (stmt)) - { - /* Set to the declaration referenced by LHS (if known). */ - tree decl = NULL_TREE; - if (tree dstsize = compute_objsize (lhs, 1, &decl)) - if (compare_tree_int (dstsize, lenrange[2]) < 0) - { - /* Fall back on the LHS location if the statement - doesn't have one. */ - location_t loc = gimple_nonartificial_location (stmt); - if (loc == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (lhs)) - loc = tree_nonartificial_location (lhs); - loc = expansion_point_location_if_in_system_header (loc); - if (warning_n (loc, OPT_Wstringop_overflow_, - lenrange[2], - "%Gwriting %u byte into a region of size %E", - "%Gwriting %u bytes into a region of size %E", - stmt, lenrange[2], dstsize)) - { - if (decl) - inform (DECL_SOURCE_LOCATION (decl), - "destination object declared here"); - gimple_set_no_warning (stmt, true); - } - } - } + maybe_warn_overflow (stmt, lenrange[2], rvals); } else {