From patchwork Thu Jun 8 09:30:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Priour X-Patchwork-Id: 104891 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp153350vqr; Thu, 8 Jun 2023 02:32:45 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4QLLjbGBluqLmp/CAdKx3ngxwftH2nIyYrEgKCy+kfZBc8r5ZcKpmUlubOOfmbGdjIOSOW X-Received: by 2002:aa7:dd0e:0:b0:510:f1dc:86c0 with SMTP id i14-20020aa7dd0e000000b00510f1dc86c0mr6454366edv.32.1686216765115; Thu, 08 Jun 2023 02:32:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1686216765; cv=none; d=google.com; s=arc-20160816; b=x/Ld2/5PRGY+X3GAzWDk4AFPYwuDzvs2wJJIOjl4w94GBl80T96jj+7iIZ9x7f7XoL DXO09UCE1V+xZUgEgW+0svrTS+JkTj0nvJGMti2sDmQgImI/Byx/J+FTs7f6EK05o/zr Ah2kyY9iR+ijv9o5X4HOJFixOrwV7NEwwd82ncKWYQ0bNnhtZyWAqOwT0RQgi1iKe21T eo115I42MlEhoObjvWaCYF2U03PWD8SSEB3qs5gZfLgp+WCP0Wq/mj2Ttu/eYNYJKrFD P5bVmydEKzcWyMpq3U3HFlNsKv3cnTffmdbeaK0t/9MkMKCGavxFzAJ9X2wXhPonYm5x lV/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:cc :to:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=RPskdFzT4banUHsWLSpbd5jawQfSN7ZjwKzjSWPde2w=; b=jTji02yIbyUnMfIHx5fvVZH4dcnwljtQvcY6xwbH6Mdc2yNzq2eDyDWJpUIfi9/vN1 nCj/rz48+8QF8ZO/M0WqlZs9Lsng9ITYAX+Bqzc34OIsRx/Rm0JSRD78nUCECFyo67Gw G6N2WTG9SynrwKc1BISdd8Gm7vubcCTJFhNqlxu7O4CDmPtaRqEbIL4Ug8nvL4hx9HG8 +9E4oG+yWRWEMXUUAF4MU7OV4dt2sW02wbjDHGz0zwcy2N5T2hP5QIgvGm4zlRYsmZD2 FWP77NS38LrPpRHtNt01L50KWKJzCXduv67NJeKuyyBRvltb3CMDjRgXKreapKvRKWQa wjtw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=IkQj6Ms2; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id p26-20020aa7cc9a000000b005148f8c24d5si435881edt.382.2023.06.08.02.32.44 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 08 Jun 2023 02:32:45 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=IkQj6Ms2; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id DB71A385781F for ; Thu, 8 Jun 2023 09:32:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DB71A385781F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1686216763; bh=RPskdFzT4banUHsWLSpbd5jawQfSN7ZjwKzjSWPde2w=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=IkQj6Ms2IvpCnv0EF9X9z+n4refT9LHdLk6NFqXwCOGFOl8TsMizbIWp1LFAICb9M BPUmKwKBfy6QxNaIg9LB94kwYR5SkHpPfaTwLgJ3DK5ATtCHcIDCnoZFaebNJ3CgQu KH++fIdWpwP6U7BRhl1eJW+oq6nA3K3rjlFvY+5g= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lf1-x131.google.com (mail-lf1-x131.google.com [IPv6:2a00:1450:4864:20::131]) by sourceware.org (Postfix) with ESMTPS id 993C13858C62; Thu, 8 Jun 2023 09:31:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 993C13858C62 Received: by mail-lf1-x131.google.com with SMTP id 2adb3069b0e04-4f60a27c4a2so483582e87.2; Thu, 08 Jun 2023 02:31:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686216717; x=1688808717; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=RPskdFzT4banUHsWLSpbd5jawQfSN7ZjwKzjSWPde2w=; b=bJvX/cqb8/q2FfEE0lPL5/8KMyz1WtAqX2Pq3nPOYYk4khzTQ1LXvDi4xpyPl5JHdT ZWjjH2MOKvHxl/VPW3HnwjRBxcPNG3t9sSMbFzyEiOqtqyDJq3H+fwP72PV/+d18rFd6 pWGkoGS5RVBvnWUDGWk8D+gRX7F4lEygFPmiZPgM44FidWEXyefi+Y51xXVxR7en1ZJu Ygdck1D87LQ02a+E/kBdXZ+VUiLTZCRMzP+HUSKSTjxQmpgsTrYtrLkimT4+WVJxlTAq mzDqL0fJ97QIkSavDE/v3vDm/N8Brd8jq0GvjJuhRqa9M6h885a+JIxBbfp6McPvunBR vEPg== X-Gm-Message-State: AC+VfDwKIyXP2Gs4BXgZxZRIcbB7ekMAqvlySEmCpp4Ux8eaL+grxXXs vsYxq9VqYC2GjG4eAYWhfv2GfQxjE0GG X-Received: by 2002:ac2:5314:0:b0:4f4:b864:1d96 with SMTP id c20-20020ac25314000000b004f4b8641d96mr2758973lfh.32.1686216716728; Thu, 08 Jun 2023 02:31:56 -0700 (PDT) Received: from localhost (h-155-4-156-99.A163.priv.bahnhof.se. [155.4.156.99]) by smtp.gmail.com with UTF8SMTPSA id j20-20020a19f514000000b004eae73a0530sm123139lfb.39.2023.06.08.02.31.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 08 Jun 2023 02:31:56 -0700 (PDT) X-Google-Original-From: vultkayn@gcc.gnu.org To: gcc-patches@gcc.gnu.org Cc: Benjamin Priour Subject: [COMMITTED] analyzer: Standalone OOB-warning, formatting fixed [PR109437, PR109439] Date: Thu, 8 Jun 2023 11:30:50 +0200 Message-Id: <20230608093048.1677718-1-vultkayn@gcc.gnu.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Benjamin Priour via Gcc-patches From: Benjamin Priour Reply-To: priour.be@gmail.com Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1768126430786866355?= X-GMAIL-MSGID: =?utf-8?q?1768126430786866355?= From: Benjamin Priour For the record, below is the previous patch I submitted, with the little formatting issues fixed - multiline docstring no ends on a newline. It was otherwise validated by David Malcolm, so I already committed it. This patch enhances -Wanalyzer-out-of-bounds that is no longer paired with a -Wanalyzer-use-of-uninitialized-value on out-of-bounds-read. This also fixes PR analyzer/109437. Before there could always be at most one OOB-read warning per frame because -Wanalyzer-use-of-uninitialized-value always terminates the analysis path. PR analyzer/109439 gcc/analyzer/ChangeLog: * bounds-checking.cc (region_model::check_symbolic_bounds): Returns whether the BASE_REG region access was OOB. (region_model::check_region_bounds): Likewise. * region-model.cc (region_model::get_store_value): Creates an unknown svalue on OOB-read access to REG. (region_model::check_region_access): Returns whether an unknown svalue needs be created. (region_model::check_region_for_read): Passes check_region_access return value. * region-model.h: Update prior function definitions. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/out-of-bounds-2.c: Cleaned test for uninitialized-value warning. * gcc.dg/analyzer/out-of-bounds-5.c: Likewise. * gcc.dg/analyzer/pr101962.c: Likewise. * gcc.dg/analyzer/realloc-5.c: Likewise. * gcc.dg/analyzer/pr109439.c: New test. --- gcc/analyzer/bounds-checking.cc | 28 +++++++++++++------ gcc/analyzer/region-model.cc | 22 +++++++++------ gcc/analyzer/region-model.h | 8 +++--- .../gcc.dg/analyzer/out-of-bounds-2.c | 1 - .../gcc.dg/analyzer/out-of-bounds-5.c | 2 -- gcc/testsuite/gcc.dg/analyzer/pr101962.c | 1 - gcc/testsuite/gcc.dg/analyzer/pr109439.c | 12 ++++++++ gcc/testsuite/gcc.dg/analyzer/realloc-5.c | 1 - 8 files changed, 49 insertions(+), 26 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr109439.c diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index 3bf542a8eba..a5692cf9319 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -767,9 +767,10 @@ public: } }; -/* Check whether an access is past the end of the BASE_REG. */ +/* Check whether an access is past the end of the BASE_REG. + Return TRUE if the access was valid, FALSE otherwise. */ -void +bool region_model::check_symbolic_bounds (const region *base_reg, const svalue *sym_byte_offset, const svalue *num_bytes_sval, @@ -800,6 +801,7 @@ region_model::check_symbolic_bounds (const region *base_reg, offset_tree, num_bytes_tree, capacity_tree)); + return false; break; case DIR_WRITE: ctxt->warn (make_unique (base_reg, @@ -807,9 +809,11 @@ region_model::check_symbolic_bounds (const region *base_reg, offset_tree, num_bytes_tree, capacity_tree)); + return false; break; } } + return true; } static tree @@ -822,9 +826,10 @@ maybe_get_integer_cst_tree (const svalue *sval) return NULL_TREE; } -/* May complain when the access on REG is out-of-bounds. */ +/* May complain when the access on REG is out-of-bounds. + Return TRUE if the access was valid, FALSE otherwise. */ -void +bool region_model::check_region_bounds (const region *reg, enum access_direction dir, region_model_context *ctxt) const @@ -839,14 +844,14 @@ region_model::check_region_bounds (const region *reg, (e.g. because the analyzer did not see previous offsets on the latter, it might think that a negative access is before the buffer). */ if (base_reg->symbolic_p ()) - return; + return true; /* Find out how many bytes were accessed. */ const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr); tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval); /* Bail out if 0 bytes are accessed. */ if (num_bytes_tree && zerop (num_bytes_tree)) - return; + return true; /* Get the capacity of the buffer. */ const svalue *capacity = get_capacity (base_reg); @@ -877,13 +882,13 @@ region_model::check_region_bounds (const region *reg, } else byte_offset_sval = reg_offset.get_symbolic_byte_offset (); - check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval, + return check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval, capacity, dir, ctxt); - return; } /* Otherwise continue to check with concrete values. */ byte_range out (0, 0); + bool oob_safe = true; /* NUM_BYTES_TREE should always be interpreted as unsigned. */ byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree); byte_range read_bytes (offset, num_bytes_unsigned); @@ -899,10 +904,12 @@ region_model::check_region_bounds (const region *reg, case DIR_READ: ctxt->warn (make_unique (reg, diag_arg, out)); + oob_safe = false; break; case DIR_WRITE: ctxt->warn (make_unique (reg, diag_arg, out)); + oob_safe = false; break; } } @@ -911,7 +918,7 @@ region_model::check_region_bounds (const region *reg, do a symbolic check here because the inequality check does not reason whether constants are greater than symbolic values. */ if (!cst_capacity_tree) - return; + return oob_safe; byte_range buffer (0, wi::to_offset (cst_capacity_tree)); /* If READ_BYTES exceeds BUFFER, we do have an overflow. */ @@ -929,13 +936,16 @@ region_model::check_region_bounds (const region *reg, case DIR_READ: ctxt->warn (make_unique (reg, diag_arg, out, byte_bound)); + oob_safe = false; break; case DIR_WRITE: ctxt->warn (make_unique (reg, diag_arg, out, byte_bound)); + oob_safe = false; break; } } + return oob_safe; } } // namespace ana diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 3bb3df2f063..fb96cd54940 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -2396,7 +2396,8 @@ region_model::get_store_value (const region *reg, if (reg->empty_p ()) return m_mgr->get_or_create_unknown_svalue (reg->get_type ()); - check_region_for_read (reg, ctxt); + if (check_region_for_read (reg, ctxt)) + return m_mgr->get_or_create_unknown_svalue(reg->get_type()); /* Special-case: handle var_decls in the constant pool. */ if (const decl_region *decl_reg = reg->dyn_cast_decl_region ()) @@ -2802,19 +2803,22 @@ region_model::get_string_size (const region *reg) const } /* If CTXT is non-NULL, use it to warn about any problems accessing REG, - using DIR to determine if this access is a read or write. */ + using DIR to determine if this access is a read or write. + Return TRUE if an UNKNOWN_SVALUE needs be created. */ -void +bool region_model::check_region_access (const region *reg, enum access_direction dir, region_model_context *ctxt) const { /* Fail gracefully if CTXT is NULL. */ if (!ctxt) - return; + return false; + bool need_unknown_sval = false; check_region_for_taint (reg, dir, ctxt); - check_region_bounds (reg, dir, ctxt); + if (!check_region_bounds (reg, dir, ctxt)) + need_unknown_sval = true; switch (dir) { @@ -2827,6 +2831,7 @@ region_model::check_region_access (const region *reg, check_for_writable_region (reg, ctxt); break; } + return need_unknown_sval; } /* If CTXT is non-NULL, use it to warn about any problems writing to REG. */ @@ -2838,13 +2843,14 @@ region_model::check_region_for_write (const region *dest_reg, check_region_access (dest_reg, DIR_WRITE, ctxt); } -/* If CTXT is non-NULL, use it to warn about any problems reading from REG. */ +/* If CTXT is non-NULL, use it to warn about any problems reading from REG. + Returns TRUE if an unknown svalue needs be created. */ -void +bool region_model::check_region_for_read (const region *src_reg, region_model_context *ctxt) const { - check_region_access (src_reg, DIR_READ, ctxt); + return check_region_access (src_reg, DIR_READ, ctxt); } /* Concrete subclass for casts of pointers that lead to trailing bytes. */ diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index fe3db0b0c98..12f84b20463 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -553,22 +553,22 @@ private: void check_for_writable_region (const region* dest_reg, region_model_context *ctxt) const; - void check_region_access (const region *reg, + bool check_region_access (const region *reg, enum access_direction dir, region_model_context *ctxt) const; - void check_region_for_read (const region *src_reg, + bool check_region_for_read (const region *src_reg, region_model_context *ctxt) const; void check_region_size (const region *lhs_reg, const svalue *rhs_sval, region_model_context *ctxt) const; /* Implemented in bounds-checking.cc */ - void check_symbolic_bounds (const region *base_reg, + bool check_symbolic_bounds (const region *base_reg, const svalue *sym_byte_offset, const svalue *num_bytes_sval, const svalue *capacity, enum access_direction dir, region_model_context *ctxt) const; - void check_region_bounds (const region *reg, enum access_direction dir, + bool check_region_bounds (const region *reg, enum access_direction dir, region_model_context *ctxt) const; void check_call_args (const call_details &cd) const; diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c index 1330090f348..336f624441c 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c @@ -82,5 +82,4 @@ void test5 (void) /* { dg-warning "heap-based buffer over-read" "bounds warning" { target *-*-* } test5 } */ /* { dg-message "read of 4 bytes from after the end of the region" "num bad bytes note" { target *-*-* } test5 } */ - /* { dg-warning "use of uninitialized value" "uninit warning" { target *-*-* } test5 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c index 2a61d8ca236..568f9cad199 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c @@ -68,7 +68,6 @@ void test8 (size_t size, size_t offset) char dst[size]; memcpy (dst, src, size + offset); /* { dg-line test8 } */ /* { dg-warning "over-read" "warning" { target *-*-* } test8 } */ - /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } test8 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test8 } */ } @@ -78,7 +77,6 @@ void test9 (size_t size, size_t offset) int32_t dst[size]; memcpy (dst, src, 4 * size + 1); /* { dg-line test9 } */ /* { dg-warning "over-read" "warning" { target *-*-* } test9 } */ - /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } test9 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test9 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c b/gcc/testsuite/gcc.dg/analyzer/pr101962.c index 08c0aba5cbf..b878aad9cf1 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c @@ -24,7 +24,6 @@ test_1 (void) __analyzer_eval (a != NULL); /* { dg-warning "TRUE" } */ return *a; /* { dg-line test_1 } */ - /* { dg-warning "use of uninitialized value '\\*a'" "warning" { target *-*-* } test_1 } */ /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test_1 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr109439.c b/gcc/testsuite/gcc.dg/analyzer/pr109439.c new file mode 100644 index 00000000000..01c87cf171c --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/pr109439.c @@ -0,0 +1,12 @@ +int would_like_only_oob (int i) +{ + int arr[] = {1,2,3,4,5,6,7}; + arr[10] = 9; /* { dg-warning "stack-based buffer overflow" } */ + arr[11] = 15; /* { dg-warning "stack-based buffer overflow" } */ + int y1 = arr[9]; /* { dg-warning "stack-based buffer over-read" } */ + /* { dg-bogus "use of uninitialized value" "" { target *-*-* } .-1 } */ + + arr[18] = 15; /* { dg-warning "stack-based buffer overflow" } */ + + return y1; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c index 137e05b87aa..f65f2c6ca25 100644 --- a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c @@ -40,7 +40,6 @@ void test_1 () /* { dg-warning "UNKNOWN" "warning" { target *-*-* } eval } */ /* { dg-warning "heap-based buffer over-read" "warning" { target *-*-* } eval } */ - /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } eval } */ } free (q);