From patchwork Tue Aug 9 21:19:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Lange X-Patchwork-Id: 56631 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 B4E7E385C33F for ; Tue, 9 Aug 2022 21:20:17 +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 12FF23858401 for ; Tue, 9 Aug 2022 21:20:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 12FF23858401 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: 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:In-Reply-To:References; bh=KMb2Kkj//FV43Cb+IP1CyZUlvj6sQrh0iJnvVj7nmhQ=; b=w8Vl3Faa56eGRBciF3V/dW67nA gWEoTg8bc6+bk5Hn/vAQgOlJ2nGrcOePa6Y52sXdzCmzRzsnqrzHfNNeIc2AyAWmXTTxK8AR+DH9c YXmAw+bxP+kUoZo0AgEtJyVY+LYCsYSBShD/aF7Fk3DSFuwPTkqXcUzsIhy+Tq+4CFlqYkSF5203K 0OtYBS+4vN8xbf85GDLr29NrX941tF8qWblW715ciFuVw56aP+NkL0PcM1qgbB3ojq+uCUXK1cELm 3HRytyIk860Fl8F6odP7/VVBxwlM5m+d/dNhMINhNSWpkGEArZdQO2e7e5gKHS62X366P+Xw4lK1u JSYIvhDQ==; Received: from sslproxy02.your-server.de ([78.47.166.47]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1oLWdr-000EjX-9D; Tue, 09 Aug 2022 23:19:59 +0200 Received: from [2a02:908:1861:d6a0::6b5] (helo=fedora..) by sslproxy02.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oLWdr-000CU7-2f; Tue, 09 Aug 2022 23:19:59 +0200 From: Tim Lange To: gcc-patches@gcc.gnu.org Subject: [PATCH 1/2] analyzer: consider that realloc could shrink the buffer [PR106539] Date: Tue, 9 Aug 2022 23:19:42 +0200 Message-Id: <20220809211943.82098-1-mail@tim-lange.me> X-Mailer: git-send-email 2.37.1 MIME-Version: 1.0 X-Authenticated-Sender: mail@tim-lange.me X-Virus-Scanned: Clear (ClamAV 0.103.6/26622/Tue Aug 9 09:53:52 2022) X-Spam-Status: No, score=-11.8 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. 2022-08-09 Tim Lange gcc/analyzer/ChangeLog: PR analyzer/106539 * region-model-impl-calls.cc (region_model::impl_call_realloc): Add get_copied_size function and pass the result as the size of the new sized_region. --- gcc/analyzer/region-model-impl-calls.cc | 37 ++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index 8c38e9206fa..50a19a52a21 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -737,9 +737,11 @@ region_model::impl_call_realloc (const call_details &cd) old_size_sval); const svalue *buffer_content_sval = model->get_store_value (sized_old_reg, cd.get_ctxt ()); + const svalue *copied_size_sval + = get_copied_size (old_size_sval, new_size_sval); const region *sized_new_reg = model->m_mgr->get_sized_region (new_reg, NULL, - old_size_sval); + copied_size_sval); model->set_value (sized_new_reg, buffer_content_sval, cd.get_ctxt ()); } @@ -774,6 +776,39 @@ region_model::impl_call_realloc (const call_details &cd) else return true; } + + private: + /* Return the size svalue for the new region allocated by realloc. */ + 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. */ From patchwork Tue Aug 9 21:19:43 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Lange X-Patchwork-Id: 56632 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 BF834385C405 for ; Tue, 9 Aug 2022 21:20:24 +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 C8CCF3858401 for ; Tue, 9 Aug 2022 21:20:03 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C8CCF3858401 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=5uOETY9qbAJLm7k93fjh+6Qqvzi8B1FfQPz65bMEaAY=; b=nsNnTMSA1UoqMmm9AcMzorrFCn a+4VlKptbhLvZard3qqHtVc1iIw3IzaOeQJaOGz2ATrHOlBxK1Yn9WeocvcDL3xgz1tzJfJYXnpkq t5B/0mqByX+2t9KDTIEeFCVitIvKi64GntpU+CM5vjVlYF+TbPB4XZxvniHNO9p7wHzICXonhGRZu e7cryRHMO6SKHVQn5zM/z/wTIieUZ2PJzGx0masxJdVYrwt5iYATvLFijhnTaSngq+Zom2JZeAtmB Nn8EVNDtqgi2gS60ShefyJnuIBTmM51weRk+oHQkCReyB+gMPXuvqZFGSk0E6Im9CwMbSTHyvSvqq Pz4Yt0/w==; Received: from sslproxy02.your-server.de ([78.47.166.47]) by www523.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1oLWdu-000Ejx-Qn; Tue, 09 Aug 2022 23:20:02 +0200 Received: from [2a02:908:1861:d6a0::6b5] (helo=fedora..) by sslproxy02.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1oLWdu-000CU7-JA; Tue, 09 Aug 2022 23:20:02 +0200 From: Tim Lange To: gcc-patches@gcc.gnu.org Subject: [PATCH 2/2] analyzer: out-of-bounds checker [PR106000] Date: Tue, 9 Aug 2022 23:19:43 +0200 Message-Id: <20220809211943.82098-2-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/26622/Tue Aug 9 09:53:52 2022) X-Spam-Status: No, score=-11.9 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. (Regrtests still running with the latest changes, will report back later.) 2022-08-09 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 * 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/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. * 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 | 410 ++++++++++++++++++ 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 | 12 + .../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 | 119 +++++ .../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 | 5 +- 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 +- 20 files changed, 970 insertions(+), 6 deletions(-) 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..8e73af60ceb 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -158,6 +158,10 @@ Wanalyzer-tainted-size Common Var(warn_analyzer_tainted_size) Init(1) Warning Warn about code paths in which an unsanitized value is used as a size. +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-use-after-free Common Var(warn_analyzer_use_after_free) Init(1) Warning Warn about code paths in which a freed value is used. diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index f7df2fca245..2f9382ed96c 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1268,6 +1268,402 @@ 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 range) + : m_reg (reg), m_diag_arg (diag_arg), m_range (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_range == other.m_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_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 m_reg == other.m_reg + && m_range == other.m_range + && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg) + && 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_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_range.get_start_byte_offset (); + byte_size_t end = m_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_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_range.get_start_byte_offset (); + byte_size_t end = m_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_range.get_start_byte_offset (); + byte_size_t end = m_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_range.get_start_byte_offset (); + byte_size_t end = m_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 = reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT; + + /* 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 +3207,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 +4203,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..fee0ba3bde3 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 mgr->get_or_create_unknown_svalue (size_type_node); +} + /* 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..5ab834af780 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -459,6 +459,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-null-dereference @gol -Wno-analyzer-possible-null-argument @gol -Wno-analyzer-possible-null-dereference @gol +-Wno-analyzer-out-of-bounds @gol -Wno-analyzer-shift-count-negative @gol -Wno-analyzer-shift-count-overflow @gol -Wno-analyzer-stale-setjmp-buffer @gol @@ -9991,6 +9992,17 @@ This warning requires @option{-fanalyzer}, which enables it; use This diagnostic warns for paths through the code in which a value known to be NULL is dereferenced. +@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 +accessed or written out-of-bounds. + +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-shift-count-negative @opindex Wanalyzer-shift-count-negative @opindex Wno-analyzer-shift-count-negative 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..715c8b7460f --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c @@ -0,0 +1,119 @@ +#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.*?overflow|overflow.*?overread" "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..90b4a65a067 --- /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 "overread" "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..1490e7c0d7f 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c @@ -22,8 +22,9 @@ 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 "overread.*?use of uninitialized value '\\*a'|use of uninitialized value '\\*a'.*?overread" "warning" { target *-*-* } test_1 } */ } static const char * __attribute__((noinline)) 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;