From patchwork Thu Aug 11 17:24:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Lange X-Patchwork-Id: 56684 X-Patchwork-Delegate: dmalcolm@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3DD6E385609A for ; Thu, 11 Aug 2022 17:26:05 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from www523.your-server.de (www523.your-server.de [159.69.224.22]) by sourceware.org (Postfix) with ESMTPS id 935ED3856251 for ; Thu, 11 Aug 2022 17:25:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 935ED3856251 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tim-lange.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tim-lange.me DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tim-lange.me; s=default2108; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=H8/zl94gO5lUjoS9SpUcVYA6SnOwR66/jSEvNWHPmvY=; b=PT64vXKrJnVEbQm0fUcT3AZcN4 AKlxCNnkbubUVEgBI1ffY9alsDBbZozEDCVJwRh2v89kmXg+dM4Xj5za0IOHfAHjbEuXXPxe7ZubD eDv8YPg29d7svYqWcb2kyE4TQfgoqq2IxoRugwbcRye4WNmzXkeqmOm2W8psfjmCPPef3Y8hzPPLi 6o7doh61xrFStNDYVpofOmiIFIK1IRTFZU/lp/m+K7a654I40heKOengmjIctWBX1NJBLQ/fB1DfB o9joZ2r1oVVF3Kfy/zFw57ESClk5AmmguAQpRLf/E9EXRuP0fDJqR57ZH9MceC73U9SsC0uyG2YKd hmcS2HEA==; Received: from sslproxy05.your-server.de ([78.46.172.2]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1oMBvc-0006uE-ND; Thu, 11 Aug 2022 19:25:04 +0200 Received: from [2a02:908:1861:d6a0::6b5] (helo=fedora..) by sslproxy05.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oMBvc-000RVC-H5; Thu, 11 Aug 2022 19:25:04 +0200 From: Tim Lange To: gcc-patches@gcc.gnu.org Subject: [PATCH 1/2 v2] analyzer: consider that realloc could shrink the buffer [PR106539] Date: Thu, 11 Aug 2022 19:24:51 +0200 Message-Id: <20220811172452.65996-1-mail@tim-lange.me> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220809211943.82098-1-mail@tim-lange.me> References: <20220809211943.82098-1-mail@tim-lange.me> MIME-Version: 1.0 X-Authenticated-Sender: mail@tim-lange.me X-Virus-Scanned: Clear (ClamAV 0.103.6/26624/Thu Aug 11 09:52:26 2022) X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_INFOUSMEBIZ, 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: , Cc: Tim Lange Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch adds the "shrinks buffer" case to the success_with_move modelling of realloc. Regression-tested on Linux x86-64, further ran the analyzer tests with the -m32 option. 2022-08-11 Tim Lange gcc/analyzer/ChangeLog: PR analyzer/106539 * region-model-impl-calls.cc (region_model::impl_call_realloc): Use the result of get_copied_size as the size for the sized_regions in realloc. (success_with_move::get_copied_size): New function. gcc/testsuite/ChangeLog: PR analyzer/106539 * gcc.dg/analyzer/pr106539.c: New test. * gcc.dg/analyzer/realloc-5.c: New test. --- gcc/analyzer/region-model-impl-calls.cc | 48 ++++++++++++++++++++--- gcc/testsuite/gcc.dg/analyzer/pr106539.c | 15 +++++++ gcc/testsuite/gcc.dg/analyzer/realloc-5.c | 45 +++++++++++++++++++++ 3 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/pr106539.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/realloc-5.c diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index 8c38e9206fa..fa0ec88b1f4 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -732,15 +732,17 @@ region_model::impl_call_realloc (const call_details &cd) const svalue *old_size_sval = model->get_dynamic_extents (freed_reg); if (old_size_sval) { - const region *sized_old_reg + const svalue *copied_size_sval + = get_copied_size (old_size_sval, new_size_sval); + const region *copied_old_reg = model->m_mgr->get_sized_region (freed_reg, NULL, - old_size_sval); + copied_size_sval); const svalue *buffer_content_sval - = model->get_store_value (sized_old_reg, cd.get_ctxt ()); - const region *sized_new_reg + = model->get_store_value (copied_old_reg, cd.get_ctxt ()); + const region *copied_new_reg = model->m_mgr->get_sized_region (new_reg, NULL, - old_size_sval); - model->set_value (sized_new_reg, buffer_content_sval, + copied_size_sval); + model->set_value (copied_new_reg, buffer_content_sval, cd.get_ctxt ()); } else @@ -774,6 +776,40 @@ region_model::impl_call_realloc (const call_details &cd) else return true; } + + private: + /* Return the lesser of OLD_SIZE_SVAL and NEW_SIZE_SVAL. + If either one is symbolic, the symbolic svalue is returned. */ + const svalue *get_copied_size (const svalue *old_size_sval, + const svalue *new_size_sval) const + { + tree old_size_cst = old_size_sval->maybe_get_constant (); + tree new_size_cst = new_size_sval->maybe_get_constant (); + + if (old_size_cst && new_size_cst) + { + /* Both are constants and comparable. */ + tree cmp = fold_binary (LT_EXPR, boolean_type_node, + old_size_cst, new_size_cst); + + if (cmp == boolean_true_node) + return old_size_sval; + else + return new_size_sval; + } + else if (new_size_cst) + { + /* OLD_SIZE_SVAL is symbolic, so return that. */ + return old_size_sval; + } + else + { + /* NEW_SIZE_SVAL is symbolic or both are symbolic. + Return NEW_SIZE_SVAL, because implementations of realloc + probably only moves the buffer if the new size is larger. */ + return new_size_sval; + } + } }; /* Body of region_model::impl_call_realloc. */ diff --git a/gcc/testsuite/gcc.dg/analyzer/pr106539.c b/gcc/testsuite/gcc.dg/analyzer/pr106539.c new file mode 100644 index 00000000000..fd270868e36 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/pr106539.c @@ -0,0 +1,15 @@ +#include + +void *test (void) +{ + void **p = (void **)malloc (sizeof (void *) * 2); + if (!p) + return NULL; + p[0] = malloc(10); + p[1] = malloc(20); /* { dg-message "allocated here" } */ + void *q = realloc (p, sizeof (void *)); /* { dg-message "when 'realloc' succeeds, moving buffer" } */ + if (!q) + /* { dg-warning "leak of ''" "leak of unknown" { target *-*-* } .-1 } */ + return p; + return q; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c new file mode 100644 index 00000000000..2efe3371e12 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c @@ -0,0 +1,45 @@ +#include "analyzer-decls.h" + +typedef __SIZE_TYPE__ size_t; + +#define NULL ((void *)0) + +extern void *malloc (size_t __size) + __attribute__ ((__nothrow__ , __leaf__)) + __attribute__ ((__malloc__)) + __attribute__ ((__alloc_size__ (1))); +extern void *realloc (void *__ptr, size_t __size) + __attribute__ ((__nothrow__ , __leaf__)) + __attribute__ ((__warn_unused_result__)) + __attribute__ ((__alloc_size__ (2))); +extern void free (void *__ptr) + __attribute__ ((__nothrow__ , __leaf__)); +extern void *memset (void *__ptr, int __value, size_t __size); + +/* realloc where the region shrinks on success_with_move. */ + +void test_1 () +{ + char *p = malloc (16); + if (!p) + return; + memset (p, 1, 16); + + char *q = realloc (p, 8); + if (!q) + { + free (p); + return; + } + else if (p != q) + { + __analyzer_dump_capacity (q); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)8'" } */ + __analyzer_eval (q[8] == 1); /* { dg-line eval } */ + + /* { dg-warning "UNKNOWN" "warning" { target *-*-* } eval } */ + /* { dg-warning "overread" "warning" { target *-*-* } eval } */ + /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } eval } */ + } + + free (q); +} From patchwork Thu Aug 11 17:24:52 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Lange X-Patchwork-Id: 56685 X-Patchwork-Delegate: dmalcolm@redhat.com Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 0801D3856DF5 for ; Thu, 11 Aug 2022 17:26:36 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from www523.your-server.de (www523.your-server.de [159.69.224.22]) by sourceware.org (Postfix) with ESMTPS id 4C06A3856241 for ; Thu, 11 Aug 2022 17:25:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4C06A3856241 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=tim-lange.me Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tim-lange.me DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tim-lange.me; s=default2108; h=Content-Transfer-Encoding:MIME-Version: References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: Content-Type:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=6fne2BvzbWIiLa94iWg5eUkuLE2sfgbpxx/U6JXs52Q=; b=NB1QKMPiFPn/kI6VL/rQT1aTPj MwwHD7q2JJdDU4LKcqnlet9hJcTCxqCgmD/5DYWKcPm3sMnrZH675hgH6xb5V1EO+uu0HmS9NhVer 9Fz/lWI0wcIGhs0yudBDSb1Ac0wJe98oVJvoxmDWSKL5EQx0qncOYQajvsSR/SS+Da+aTsO1BtRM2 dnSJJ3IlGg1WtI3idC1Cd+OKkrH/4pmRzfbEmskkkjkbWAzZxlLrDLw99ujQsALqoKSIJhrlQq5AH w/FpuSRdOOHX8JOwBlgqa9dHe2Gw6UnI70/YaMJFwRfaE7ZdlFQFl+vpHjIUVA2OyEJ29Z8PeNyqG qq38oIGA==; Received: from sslproxy05.your-server.de ([78.46.172.2]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1oMBve-0006uV-8I; Thu, 11 Aug 2022 19:25:06 +0200 Received: from [2a02:908:1861:d6a0::6b5] (helo=fedora..) by sslproxy05.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oMBve-000RVC-0f; Thu, 11 Aug 2022 19:25:06 +0200 From: Tim Lange To: gcc-patches@gcc.gnu.org Subject: [PATCH 2/2 v2] analyzer: out-of-bounds checker [PR106000] Date: Thu, 11 Aug 2022 19:24:52 +0200 Message-Id: <20220811172452.65996-2-mail@tim-lange.me> X-Mailer: git-send-email 2.37.1 In-Reply-To: <20220811172452.65996-1-mail@tim-lange.me> References: <20220809211943.82098-1-mail@tim-lange.me> <20220811172452.65996-1-mail@tim-lange.me> MIME-Version: 1.0 X-Authenticated-Sender: mail@tim-lange.me X-Virus-Scanned: Clear (ClamAV 0.103.6/26624/Thu Aug 11 09:52:26 2022) X-Spam-Status: No, score=-12.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_INFOUSMEBIZ, 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: , Cc: Tim Lange Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch adds an experimental out-of-bounds checker to the analyzer. The checker was tested on coreutils, curl, httpd and openssh. It is mostly accurate but does produce false-positives on yacc-generated files and sometimes when the analyzer misses an invariant. These cases will be documented in bugzilla. Regression-tested on Linux x86-64, further ran the analyzer tests with the -m32 option. 2022-08-11 Tim Lange gcc/analyzer/ChangeLog: PR analyzer/106000 * analyzer.opt: Add Wanalyzer-out-of-bounds. * region-model.cc (class out_of_bounds): Diagnostics base class for all out-of-bounds diagnostics. (class past_the_end): Base class derived from out_of_bounds for the buffer_overflow and buffer_overread diagnostics. (class buffer_overflow): Buffer overflow diagnostics. (class buffer_overread): Buffer overread diagnostics. (class buffer_underflow): Buffer underflow diagnostics. (class buffer_underread): Buffer overread diagnostics. (region_model::check_region_bounds): New function to check region bounds for out-of-bounds accesses. (region_model::check_region_access): Add call to check_region_bounds. (region_model::get_representative_tree): New function that accepts a region instead of an svalue. * region-model.h (class region_model): Add region_model::check_region_bounds. * region.cc (region::symbolic_p): New predicate. (offset_region::get_byte_size_sval): Only return the remaining byte size on offset_regions. * region.h: Add region::symbolic_p. * store.cc (byte_range::intersects_p): Add new function equivalent to bit_range::intersects_p. (byte_range::exceeds_p): New function. (byte_range::falls_short_of_p): New function. * store.h (struct byte_range): Add byte_range::intersects_p, byte_range::exceeds_p and byte_range::falls_short_of_p. gcc/ChangeLog: PR analyzer/106000 * doc/invoke.texi: Add Wanalyzer-out-of-bounds. gcc/testsuite/ChangeLog: PR analyzer/106000 * g++.dg/analyzer/pr100244.C: Disable out-of-bounds warning. * gcc.dg/analyzer/allocation-size-3.c: Disable out-of-bounds warning. * gcc.dg/analyzer/memcpy-2.c: Disable out-of-bounds warning. * gcc.dg/analyzer/pr101962.c: Add dg-warning. * gcc.dg/analyzer/pr96764.c: Disable out-of-bounds warning. * gcc.dg/analyzer/pr97029.c: Add dummy buffer to prevent an out-of-bounds warning. * gcc.dg/analyzer/test-setjmp.h: Add dummy buffer to prevent an out-of-bounds warning. * gcc.dg/analyzer/zlib-3.c: Add dg-bogus. * g++.dg/analyzer/out-of-bounds-placement-new.C: New test. * gcc.dg/analyzer/out-of-bounds-1.c: New test. * gcc.dg/analyzer/out-of-bounds-2.c: New test. * gcc.dg/analyzer/out-of-bounds-3.c: New test. * gcc.dg/analyzer/out-of-bounds-container_of.c: New test. * gcc.dg/analyzer/out-of-bounds-coreutils.c: New test. * gcc.dg/analyzer/out-of-bounds-curl.c: New test. --- gcc/analyzer/analyzer.opt | 4 + gcc/analyzer/region-model.cc | 422 ++++++++++++++++++ gcc/analyzer/region-model.h | 3 + gcc/analyzer/region.cc | 32 ++ gcc/analyzer/region.h | 4 + gcc/analyzer/store.cc | 67 +++ gcc/analyzer/store.h | 9 + gcc/doc/invoke.texi | 15 + .../analyzer/out-of-bounds-placement-new.C | 19 + gcc/testsuite/g++.dg/analyzer/pr100244.C | 5 +- .../gcc.dg/analyzer/allocation-size-3.c | 2 + gcc/testsuite/gcc.dg/analyzer/memcpy-2.c | 2 +- .../gcc.dg/analyzer/out-of-bounds-1.c | 120 +++++ .../gcc.dg/analyzer/out-of-bounds-2.c | 83 ++++ .../gcc.dg/analyzer/out-of-bounds-3.c | 91 ++++ .../analyzer/out-of-bounds-container_of.c | 51 +++ .../gcc.dg/analyzer/out-of-bounds-coreutils.c | 29 ++ .../gcc.dg/analyzer/out-of-bounds-curl.c | 41 ++ gcc/testsuite/gcc.dg/analyzer/pr101962.c | 6 +- gcc/testsuite/gcc.dg/analyzer/pr96764.c | 2 + gcc/testsuite/gcc.dg/analyzer/pr97029.c | 4 +- gcc/testsuite/gcc.dg/analyzer/test-setjmp.h | 4 +- gcc/testsuite/gcc.dg/analyzer/zlib-3.c | 4 +- 23 files changed, 1012 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/analyzer/out-of-bounds-placement-new.C create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-coreutils.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-curl.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 5021376b6fb..b2a631bb9db 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -106,6 +106,10 @@ Wanalyzer-mismatching-deallocation Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning Warn about code paths in which the wrong deallocation function is called. +Wanalyzer-out-of-bounds +Common Var(warn_analyzer_out_of_bounds) Init(1) Warning +Warn about code paths in which a write or read to a buffer is out-of-bounds. + Wanalyzer-possible-null-argument Common Var(warn_analyzer_possible_null_argument) Init(1) Warning Warn about code paths in which a possibly-NULL value is passed to a must-not-be-NULL function argument. diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index f7df2fca245..a5ed19be2ff 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1268,6 +1268,414 @@ region_model::on_stmt_pre (const gimple *stmt, } } +/* Abstract base class for all out-of-bounds warnings. */ + +class out_of_bounds : public pending_diagnostic_subclass +{ +public: + out_of_bounds (const region *reg, tree diag_arg, + byte_range out_of_bounds_range) + : m_reg (reg), m_diag_arg (diag_arg), + m_out_of_bounds_range (out_of_bounds_range) + {} + + const char *get_kind () const final override + { + return "out_of_bounds_diagnostic"; + } + + bool operator== (const out_of_bounds &other) const + { + return m_reg == other.m_reg + && m_out_of_bounds_range == other.m_out_of_bounds_range + && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg); + } + + int get_controlling_option () const final override + { + return OPT_Wanalyzer_out_of_bounds; + } + + void mark_interesting_stuff (interesting_t *interest) final override + { + interest->add_region_creation (m_reg); + } + +protected: + const region *m_reg; + tree m_diag_arg; + byte_range m_out_of_bounds_range; +}; + +/* Abstract subclass to complaing about out-of-bounds + past the end of the buffer. */ + +class past_the_end : public out_of_bounds +{ +public: + past_the_end (const region *reg, tree diag_arg, byte_range range, + tree byte_bound) + : out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound) + {} + + bool operator== (const past_the_end &other) const + { + return out_of_bounds::operator== (other) + && pending_diagnostic::same_tree_p (m_byte_bound, + other.m_byte_bound); + } + + label_text + describe_region_creation_event (const evdesc::region_creation &ev) final + override + { + if (m_byte_bound && TREE_CODE (m_byte_bound) == INTEGER_CST) + return ev.formatted_print ("capacity is %E bytes", m_byte_bound); + + return label_text (); + } + +protected: + tree m_byte_bound; +}; + +/* Concrete subclass to complain about buffer overflows. */ + +class buffer_overflow : public past_the_end +{ +public: + buffer_overflow (const region *reg, tree diag_arg, + byte_range range, tree byte_bound) + : past_the_end (reg, diag_arg, range, byte_bound) + {} + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + bool warned; + switch (m_reg->get_memory_space ()) + { + default: + m.add_cwe (787); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer overflow"); + break; + case MEMSPACE_STACK: + m.add_cwe (121); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer overflow"); + break; + case MEMSPACE_HEAP: + m.add_cwe (122); + warned = warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer overflow"); + break; + } + + if (warned) + { + char num_bytes_past_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (m_out_of_bounds_range.m_size_in_bytes, + num_bytes_past_buf, UNSIGNED); + if (m_diag_arg) + inform (rich_loc->get_loc (), "write is %s bytes past the end" + " of %qE", num_bytes_past_buf, + m_diag_arg); + else + inform (rich_loc->get_loc (), "write is %s bytes past the end" + "of the region", + num_bytes_past_buf); + } + + return warned; + } + + label_text describe_final_event (const evdesc::final_event &ev) + final override + { + byte_size_t start = m_out_of_bounds_range.get_start_byte_offset (); + byte_size_t end = m_out_of_bounds_range.get_last_byte_offset (); + char start_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (start, start_buf, SIGNED); + char end_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (end, end_buf, SIGNED); + + if (start == end) + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds write at byte %s but %qE" + " ends at byte %E", start_buf, m_diag_arg, + m_byte_bound); + return ev.formatted_print ("out-of-bounds write at byte %s but region" + " ends at byte %E", start_buf, + m_byte_bound); + } + else + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds write from byte %s till" + " byte %s but %qE ends at byte %E", + start_buf, end_buf, m_diag_arg, + m_byte_bound); + return ev.formatted_print ("out-of-bounds write from byte %s till" + " byte %s but region ends at byte %E", + start_buf, end_buf, m_byte_bound); + } + } +}; + +/* Concrete subclass to complain about buffer overreads. */ + +class buffer_overread : public past_the_end +{ +public: + buffer_overread (const region *reg, tree diag_arg, + byte_range range, tree byte_bound) + : past_the_end (reg, diag_arg, range, byte_bound) + {} + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + m.add_cwe (126); + bool warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer overread"); + + if (warned) + { + char num_bytes_past_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (m_out_of_bounds_range.m_size_in_bytes, + num_bytes_past_buf, UNSIGNED); + if (m_diag_arg) + inform (rich_loc->get_loc (), "write is %s bytes past the end" + " of %qE", num_bytes_past_buf, + m_diag_arg); + else + inform (rich_loc->get_loc (), "write is %s bytes past the end" + "of the region", + num_bytes_past_buf); + } + + return warned; + } + + label_text describe_final_event (const evdesc::final_event &ev) + final override + { + byte_size_t start = m_out_of_bounds_range.get_start_byte_offset (); + byte_size_t end = m_out_of_bounds_range.get_last_byte_offset (); + char start_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (start, start_buf, SIGNED); + char end_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (end, end_buf, SIGNED); + + if (start == end) + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds read at byte %s but %qE" + " ends at byte %E", start_buf, m_diag_arg, + m_byte_bound); + return ev.formatted_print ("out-of-bounds read at byte %s but region" + " ends at byte %E", start_buf, + m_byte_bound); + } + else + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds read from byte %s till" + " byte %s but %qE ends at byte %E", + start_buf, end_buf, m_diag_arg, + m_byte_bound); + return ev.formatted_print ("out-of-bounds read from byte %s till" + " byte %s but region ends at byte %E", + start_buf, end_buf, m_byte_bound); + } + } +}; + +/* Concrete subclass to complain about buffer underflows. */ + +class buffer_underflow : public out_of_bounds +{ +public: + buffer_underflow (const region *reg, tree diag_arg, byte_range range) + : out_of_bounds (reg, diag_arg, range) + {} + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + m.add_cwe (124); + return warning_meta (rich_loc, m, get_controlling_option (), + "buffer underflow"); + } + + label_text describe_final_event (const evdesc::final_event &ev) + final override + { + byte_size_t start = m_out_of_bounds_range.get_start_byte_offset (); + byte_size_t end = m_out_of_bounds_range.get_last_byte_offset (); + char start_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (start, start_buf, SIGNED); + char end_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (end, end_buf, SIGNED); + + if (start == end) + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds write at byte %s but %qE" + " starts at byte 0", start_buf, + m_diag_arg); + return ev.formatted_print ("out-of-bounds write at byte %s but region" + " starts at byte 0", start_buf); + } + else + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds write from byte %s till" + " byte %s but %qE starts at byte 0", + start_buf, end_buf, m_diag_arg); + return ev.formatted_print ("out-of-bounds write from byte %s till" + " byte %s but region starts at byte 0", + start_buf, end_buf);; + } + } +}; + +/* Concrete subclass to complain about buffer underreads. */ + +class buffer_underread : public out_of_bounds +{ +public: + buffer_underread (const region *reg, tree diag_arg, byte_range range) + : out_of_bounds (reg, diag_arg, range) + {} + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + m.add_cwe (127); + return warning_meta (rich_loc, m, get_controlling_option (), + "buffer underread"); + } + + label_text describe_final_event (const evdesc::final_event &ev) + final override + { + byte_size_t start = m_out_of_bounds_range.get_start_byte_offset (); + byte_size_t end = m_out_of_bounds_range.get_last_byte_offset (); + char start_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (start, start_buf, SIGNED); + char end_buf[WIDE_INT_PRINT_BUFFER_SIZE]; + print_dec (end, end_buf, SIGNED); + + if (start == end) + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds read at byte %s but %qE" + " starts at byte 0", start_buf, + m_diag_arg); + return ev.formatted_print ("out-of-bounds read at byte %s but region" + " starts at byte 0", start_buf); + } + else + { + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds read from byte %s till" + " byte %s but %qE starts at byte 0", + start_buf, end_buf, m_diag_arg); + return ev.formatted_print ("out-of-bounds read from byte %s till" + " byte %s but region starts at byte 0", + start_buf, end_buf);; + } + } +}; + +/* May complain when the access on REG is out-of-bounds. */ + +void region_model::check_region_bounds (const region *reg, + enum access_direction dir, + region_model_context *ctxt) const +{ + gcc_assert (ctxt); + + region_offset reg_offset = reg->get_offset (); + const region *base_reg = reg_offset.get_base_region (); + + /* Bail out on symbolic offsets or symbolic regions. + (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 (reg_offset.symbolic_p () || base_reg->symbolic_p ()) + return; + byte_offset_t offset_unsigned + = reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT; + /* The constant offset from a pointer is represented internally as a sizetype + but should be interpreted as a signed value here. The statement below + converts the offset to a signed integer with the same precision the + sizetype has on the target system. + + For example, this is needed for out-of-bounds-3.c test1 to pass when + compiled with a 64-bit gcc build targeting 32-bit systems. */ + byte_offset_t offset + = offset_unsigned.to_shwi (TYPE_PRECISION (size_type_node)); + + /* Find out how many bytes were accessed. */ + const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr); + tree num_bytes_tree = num_bytes_sval->maybe_get_constant (); + if (!num_bytes_tree || TREE_CODE (num_bytes_tree) != INTEGER_CST) + /* If we do not know how many bytes were read/written, + assume that at least one byte was read/written. */ + num_bytes_tree = integer_one_node; + + byte_range out (0, 0); + /* NUM_BYTES_TREE should always be interpreted as unsigned. */ + byte_range read_bytes (offset, wi::to_offset (num_bytes_tree).to_uhwi ()); + /* If read_bytes has a subset < 0, we do have an underflow. */ + if (read_bytes.falls_short_of_p (0, &out)) + { + tree diag_arg = get_representative_tree (reg->get_base_region ()); + switch (dir) + { + default: + gcc_unreachable (); + break; + case DIR_READ: + ctxt->warn (new buffer_underread (reg, diag_arg, out)); + break; + case DIR_WRITE: + ctxt->warn (new buffer_underflow (reg, diag_arg, out)); + break; + } + } + + const svalue *capacity = get_capacity (base_reg); + tree cst_capacity_tree = capacity->maybe_get_constant (); + if (!cst_capacity_tree || TREE_CODE (cst_capacity_tree) != INTEGER_CST) + return; + + byte_range buffer (0, wi::to_offset (cst_capacity_tree)); + /* If READ_BYTES exceeds BUFFER, we do have an overflow. */ + if (read_bytes.exceeds_p (buffer, &out)) + { + tree byte_bound = wide_int_to_tree (size_type_node, + buffer.get_next_byte_offset ()); + tree diag_arg = get_representative_tree (reg->get_base_region ()); + + switch (dir) + { + default: + gcc_unreachable (); + break; + case DIR_READ: + ctxt->warn (new buffer_overread (reg, diag_arg, out, byte_bound)); + break; + case DIR_WRITE: + ctxt->warn (new buffer_overflow (reg, diag_arg, out, byte_bound)); + break; + } + } +} + /* Ensure that all arguments at the call described by CD are checked for poisoned values, by calling get_rvalue on each argument. */ @@ -2811,6 +3219,7 @@ region_model::check_region_access (const region *reg, return; check_region_for_taint (reg, dir, ctxt); + check_region_bounds (reg, dir, ctxt); switch (dir) { @@ -3806,6 +4215,19 @@ region_model::get_representative_tree (const svalue *sval) const return fixup_tree_for_diagnostic (expr); } +tree +region_model::get_representative_tree (const region *reg) const +{ + svalue_set visited; + tree expr = get_representative_path_var (reg, &visited).m_tree; + + /* Strip off any top-level cast. */ + if (expr && TREE_CODE (expr) == NOP_EXPR) + expr = TREE_OPERAND (expr, 0); + + return fixup_tree_for_diagnostic (expr); +} + /* Implementation of region_model::get_representative_path_var. Attempt to return a path_var that represents REG, or return diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 42f8abeb043..1f2c65d46ae 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -732,6 +732,7 @@ class region_model region_model_context *ctxt); tree get_representative_tree (const svalue *sval) const; + tree get_representative_tree (const region *reg) const; path_var get_representative_path_var (const svalue *sval, svalue_set *visited) const; @@ -867,6 +868,8 @@ class region_model region_model_context *ctxt) const; void check_region_size (const region *lhs_reg, const svalue *rhs_sval, region_model_context *ctxt) const; + void check_region_bounds (const region *reg, enum access_direction dir, + region_model_context *ctxt) const; void check_call_args (const call_details &cd) const; void check_external_function_for_access_attr (const gcall *call, diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc index b78bf4ec1b7..04c00266b46 100644 --- a/gcc/analyzer/region.cc +++ b/gcc/analyzer/region.cc @@ -629,6 +629,14 @@ region::symbolic_for_unknown_ptr_p () const return false; } +/* Return true if this is a symbolic region. */ + +bool +region::symbolic_p () const +{ + return get_kind () == RK_SYMBOLIC; +} + /* Return true if this is a region for a decl with name DECL_NAME. Intended for use when debugging (for assertions and conditional breakpoints). */ @@ -1430,6 +1438,30 @@ offset_region::get_relative_concrete_offset (bit_offset_t *out) const return false; } +/* Implementation of region::get_byte_size_sval vfunc for offset_region. */ + +const svalue * +offset_region::get_byte_size_sval (region_model_manager *mgr) const +{ + tree offset_cst = get_byte_offset ()->maybe_get_constant (); + byte_size_t byte_size; + /* If the offset points in the middle of the region, + return the remaining bytes. */ + if (get_byte_size (&byte_size) && offset_cst) + { + byte_size_t offset = wi::to_offset (offset_cst); + byte_range r(0, byte_size); + if (r.contains_p (offset)) + { + tree remaining_byte_size = wide_int_to_tree (size_type_node, + byte_size - offset); + return mgr->get_or_create_constant_svalue (remaining_byte_size); + } + } + + return region::get_byte_size_sval (mgr); +} + /* class sized_region : public region. */ /* Implementation of region::accept vfunc for sized_region. */ diff --git a/gcc/analyzer/region.h b/gcc/analyzer/region.h index 1067748a7cd..65efb719cbb 100644 --- a/gcc/analyzer/region.h +++ b/gcc/analyzer/region.h @@ -201,6 +201,8 @@ public: bool symbolic_for_unknown_ptr_p () const; + bool symbolic_p () const; + /* For most base regions it makes sense to track the bindings of the region within the store. As an optimization, some are not tracked (to avoid bloating the store object with redundant binding clusters). */ @@ -907,6 +909,8 @@ public: const svalue *get_byte_offset () const { return m_byte_offset; } bool get_relative_concrete_offset (bit_offset_t *out) const final override; + const svalue * get_byte_size_sval (region_model_manager *mgr) const; + private: const svalue *m_byte_offset; diff --git a/gcc/analyzer/store.cc b/gcc/analyzer/store.cc index 46475f6a7fa..848c5e1aaff 100644 --- a/gcc/analyzer/store.cc +++ b/gcc/analyzer/store.cc @@ -424,6 +424,73 @@ byte_range::contains_p (const byte_range &other, byte_range *out) const return false; } +/* Return true if THIS and OTHER intersect and write the number + of bytes both buffers overlap to *OUT_NUM_OVERLAP_BYTES. + + Otherwise return false. */ + +bool +byte_range::intersects_p (const byte_range &other, + byte_size_t *out_num_overlap_bytes) const +{ + if (get_start_byte_offset () < other.get_next_byte_offset () + && other.get_start_byte_offset () < get_next_byte_offset ()) + { + byte_offset_t overlap_start = MAX (get_start_byte_offset (), + other.get_start_byte_offset ()); + byte_offset_t overlap_next = MIN (get_next_byte_offset (), + other.get_next_byte_offset ()); + gcc_assert (overlap_next > overlap_start); + *out_num_overlap_bytes = overlap_next - overlap_start; + return true; + } + else + return false; +} + +/* Return true if THIS exceeds OTHER and write the overhanging + byte range to OUT_OVERHANGING_BYTE_RANGE. */ + +bool +byte_range::exceeds_p (const byte_range &other, + byte_range *out_overhanging_byte_range) const +{ + if (other.get_last_byte_offset () < get_last_byte_offset ()) + { + /* THIS definitely exceeds OTHER. */ + byte_offset_t start = MAX (get_start_byte_offset (), + other.get_next_byte_offset ()); + byte_offset_t size = get_next_byte_offset () - start; + gcc_assert (size > 0); + out_overhanging_byte_range->m_start_byte_offset = start; + out_overhanging_byte_range->m_size_in_bytes = size; + return true; + } + else + return false; +} + +/* Return true if THIS falls short of OFFSET and write the + byte range fallen short to OUT_FALL_SHORT_BYTES. */ + +bool +byte_range::falls_short_of_p (byte_offset_t offset, + byte_range *out_fall_short_bytes) const +{ + if (get_start_byte_offset () < offset) + { + /* THIS falls short of OFFSET. */ + byte_offset_t start = get_start_byte_offset (); + byte_offset_t size = MIN (offset, get_next_byte_offset ()) - start; + gcc_assert (size > 0); + out_fall_short_bytes->m_start_byte_offset = start; + out_fall_short_bytes->m_size_in_bytes = size; + return true; + } + else + return false; +} + /* qsort comparator for byte ranges. */ int diff --git a/gcc/analyzer/store.h b/gcc/analyzer/store.h index 9b54c7b9156..ac8b6853f4b 100644 --- a/gcc/analyzer/store.h +++ b/gcc/analyzer/store.h @@ -310,6 +310,15 @@ struct byte_range && m_size_in_bytes == other.m_size_in_bytes); } + bool intersects_p (const byte_range &other, + byte_size_t *out_num_overlap_bytes) const; + + bool exceeds_p (const byte_range &other, + byte_range *out_overhanging_byte_range) const; + + bool falls_short_of_p (byte_offset_t offset, + byte_range *out_fall_short_bytes) const; + byte_offset_t get_start_byte_offset () const { return m_start_byte_offset; diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index fa23fbeaaaa..094135dbf74 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -457,6 +457,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-mismatching-deallocation @gol -Wno-analyzer-null-argument @gol -Wno-analyzer-null-dereference @gol +-Wno-analyzer-out-of-bounds @gol -Wno-analyzer-possible-null-argument @gol -Wno-analyzer-possible-null-dereference @gol -Wno-analyzer-shift-count-negative @gol @@ -9759,6 +9760,7 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-mismatching-deallocation @gol -Wanalyzer-null-argument @gol -Wanalyzer-null-dereference @gol +-Wanalyzer-out-of-bounds @gol -Wanalyzer-possible-null-argument @gol -Wanalyzer-possible-null-dereference @gol -Wanalyzer-shift-count-negative @gol @@ -9951,6 +9953,19 @@ will warn about mismatches between @code{free}, scalar @code{delete} and vector @code{delete[]}, and those marked as allocator/deallocator pairs using attribute @code{malloc}. +@item -Wno-analyzer-out-of-bounds +@opindex Wanalyzer-out-of-bounds +@opindex Wno-analyzer-out-of-bounds +This warning requires @option{-fanalyzer} to enable it; use +@option{-Wno-analyzer-out-of-bounds} to disable it. + +This diagnostic warns for path through the code in which a buffer is +definitely read or written out-of-bounds. The diagnostic only applies +for cases where the analyzer is able to determine a constant offset and +for accesses past the end of a buffer, also a constant capacity. + +See @url{https://cwe.mitre.org/data/definitions/119.html, CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer}. + @item -Wno-analyzer-possible-null-argument @opindex Wanalyzer-possible-null-argument @opindex Wno-analyzer-possible-null-argument diff --git a/gcc/testsuite/g++.dg/analyzer/out-of-bounds-placement-new.C b/gcc/testsuite/g++.dg/analyzer/out-of-bounds-placement-new.C new file mode 100644 index 00000000000..d3076c3cf25 --- /dev/null +++ b/gcc/testsuite/g++.dg/analyzer/out-of-bounds-placement-new.C @@ -0,0 +1,19 @@ +/* Reduced from pr100244.C. */ +inline void *operator new (__SIZE_TYPE__, void *__p) { return __p; } + +struct int_container { + int i; + int *addr () { return &i; } +}; + +struct int_and_addr { + int i; + int *addr; + int_and_addr () { addr = &i; } /* { dg-warning "overflow" } */ +}; + +int test (int_container ic) +{ + int_and_addr *iaddr = new (ic.addr ()) int_and_addr; + return iaddr->i; +} diff --git a/gcc/testsuite/g++.dg/analyzer/pr100244.C b/gcc/testsuite/g++.dg/analyzer/pr100244.C index 1d5e13dd865..6e2ac4c65bd 100644 --- a/gcc/testsuite/g++.dg/analyzer/pr100244.C +++ b/gcc/testsuite/g++.dg/analyzer/pr100244.C @@ -1,4 +1,7 @@ -// { dg-additional-options "-O1 -Wno-free-nonheap-object" } +// { dg-additional-options "-O1 -Wno-free-nonheap-object -Wno-analyzer-out-of-bounds" } +/* Disabled out-of-bounds checker because the output relied + on optimizations. out-of-bounds-placement-new.C tests + the same pattern but without optimizations. */ inline void *operator new (__SIZE_TYPE__, void *__p) { return __p; } diff --git a/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c index 0c86f09f15b..9590e3137e0 100644 --- a/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c +++ b/gcc/testsuite/gcc.dg/analyzer/allocation-size-3.c @@ -1,3 +1,5 @@ +/* { dg-additional-options -Wno-analyzer-out-of-bounds } */ + #include #include #include diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-2.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-2.c index 88ec84cb640..51e4a694951 100644 --- a/gcc/testsuite/gcc.dg/analyzer/memcpy-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/memcpy-2.c @@ -1,4 +1,4 @@ -/* { dg-additional-options "-Wno-stringop-overflow" } */ +/* { dg-additional-options "-Wno-stringop-overflow -Wno-analyzer-out-of-bounds" } */ void main (int c, void *v) diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c new file mode 100644 index 00000000000..9f3cda6e02b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c @@ -0,0 +1,120 @@ +#include +#include +#include +#include + +/* Wanalyzer-out-of-bounds tests for buffer overflows. */ + +/* Avoid folding of memcpy. */ +typedef void * (*memcpy_t) (void *dst, const void *src, size_t n); + +static memcpy_t __attribute__((noinline)) +get_memcpy (void) +{ + return memcpy; +} + + +/* Taken from CWE-787. */ +void test1 (void) +{ + int id_sequence[3]; + + id_sequence[0] = 123; + id_sequence[1] = 234; + id_sequence[2] = 345; + id_sequence[3] = 456; /* { dg-line test1 } */ + + /* { dg-warning "overflow" "warning" { target *-*-* } test1 } */ + /* { dg-message "" "note" { target *-*-* } test1 } */ +} + +void test2 (void) +{ + int n = 4; + int arr[4]; + + for (int i = n - 1; i >= 0; i--) + arr[i] = i; +} + +void test3 (void) +{ + int n = 4; + int arr[4]; + + for (int i = n; i >= 0; i--) + arr[i] = i; /* { dg-line test3 } */ + + /* { dg-warning "overflow" "warning" { target *-*-* } test3 } */ + /* { dg-message "" "note" { target *-*-* } test3 } */ +} + +void test4 (void) +{ + int *arr = malloc (4 * sizeof (int)); + if (!arr) + return; + + int *last_el = arr + 3; + *last_el = 3; + + free (arr); +} + +void test5 (void) +{ + int *arr = malloc (4 * sizeof (int)); + if (!arr) + return; + + int *last_el = arr + 4; + *last_el = 4; /* { dg-line test5 } */ + + free (arr); + /* { dg-warning "overflow" "warning" { target *-*-* } test5 } */ + /* { dg-message "" "note" { target *-*-* } test5 } */ +} + +/* Taken from "A Provenance-aware Memory Object Model for C". */ +int y = 2, x = 1; /* { dg-message "capacity" } */ +void test6 (void) +{ + int *p = &x + 1; + int *q = &y; + printf ("Addresses: p=% p q=% p \n" , (void *) p, (void *) q); + if (memcmp (&p , &q , sizeof (p)) == 0) + { + *p = 11; /* { dg-line test6b } */ + printf ("x=%d y=%d *p=%d *q=%d\n" , x, y, *p, *q); /* { dg-line test6c } */ + } + + /* { dg-warning "overflow" "warning" { target *-*-* } test6b } */ + /* { dg-message "" "note" { target *-*-* } test6b } */ + /* { dg-warning "overread" "warning" { target *-*-* } test6c } */ + /* { dg-message "" "note" { target *-*-* } test6c } */ +} + +extern int is_valid (void); + +int returnChunkSize (void *ptr) +{ + /* If chunk info is valid, return the size of usable memory, + else, return -1 to indicate an error. */ + return is_valid () ? sizeof (*ptr) : -1; +} + +/* Taken from CWE-787. */ +void test7 (void) +{ + memcpy_t fn = get_memcpy (); + + int destBuf[4]; + int srcBuf[4]; + fn (destBuf, srcBuf, returnChunkSize (destBuf)); /* { dg-line test7 } */ + + // TODO: Should we handle widening_svalues as a follow-up? + /* { dg-warning "overread" "warning" { xfail *-*-* } test7 } */ + /* { dg-warning "overflow" "warning" { xfail *-*-* } test7 } */ + /* { dg-message "" "note" { xfail *-*-* } test7 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c new file mode 100644 index 00000000000..0df9364c5c1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c @@ -0,0 +1,83 @@ +#include +#include +#include +#include + +/* Wanalyzer-out-of-bounds tests for buffer overreads. */ + +/* Avoid folding of memcpy. */ +typedef void * (*memcpy_t) (void *dst, const void *src, size_t n); + +static memcpy_t __attribute__((noinline)) +get_memcpy (void) +{ + return memcpy; +} + + +void test1 (void) +{ + int id_sequence[3]; + memset (id_sequence, 0, 3 * sizeof(int)); + printf ("%i", id_sequence[3]); /* { dg-line test1 } */ + + /* { dg-warning "overread" "warning" { target *-*-* } test1 } */ + /* { dg-message "" "note" { target *-*-* } test1 } */ +} + +void test2 (void) +{ + int n = 4; + int arr[n]; + memset (arr, 0, n * sizeof (int)); + + int sum = 0; + for (int i = n - 1; i >= 0; i--) + sum += arr[i]; +} + +void test3 (void) +{ + int n = 4; + int arr[4]; + memset (arr, 0, n * sizeof (int)); + + int sum = 0; + for (int i = n; i > 0; i--) + sum += arr[i]; /* { dg-line test3 } */ + + /* { dg-warning "overread" "warning" { target *-*-* } test3 } */ + /* { dg-message "" "note" { target *-*-* } test3 } */ +} + +void test4 (void) +{ + int n = 4; + int *arr = malloc (n * sizeof (int)); + if (!arr) + return; + memset (arr, 0, n * sizeof(int)); + + int sum = 0; + for (int i = n - 1; i >= 0; i--) + sum += *(arr + i); + + free (arr); +} + +void test5 (void) +{ + int n = 4; + int *arr = malloc (n * sizeof (int)); + if (!arr) + return; + memset (arr, 0, n * sizeof(int)); + + int sum = 0; + for (int i = n; i > 0; i--) + sum += *(arr + i); /* { dg-line test5 } */ + + free (arr); + /* { dg-warning "overread" "warning" { target *-*-* } test5 } */ + /* { dg-message "" "note" { target *-*-* } test5 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c new file mode 100644 index 00000000000..7446b182e48 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c @@ -0,0 +1,91 @@ +#include +#include +#include + +/* Wanalyzer-out-of-bounds tests for buffer underreads and writes. */ + +/* Avoid folding of memcpy. */ +typedef void * (*memcpy_t) (void *dst, const void *src, size_t n); +static memcpy_t __attribute__((noinline)) +get_memcpy (void) +{ + return memcpy; +} + + +void test1 (void) +{ + int buf[4]; + int *e = buf - 1; + *e = 42; /* { dg-line test1 } */ + + /* { dg-warning "underflow" "warning" { target *-*-* } test1 } */ + /* { dg-message "" "note" { target *-*-* } test1 } */ +} + +void test2 (void) +{ + int buf[4]; + int *e = buf + 1; + *e = 123; + *(e - 1) = 321; +} + +void test3 (void) +{ + int buf[4]; + int *e = buf + 1; + *e = 123; + *(e - 2) = 321; /* { dg-line test3 } */ + + /* { dg-warning "underflow" "warning" { target *-*-* } test3 } */ + /* { dg-message "" "note" { target *-*-* } test3 } */ +} + +void test4 (void) +{ + memcpy_t fn = get_memcpy (); + int buf[4]; + memset (buf, 1, 4 * sizeof (int)); + int n = -4; + fn (&(buf[n]), buf, sizeof (int)); /* { dg-line test4 } */ + + /* { dg-warning "underflow" "warning" { target *-*-* } test4 } */ + /* { dg-message "" "note" { target *-*-* } test4 } */ +} + +void test5 (void) +{ + int buf[4]; + memset (buf, 1, 4 * sizeof (int)); + + int sum = 0; + for (int i = 4; i >= 0; i++) + sum += *(buf - i); /* { dg-line test5 } */ + + /* { dg-warning "underread" "warning" { target *-*-* } test5 } */ + /* { dg-message "" "note" { target *-*-* } test5 } */ +} + +void test6 (void) +{ + int buf[4]; + memset (buf, 1, 4 * sizeof (int)); + + int *view = buf + 1; + int sum = 0; + for (int i = 0; i < 4; i++) + sum += *(view++); +} + +void test8 (void) +{ + memcpy_t fn = get_memcpy (); + int buf[4]; + memset (buf, 1, 4 * sizeof (int)); + int n = -4; + fn (buf, &(buf[n]), sizeof (int)); /* { dg-line test8 } */ + + /* { dg-warning "underread" "warning" { target *-*-* } test8 } */ + /* { dg-message "" "note" { target *-*-* } test8 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c new file mode 100644 index 00000000000..172ec474c42 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c @@ -0,0 +1,51 @@ +/* Further reduced container_of pattern from the Linux Kernel. */ + +struct inner { + /* Don't care */ +}; + +struct outer { + int i; + struct inner inner_struct; +}; + +struct outer *container_of (struct inner *ptr_to_inner) +{ + struct outer *ptr_to_outer = ((struct outer *) (((void *) ptr_to_inner) - __builtin_offsetof(struct outer, inner_struct))); + return ptr_to_outer; +} + +int test (struct outer *outer_p, struct inner *inner_p) +{ + struct outer test; + test.i = 42; + struct inner test2; + int sum = 0; + struct outer *o; + + /* Symbolic inner struct. */ + o = container_of (inner_p); + sum += o->i; // ok + /* Not ok, but we can't be sure that outer + is actually the container of inner. */ + sum += (o - 1)->i; + /* Symbolic outer struct. */ + o = container_of (&(outer_p->inner_struct)); + sum += o->i; // ok + /* Not ok, but indistinguishable from the case above. */ + sum += (o - 1)->i; + /* Concrete outer struct. */ + o = container_of (&(test.inner_struct)); + sum += o->i; // ok + /* Not ok and we do have a concrete region. */ + sum += (o - 1)->i; /* { dg-line testA } */ + /* Concrete inner struct, has no container. */ + o = container_of (&test2); + sum += o->i; /* { dg-line testB } */ + + return sum; + /* { dg-warning "underread" "warning" { target *-*-* } testA } */ + /* { dg-message "" "note" { target *-*-* } testA } */ + /* { dg-warning "underread" "warning" { target *-*-* } testB } */ + /* { dg-message "" "note" { target *-*-* } testB } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-coreutils.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-coreutils.c new file mode 100644 index 00000000000..cd0f4b7f7b2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-coreutils.c @@ -0,0 +1,29 @@ +/* Reduced from coreutils/ls.c attach. */ + +void add_zero_terminator (char *buf) +{ + char *end = buf; + while (end++); + if (buf < end) + end[-1] = '\0'; +} + +/* Reduced from coreutils/cat.c. */ + +#define LINE_COUNTER_BUF_LEN 20 +static char line_buf[LINE_COUNTER_BUF_LEN] = + { + ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', + ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '0', + '\t', '\0' + }; + +/* Position of the first digit in 'line_buf'. */ +static char *line_num_start = line_buf + LINE_COUNTER_BUF_LEN - 3; + +static void +next_line_num (void) +{ + if (line_num_start > line_buf) + *--line_num_start = '1'; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-curl.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-curl.c new file mode 100644 index 00000000000..e34b572966e --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-curl.c @@ -0,0 +1,41 @@ +/* { dg-additional-options "-O2" } */ +#include + +/* Reduced from curl lib/smb.c. */ +typedef int CURLcode; + +struct smb_conn { + // [...] + char *user; +}; + +struct smb_setup { + // [...] + char bytes[48]; +} __attribute__((packed)); + +struct connectdata { + // [...] + struct smb_conn *smbc; +}; + +CURLcode smb_send_setup (struct connectdata *conn) +{ + struct smb_conn *smbc = conn->smbc; + struct smb_setup msg; + char *p = msg.bytes; + unsigned char lm[24]; + + /* Init to prevent uninit warning. */ + memset(&msg, 0, sizeof(msg)); + memset (&lm, 0, sizeof(lm)); + + memcpy(p, lm, sizeof(lm)); + p += sizeof(lm); + /* Had a false-positive overflow at p. Checker had a number of bytes copied + relative to the start but offset points in the middle the field. */ + strcpy(p, (smbc->user)); + p += strlen(smbc->user) + 1; + + return 1; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c b/gcc/testsuite/gcc.dg/analyzer/pr101962.c index d15820af27f..cf0041b2fcd 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c @@ -22,8 +22,10 @@ test_1 (void) a = maybe_inc_int_ptr (a); __analyzer_eval (a == NULL); /* { dg-warning "FALSE" } */ __analyzer_eval (a != NULL); /* { dg-warning "TRUE" } */ - return *a; /* { dg-warning "use of uninitialized value '\\*a'" } */ - /* TODO: a complaint about out-of-bounds would be a better warning. */ + return *a; /* { dg-line test_1 } */ + + /* { dg-warning "use of uninitialized value '\\*a'" "warning" { target *-*-* } test_1 } */ + /* { dg-warning "overread" "warning" { target *-*-* } test_1 } */ } static const char * __attribute__((noinline)) diff --git a/gcc/testsuite/gcc.dg/analyzer/pr96764.c b/gcc/testsuite/gcc.dg/analyzer/pr96764.c index 70b25d3ab81..6024ebaaa2a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr96764.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr96764.c @@ -1,3 +1,5 @@ +/* { dg-additional-options "-Wno-analyzer-out-of-bounds" } */ + void ar (int *hd) { diff --git a/gcc/testsuite/gcc.dg/analyzer/pr97029.c b/gcc/testsuite/gcc.dg/analyzer/pr97029.c index ff83ad4d56e..e3b45311779 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr97029.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr97029.c @@ -1,4 +1,6 @@ -struct vj {}; +struct vj { + char buf[1]; +}; void setjmp (struct vj pl) diff --git a/gcc/testsuite/gcc.dg/analyzer/test-setjmp.h b/gcc/testsuite/gcc.dg/analyzer/test-setjmp.h index db2422709e2..52c57d02b70 100644 --- a/gcc/testsuite/gcc.dg/analyzer/test-setjmp.h +++ b/gcc/testsuite/gcc.dg/analyzer/test-setjmp.h @@ -12,7 +12,9 @@ #pragma GCC system_header -struct __jmp_buf_tag {}; +struct __jmp_buf_tag { + char buf[1]; +}; typedef struct __jmp_buf_tag jmp_buf[1]; typedef struct __jmp_buf_tag sigjmp_buf[1]; diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c index 5098b4fc363..b05b8629a7f 100644 --- a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c +++ b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c @@ -182,7 +182,9 @@ static int huft_build(uInt *b, uInt n, uInt s, const uInt *d, const uInt *e, q[j] = r; /* { dg-warning "use of uninitialized value 'r.base'" } */ mask = (1 << w) - 1; - while ((i & mask) != x[h]) { + /* The analyzer thinks that h can be -1 here. + This is probably a false positive. */ + while ((i & mask) != x[h]) { /* { dg-bogus "underread" "" { xfail *-*-* } } */ h--; w -= l; mask = (1 << w) - 1;