From patchwork Thu Dec 1 02:41:54 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61294 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 D491C3852C6A for ; Thu, 1 Dec 2022 02:44:04 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D491C3852C6A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862644; bh=stsUK509bgTXBUhgg2eJIeMtntdAHJWe08lq3jdAECU=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=wdB1wUIeauzsh89Id9V7twz0nL1BZZW+KlAVMRQcIQEzq4ff9J9K/wESZUXh3lAwu YUwpWdWytOgayr21kYJM3PeCOVnAw/Tl9oQnFReA90LJ6qL989Rai5nfQog+9PKDpU m1BILcvLNj0G5WQ/C+ZdjryQMgCLz9afSzoLlK9Y= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 27B443858D37 for ; Thu, 1 Dec 2022 02:42:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 27B443858D37 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-617-V0mus3ArN6acOr38zrLVCg-1; Wed, 30 Nov 2022 21:42:05 -0500 X-MC-Unique: V0mus3ArN6acOr38zrLVCg-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3BCFF3813F2F for ; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id ECECD2028CE4; Thu, 1 Dec 2022 02:42:04 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 1/7] analyzer: move bounds checking to a new bounds-checking.cc Date: Wed, 30 Nov 2022 21:41:54 -0500 Message-Id: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4425-gb82b361af888a1. gcc/ChangeLog: * Makefile.in (ANALYZER_OBJS): Add analyzer/bounds-checking.o. gcc/analyzer/ChangeLog: * bounds-checking.cc: New file, taken from region-model.cc. * region-model.cc (class out_of_bounds): Move to bounds-checking.cc. (class past_the_end): Likewise. (class buffer_overflow): Likewise. (class buffer_overread): Likewise. (class buffer_underflow): Likewise. (class buffer_underread): Likewise. (class symbolic_past_the_end): Likewise. (class symbolic_buffer_overflow): Likewise. (class symbolic_buffer_overread): Likewise. (region_model::check_symbolic_bounds): Likewise. (maybe_get_integer_cst_tree): Likewise. (region_model::check_region_bounds): Likewise. * region-model.h: Add comment. Signed-off-by: David Malcolm --- gcc/Makefile.in | 1 + gcc/analyzer/bounds-checking.cc | 695 ++++++++++++++++++++++++++++++++ gcc/analyzer/region-model.cc | 653 ------------------------------ gcc/analyzer/region-model.h | 2 + 4 files changed, 698 insertions(+), 653 deletions(-) create mode 100644 gcc/analyzer/bounds-checking.cc diff --git a/gcc/Makefile.in b/gcc/Makefile.in index fa5e5b444bb..615a07089ee 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -1255,6 +1255,7 @@ ANALYZER_OBJS = \ analyzer/analyzer-pass.o \ analyzer/analyzer-selftests.o \ analyzer/bar-chart.o \ + analyzer/bounds-checking.o \ analyzer/call-info.o \ analyzer/call-string.o \ analyzer/call-summary.o \ diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc new file mode 100644 index 00000000000..19aaa51e6a8 --- /dev/null +++ b/gcc/analyzer/bounds-checking.cc @@ -0,0 +1,695 @@ +/* Bounds-checking of reads and writes to memory regions. + Copyright (C) 2019-2022 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it +under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#include "config.h" +#define INCLUDE_MEMORY +#include "system.h" +#include "coretypes.h" +#include "make-unique.h" +#include "tree.h" +#include "function.h" +#include "basic-block.h" +#include "gimple.h" +#include "gimple-iterator.h" +#include "diagnostic-core.h" +#include "diagnostic-metadata.h" +#include "analyzer/analyzer.h" +#include "analyzer/analyzer-logging.h" +#include "analyzer/region-model.h" + +#if ENABLE_ANALYZER + +namespace ana { + +/* Abstract base class for all out-of-bounds warnings with concrete values. */ + +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 (), "read is %s bytes past the end" + " of %qE", num_bytes_past_buf, + m_diag_arg); + else + inform (rich_loc->get_loc (), "read 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);; + } + } +}; + +/* Abstract class to complain about out-of-bounds read/writes where + the values are symbolic. */ + +class symbolic_past_the_end + : public pending_diagnostic_subclass +{ +public: + symbolic_past_the_end (const region *reg, tree diag_arg, tree offset, + tree num_bytes, tree capacity) + : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset), + m_num_bytes (num_bytes), m_capacity (capacity) + {} + + const char *get_kind () const final override + { + return "symbolic_past_the_end"; + } + + bool operator== (const symbolic_past_the_end &other) const + { + return m_reg == other.m_reg + && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg) + && pending_diagnostic::same_tree_p (m_offset, other.m_offset) + && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes) + && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity); + } + + 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); + } + + label_text + describe_region_creation_event (const evdesc::region_creation &ev) final + override + { + if (m_capacity) + return ev.formatted_print ("capacity is %qE bytes", m_capacity); + + return label_text (); + } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + const char *byte_str; + if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node)) + byte_str = "byte"; + else + byte_str = "bytes"; + + if (m_offset) + { + if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST) + { + if (m_diag_arg) + return ev.formatted_print ("%s of %E %s at offset %qE" + " exceeds %qE", m_dir_str, + m_num_bytes, byte_str, + m_offset, m_diag_arg); + else + return ev.formatted_print ("%s of %E %s at offset %qE" + " exceeds the buffer", m_dir_str, + m_num_bytes, byte_str, m_offset); + } + else if (m_num_bytes) + { + if (m_diag_arg) + return ev.formatted_print ("%s of %qE %s at offset %qE" + " exceeds %qE", m_dir_str, + m_num_bytes, byte_str, + m_offset, m_diag_arg); + else + return ev.formatted_print ("%s of %qE %s at offset %qE" + " exceeds the buffer", m_dir_str, + m_num_bytes, byte_str, m_offset); + } + else + { + if (m_diag_arg) + return ev.formatted_print ("%s at offset %qE exceeds %qE", + m_dir_str, m_offset, m_diag_arg); + else + return ev.formatted_print ("%s at offset %qE exceeds the" + " buffer", m_dir_str, m_offset); + } + } + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds %s on %qE", + m_dir_str, m_diag_arg); + return ev.formatted_print ("out-of-bounds %s", m_dir_str); + } + +protected: + const region *m_reg; + tree m_diag_arg; + tree m_offset; + tree m_num_bytes; + tree m_capacity; + const char *m_dir_str; +}; + +/* Concrete subclass to complain about overflows with symbolic values. */ + +class symbolic_buffer_overflow : public symbolic_past_the_end +{ +public: + symbolic_buffer_overflow (const region *reg, tree diag_arg, tree offset, + tree num_bytes, tree capacity) + : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) + { + m_dir_str = "write"; + } + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + switch (m_reg->get_memory_space ()) + { + default: + m.add_cwe (787); + return warning_meta (rich_loc, m, get_controlling_option (), + "buffer overflow"); + case MEMSPACE_STACK: + m.add_cwe (121); + return warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer overflow"); + case MEMSPACE_HEAP: + m.add_cwe (122); + return warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer overflow"); + } + } +}; + +/* Concrete subclass to complain about overreads with symbolic values. */ + +class symbolic_buffer_overread : public symbolic_past_the_end +{ +public: + symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset, + tree num_bytes, tree capacity) + : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) + { + m_dir_str = "read"; + } + + bool emit (rich_location *rich_loc) final override + { + diagnostic_metadata m; + m.add_cwe (126); + return warning_meta (rich_loc, m, get_controlling_option (), + "buffer overread"); + } +}; + +/* Check whether an access is past the end of the BASE_REG. */ + +void +region_model::check_symbolic_bounds (const region *base_reg, + const svalue *sym_byte_offset, + const svalue *num_bytes_sval, + const svalue *capacity, + enum access_direction dir, + region_model_context *ctxt) const +{ + gcc_assert (ctxt); + + const svalue *next_byte + = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR, + sym_byte_offset, num_bytes_sval); + + if (eval_condition (next_byte, GT_EXPR, capacity).is_true ()) + { + tree diag_arg = get_representative_tree (base_reg); + tree offset_tree = get_representative_tree (sym_byte_offset); + tree num_bytes_tree = get_representative_tree (num_bytes_sval); + tree capacity_tree = get_representative_tree (capacity); + switch (dir) + { + default: + gcc_unreachable (); + break; + case DIR_READ: + ctxt->warn (make_unique (base_reg, + diag_arg, + offset_tree, + num_bytes_tree, + capacity_tree)); + break; + case DIR_WRITE: + ctxt->warn (make_unique (base_reg, + diag_arg, + offset_tree, + num_bytes_tree, + capacity_tree)); + break; + } + } +} + +static tree +maybe_get_integer_cst_tree (const svalue *sval) +{ + tree cst_tree = sval->maybe_get_constant (); + if (cst_tree && TREE_CODE (cst_tree) == INTEGER_CST) + return cst_tree; + + return NULL_TREE; +} + +/* 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); + + /* Get the offset. */ + region_offset reg_offset = reg->get_offset (m_mgr); + const region *base_reg = reg_offset.get_base_region (); + + /* Bail out on 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 (base_reg->symbolic_p ()) + return; + + /* Find out how many bytes were accessed. */ + const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr); + tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval); + /* Bail out if 0 bytes are accessed. */ + if (num_bytes_tree && zerop (num_bytes_tree)) + return; + + /* Get the capacity of the buffer. */ + const svalue *capacity = get_capacity (base_reg); + tree cst_capacity_tree = maybe_get_integer_cst_tree (capacity); + + /* 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 from bits to bytes and then 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; + if (!reg_offset.symbolic_p ()) + offset = wi::sext (reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT, + TYPE_PRECISION (size_type_node)); + + /* If either the offset or the number of bytes accessed are symbolic, + we have to reason about symbolic values. */ + if (reg_offset.symbolic_p () || !num_bytes_tree) + { + const svalue* byte_offset_sval; + if (!reg_offset.symbolic_p ()) + { + tree offset_tree = wide_int_to_tree (integer_type_node, offset); + byte_offset_sval + = m_mgr->get_or_create_constant_svalue (offset_tree); + } + else + byte_offset_sval = reg_offset.get_symbolic_byte_offset (); + check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval, + capacity, dir, ctxt); + return; + } + + /* Otherwise continue to check with concrete values. */ + byte_range out (0, 0); + /* NUM_BYTES_TREE should always be interpreted as unsigned. */ + byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree); + byte_range read_bytes (offset, num_bytes_unsigned); + /* 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 (base_reg); + switch (dir) + { + default: + gcc_unreachable (); + break; + case DIR_READ: + ctxt->warn (make_unique (reg, diag_arg, out)); + break; + case DIR_WRITE: + ctxt->warn (make_unique (reg, diag_arg, out)); + break; + } + } + + /* For accesses past the end, we do need a concrete capacity. No need to + do a symbolic check here because the inequality check does not reason + whether constants are greater than symbolic values. */ + if (!cst_capacity_tree) + 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 (base_reg); + + switch (dir) + { + default: + gcc_unreachable (); + break; + case DIR_READ: + ctxt->warn (make_unique (reg, diag_arg, + out, byte_bound)); + break; + case DIR_WRITE: + ctxt->warn (make_unique (reg, diag_arg, + out, byte_bound)); + break; + } + } +} + +} // namespace ana + +#endif /* #if ENABLE_ANALYZER */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 7f2c0b6bd1a..91b868f7b16 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -1212,659 +1212,6 @@ region_model::on_stmt_pre (const gimple *stmt, } } -/* Abstract base class for all out-of-bounds warnings with concrete values. */ - -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 (), "read is %s bytes past the end" - " of %qE", num_bytes_past_buf, - m_diag_arg); - else - inform (rich_loc->get_loc (), "read 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);; - } - } -}; - -/* Abstract class to complain about out-of-bounds read/writes where - the values are symbolic. */ - -class symbolic_past_the_end - : public pending_diagnostic_subclass -{ -public: - symbolic_past_the_end (const region *reg, tree diag_arg, tree offset, - tree num_bytes, tree capacity) - : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset), - m_num_bytes (num_bytes), m_capacity (capacity) - {} - - const char *get_kind () const final override - { - return "symbolic_past_the_end"; - } - - bool operator== (const symbolic_past_the_end &other) const - { - return m_reg == other.m_reg - && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg) - && pending_diagnostic::same_tree_p (m_offset, other.m_offset) - && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes) - && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity); - } - - 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); - } - - label_text - describe_region_creation_event (const evdesc::region_creation &ev) final - override - { - if (m_capacity) - return ev.formatted_print ("capacity is %qE bytes", m_capacity); - - return label_text (); - } - - label_text - describe_final_event (const evdesc::final_event &ev) final override - { - const char *byte_str; - if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node)) - byte_str = "byte"; - else - byte_str = "bytes"; - - if (m_offset) - { - if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST) - { - if (m_diag_arg) - return ev.formatted_print ("%s of %E %s at offset %qE" - " exceeds %qE", m_dir_str, - m_num_bytes, byte_str, - m_offset, m_diag_arg); - else - return ev.formatted_print ("%s of %E %s at offset %qE" - " exceeds the buffer", m_dir_str, - m_num_bytes, byte_str, m_offset); - } - else if (m_num_bytes) - { - if (m_diag_arg) - return ev.formatted_print ("%s of %qE %s at offset %qE" - " exceeds %qE", m_dir_str, - m_num_bytes, byte_str, - m_offset, m_diag_arg); - else - return ev.formatted_print ("%s of %qE %s at offset %qE" - " exceeds the buffer", m_dir_str, - m_num_bytes, byte_str, m_offset); - } - else - { - if (m_diag_arg) - return ev.formatted_print ("%s at offset %qE exceeds %qE", - m_dir_str, m_offset, m_diag_arg); - else - return ev.formatted_print ("%s at offset %qE exceeds the" - " buffer", m_dir_str, m_offset); - } - } - if (m_diag_arg) - return ev.formatted_print ("out-of-bounds %s on %qE", - m_dir_str, m_diag_arg); - return ev.formatted_print ("out-of-bounds %s", m_dir_str); - } - -protected: - const region *m_reg; - tree m_diag_arg; - tree m_offset; - tree m_num_bytes; - tree m_capacity; - const char *m_dir_str; -}; - -/* Concrete subclass to complain about overflows with symbolic values. */ - -class symbolic_buffer_overflow : public symbolic_past_the_end -{ -public: - symbolic_buffer_overflow (const region *reg, tree diag_arg, tree offset, - tree num_bytes, tree capacity) - : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) - { - m_dir_str = "write"; - } - - bool emit (rich_location *rich_loc) final override - { - diagnostic_metadata m; - switch (m_reg->get_memory_space ()) - { - default: - m.add_cwe (787); - return warning_meta (rich_loc, m, get_controlling_option (), - "buffer overflow"); - case MEMSPACE_STACK: - m.add_cwe (121); - return warning_meta (rich_loc, m, get_controlling_option (), - "stack-based buffer overflow"); - case MEMSPACE_HEAP: - m.add_cwe (122); - return warning_meta (rich_loc, m, get_controlling_option (), - "heap-based buffer overflow"); - } - } -}; - -/* Concrete subclass to complain about overreads with symbolic values. */ - -class symbolic_buffer_overread : public symbolic_past_the_end -{ -public: - symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset, - tree num_bytes, tree capacity) - : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) - { - m_dir_str = "read"; - } - - bool emit (rich_location *rich_loc) final override - { - diagnostic_metadata m; - m.add_cwe (126); - return warning_meta (rich_loc, m, get_controlling_option (), - "buffer overread"); - } -}; - -/* Check whether an access is past the end of the BASE_REG. */ - -void -region_model::check_symbolic_bounds (const region *base_reg, - const svalue *sym_byte_offset, - const svalue *num_bytes_sval, - const svalue *capacity, - enum access_direction dir, - region_model_context *ctxt) const -{ - gcc_assert (ctxt); - - const svalue *next_byte - = m_mgr->get_or_create_binop (num_bytes_sval->get_type (), PLUS_EXPR, - sym_byte_offset, num_bytes_sval); - - if (eval_condition (next_byte, GT_EXPR, capacity).is_true ()) - { - tree diag_arg = get_representative_tree (base_reg); - tree offset_tree = get_representative_tree (sym_byte_offset); - tree num_bytes_tree = get_representative_tree (num_bytes_sval); - tree capacity_tree = get_representative_tree (capacity); - switch (dir) - { - default: - gcc_unreachable (); - break; - case DIR_READ: - ctxt->warn (make_unique (base_reg, - diag_arg, - offset_tree, - num_bytes_tree, - capacity_tree)); - break; - case DIR_WRITE: - ctxt->warn (make_unique (base_reg, - diag_arg, - offset_tree, - num_bytes_tree, - capacity_tree)); - break; - } - } -} - -static tree -maybe_get_integer_cst_tree (const svalue *sval) -{ - tree cst_tree = sval->maybe_get_constant (); - if (cst_tree && TREE_CODE (cst_tree) == INTEGER_CST) - return cst_tree; - - return NULL_TREE; -} - -/* 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); - - /* Get the offset. */ - region_offset reg_offset = reg->get_offset (m_mgr); - const region *base_reg = reg_offset.get_base_region (); - - /* Bail out on 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 (base_reg->symbolic_p ()) - return; - - /* Find out how many bytes were accessed. */ - const svalue *num_bytes_sval = reg->get_byte_size_sval (m_mgr); - tree num_bytes_tree = maybe_get_integer_cst_tree (num_bytes_sval); - /* Bail out if 0 bytes are accessed. */ - if (num_bytes_tree && zerop (num_bytes_tree)) - return; - - /* Get the capacity of the buffer. */ - const svalue *capacity = get_capacity (base_reg); - tree cst_capacity_tree = maybe_get_integer_cst_tree (capacity); - - /* 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 from bits to bytes and then 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; - if (!reg_offset.symbolic_p ()) - offset = wi::sext (reg_offset.get_bit_offset () >> LOG2_BITS_PER_UNIT, - TYPE_PRECISION (size_type_node)); - - /* If either the offset or the number of bytes accessed are symbolic, - we have to reason about symbolic values. */ - if (reg_offset.symbolic_p () || !num_bytes_tree) - { - const svalue* byte_offset_sval; - if (!reg_offset.symbolic_p ()) - { - tree offset_tree = wide_int_to_tree (integer_type_node, offset); - byte_offset_sval - = m_mgr->get_or_create_constant_svalue (offset_tree); - } - else - byte_offset_sval = reg_offset.get_symbolic_byte_offset (); - check_symbolic_bounds (base_reg, byte_offset_sval, num_bytes_sval, - capacity, dir, ctxt); - return; - } - - /* Otherwise continue to check with concrete values. */ - byte_range out (0, 0); - /* NUM_BYTES_TREE should always be interpreted as unsigned. */ - byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree); - byte_range read_bytes (offset, num_bytes_unsigned); - /* 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 (base_reg); - switch (dir) - { - default: - gcc_unreachable (); - break; - case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, out)); - break; - case DIR_WRITE: - ctxt->warn (make_unique (reg, diag_arg, out)); - break; - } - } - - /* For accesses past the end, we do need a concrete capacity. No need to - do a symbolic check here because the inequality check does not reason - whether constants are greater than symbolic values. */ - if (!cst_capacity_tree) - 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 (base_reg); - - switch (dir) - { - default: - gcc_unreachable (); - break; - case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, - out, byte_bound)); - break; - case DIR_WRITE: - ctxt->warn (make_unique (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. */ diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 86c42a24ff2..86bfb4ff8a0 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -606,6 +606,8 @@ private: region_model_context *ctxt) const; void check_region_size (const region *lhs_reg, const svalue *rhs_sval, region_model_context *ctxt) const; + + /* Implemented in bounds-checking.cc */ void check_symbolic_bounds (const region *base_reg, const svalue *sym_byte_offset, const svalue *num_bytes_sval, From patchwork Thu Dec 1 02:41:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61289 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 514E33852234 for ; Thu, 1 Dec 2022 02:42:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 514E33852234 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862561; bh=5F1dt/Q++L4b5AhsMoj1kxgYjtK8Isx01iLZG2ouLkA=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=OchiMgu3kqK2W/B3w9ZL1VxoAmxpsiXHxqcbIcedVxA4mviP5V4KiwTrcwlhneFbI PTOBQ9D3u0SXBwWV7tDZ+W5MGshFQZnjBK7HGOYl9Yy0cxuzHLeqRD6SnR8nVG8xlp IZpTTbYZMOx+0i0Int07kiLdq2NcaCndMKUa92ZE= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 7FA8F385840C for ; Thu, 1 Dec 2022 02:42:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7FA8F385840C Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-100-twnvgAOrPY2c8cybsj25bA-1; Wed, 30 Nov 2022 21:42:06 -0500 X-MC-Unique: twnvgAOrPY2c8cybsj25bA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7232F29AB3FB for ; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4C532200D8C3; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 2/7] analyzer: fix wording of 'number of bad bytes' note [PR106626] Date: Wed, 30 Nov 2022 21:41:55 -0500 Message-Id: <20221201024200.3722982-2-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Consider -fanalyzer on: #include int32_t arr[10]; void int_arr_write_element_after_end_far(int32_t x) { arr[100] = x; } Trunk x86_64: https://godbolt.org/z/7GqEcYGq6 Currently we emit: : In function 'int_arr_write_element_after_end_far': :7:12: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds] 7 | arr[100] = x; | ~~~~~~~~~^~~ event 1 | | 3 | int32_t arr[10]; | | ^~~ | | | | | (1) capacity is 40 bytes | +--> 'int_arr_write_element_after_end_far': events 2-3 | | 5 | void int_arr_write_element_after_end_far(int32_t x) | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) entry to 'int_arr_write_element_after_end_far' | 6 | { | 7 | arr[100] = x; | | ~~~~~~~~~~~~ | | | | | (3) out-of-bounds write from byte 400 till byte 403 but 'arr' ends at byte 40 | :7:12: note: write is 4 bytes past the end of 'arr' 7 | arr[100] = x; | ~~~~~~~~~^~~ The wording of the final note: "write is 4 bytes past the end of 'arr'" reads to me as if the "4 bytes past" is describing where the access occurs, which seems wrong, as the write is far beyond the end of the array. Looking at the implementation, it's actually describing the number of bytes within the access that are beyond the bounds of the buffer. This patch updates the wording so that the final note reads "write of 4 bytes to beyond the end of 'arr'" which more clearly expresses that it's the size of the access being described. The patch also uses inform_n to avoid emitting "1 bytes". Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4426-gd69a95c12cc91e. gcc/analyzer/ChangeLog: PR analyzer/106626 * bounds-checking.cc (buffer_overflow::emit): Use inform_n. Update wording to clarify that we're talking about the size of the bad access, rather than its position. (buffer_overread::emit): Likewise. gcc/testsuite/ChangeLog: PR analyzer/106626 * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Update for changes to expected wording. * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Likewise. * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise. * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise. Signed-off-by: David Malcolm --- gcc/analyzer/bounds-checking.cc | 66 ++++++++++++------- .../analyzer/out-of-bounds-read-char-arr.c | 11 +--- .../analyzer/out-of-bounds-read-int-arr.c | 8 +-- .../analyzer/out-of-bounds-write-char-arr.c | 11 +--- .../analyzer/out-of-bounds-write-int-arr.c | 8 +-- 5 files changed, 56 insertions(+), 48 deletions(-) diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index 19aaa51e6a8..ad7f431ea2f 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -143,17 +143,28 @@ public: 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); + if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes)) + { + unsigned HOST_WIDE_INT num_bad_bytes + = m_out_of_bounds_range.m_size_in_bytes.to_uhwi (); + if (m_diag_arg) + inform_n (rich_loc->get_loc (), + num_bad_bytes, + "write of %wu byte to beyond the end of %qE", + "write of %wu bytes to beyond the end of %qE", + num_bad_bytes, + m_diag_arg); + else + inform_n (rich_loc->get_loc (), + num_bad_bytes, + "write of %wu byte to beyond the end of the region", + "write of %wu bytes to beyond the end of the region", + num_bad_bytes); + } + else if (m_diag_arg) + inform (rich_loc->get_loc (), + "write to beyond the end of %qE", + m_diag_arg); } return warned; @@ -212,17 +223,28 @@ public: 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 (), "read is %s bytes past the end" - " of %qE", num_bytes_past_buf, - m_diag_arg); - else - inform (rich_loc->get_loc (), "read is %s bytes past the end" - "of the region", - num_bytes_past_buf); + if (wi::fits_uhwi_p (m_out_of_bounds_range.m_size_in_bytes)) + { + unsigned HOST_WIDE_INT num_bad_bytes + = m_out_of_bounds_range.m_size_in_bytes.to_uhwi (); + if (m_diag_arg) + inform_n (rich_loc->get_loc (), + num_bad_bytes, + "read of %wu byte from after the end of %qE", + "read of %wu bytes from after the end of %qE", + num_bad_bytes, + m_diag_arg); + else + inform_n (rich_loc->get_loc (), + num_bad_bytes, + "read of %wu byte from after the end of the region", + "read of %wu bytes from after the end of the region", + num_bad_bytes); + } + else if (m_diag_arg) + inform (rich_loc->get_loc (), + "read from after the end of %qE", + m_diag_arg); } return warned; diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c index 61cbfc75c11..2b43000f833 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c @@ -32,24 +32,19 @@ char int_arr_read_element_after_end_off_by_one(void) { return arr[10]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): "1 bytes" + /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } char int_arr_read_element_after_end_near(void) { return arr[11]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): is the note correct? - // FIXME(PR 106626): "1 bytes" + /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } char int_arr_read_element_after_end_far(void) { return arr[100]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): the note seems incorrect (size of access is 1 byte, but magnitude beyond boundary is 90) - // FIXME(PR 106626): "1 bytes" + /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c index 0bb30d24e9f..0dc95563620 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c @@ -34,21 +34,19 @@ int32_t int_arr_read_element_after_end_off_by_one(void) { return arr[10]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } int32_t int_arr_read_element_after_end_near(void) { return arr[11]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): is the note correct? + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } int32_t int_arr_read_element_after_end_far(void) { return arr[100]; /* { dg-warning "buffer overread" "warning" } */ /* { dg-message "out-of-bounds read from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "read is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): the note seems incorrect (size of access is 4 bytes, but magnitude beyond boundary is 390-393) + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c index 47fbc5207ee..3564476c322 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c @@ -32,24 +32,19 @@ void int_arr_write_element_after_end_off_by_one(char x) { arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): "1 bytes" + /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } void int_arr_write_element_after_end_near(char x) { arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): is the note correct? - // FIXME(PR 106626): "1 bytes" + /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } void int_arr_write_element_after_end_far(char x) { arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 1 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): the note seems incorrect (size of access is 1 byte, but magnitude beyond boundary is 90) - // FIXME(PR 106626): "1 bytes" + /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c index bf9760ee978..24a9a6bfa18 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c @@ -34,21 +34,19 @@ void int_arr_write_element_after_end_off_by_one(int32_t x) { arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } void int_arr_write_element_after_end_near(int32_t x) { arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): is the note correct? + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } void int_arr_write_element_after_end_far(int32_t x) { arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ - /* { dg-message "write is 4 bytes past the end of 'arr'" "note" { target *-*-* } .-2 } */ - // FIXME(PR 106626): the note seems incorrect (size of access is 4 bytes, but magnitude beyond boundary is 390-393) + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ } From patchwork Thu Dec 1 02:41:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61290 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 98C043852C6B for ; Thu, 1 Dec 2022 02:42:50 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 98C043852C6B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862570; bh=0j6IjnC99gYT6+jWppzOTtQ2xMzJqivwHVEou76BKS8=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=XIo97sWEi86SkksgJ3Q2S2SbEJzBn+An6BAPjrSw+xZJ8t2liFEoixnpEirJCFb27 5x49Dkxsm0Gsaxut5HHZ/silSZ7Q+31vjHxo92JgcIr6xl1bJDWzRpWlvgouEv1cHA VLRG+uJ3ah65ZUT9rc2qwYtfgJLy0r2p/YDn1ntA= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id A23A13858417 for ; Thu, 1 Dec 2022 02:42:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A23A13858417 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-617-kIUibVsxPFuHGvRwW7Jr-Q-1; Wed, 30 Nov 2022 21:42:05 -0500 X-MC-Unique: kIUibVsxPFuHGvRwW7Jr-Q-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id A5F9C185A78B for ; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 818B82024CBE; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 3/7] analyzer: add note about valid subscripts [PR106626] Date: Wed, 30 Nov 2022 21:41:56 -0500 Message-Id: <20221201024200.3722982-3-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Consider -fanalyzer on: #include int32_t arr[10]; void int_arr_write_element_after_end_off_by_one(int32_t x) { arr[10] = x; } Trunk x86_64: https://godbolt.org/z/17zn3qYY4 Currently we emit: : In function 'int_arr_write_element_after_end_off_by_one': :7:11: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds] 7 | arr[10] = x; | ~~~~~~~~^~~ event 1 | | 3 | int32_t arr[10]; | | ^~~ | | | | | (1) capacity is 40 bytes | +--> 'int_arr_write_element_after_end_off_by_one': events 2-3 | | 5 | void int_arr_write_element_after_end_off_by_one(int32_t x) | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) entry to 'int_arr_write_element_after_end_off_by_one' | 6 | { | 7 | arr[10] = x; | | ~~~~~~~~~~~ | | | | | (3) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40 | :7:11: note: write of 4 bytes to beyond the end of 'arr' 7 | arr[10] = x; | ~~~~~~~~^~~ This is worded in terms of bytes, due to the way -Wanalyzer-out-of-bounds is implemented, but this isn't what the user wrote. This patch tries to get closer to the user's code by adding a note about array bounds when we're referring to an array. In the above example it adds this trailing note: note: valid subscripts for 'arr' are '[0]' to '[9]' Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4427-g7c655699ed51b0. gcc/analyzer/ChangeLog: PR analyzer/106626 * bounds-checking.cc (out_of_bounds::maybe_describe_array_bounds): New. (buffer_overflow::emit): Call maybe_describe_array_bounds. (buffer_overread::emit): Likewise. (buffer_underflow::emit): Likewise. (buffer_underread::emit): Likewise. gcc/testsuite/ChangeLog: PR analyzer/106626 * gcc.dg/analyzer/call-summaries-2.c: Add dg-message for expected note about valid indexes. * gcc.dg/analyzer/out-of-bounds-1.c: Likewise, fixing up existing dg-message directives. * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Likewise. * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Likewise. Signed-off-by: David Malcolm --- gcc/analyzer/bounds-checking.cc | 46 +++++++++++++++++-- .../gcc.dg/analyzer/call-summaries-2.c | 1 + .../gcc.dg/analyzer/out-of-bounds-1.c | 16 ++++--- .../analyzer/out-of-bounds-write-char-arr.c | 6 +++ .../analyzer/out-of-bounds-write-int-arr.c | 6 +++ 5 files changed, 64 insertions(+), 11 deletions(-) diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index ad7f431ea2f..b02bc79a926 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -71,6 +71,34 @@ public: } protected: + /* Potentially add a note about valid ways to index this array, such + as (given "int arr[10];"): + note: valid subscripts for 'arr' are '[0]' to '[9]' + We print the '[' and ']' characters so as to express the valid + subscripts using C syntax, rather than just as byte ranges, + which hopefully is more clear to the user. */ + void + maybe_describe_array_bounds (location_t loc) const + { + if (!m_diag_arg) + return; + tree t = TREE_TYPE (m_diag_arg); + if (!t) + return; + if (TREE_CODE (t) != ARRAY_TYPE) + return; + tree domain = TYPE_DOMAIN (t); + if (!domain) + return; + tree max_idx = TYPE_MAX_VALUE (domain); + if (!max_idx) + return; + tree min_idx = TYPE_MIN_VALUE (domain); + inform (loc, + "valid subscripts for %qE are %<[%E]%> to %<[%E]%>", + m_diag_arg, min_idx, max_idx); + } + const region *m_reg; tree m_diag_arg; byte_range m_out_of_bounds_range; @@ -165,6 +193,8 @@ public: inform (rich_loc->get_loc (), "write to beyond the end of %qE", m_diag_arg); + + maybe_describe_array_bounds (rich_loc->get_loc ()); } return warned; @@ -245,6 +275,8 @@ public: inform (rich_loc->get_loc (), "read from after the end of %qE", m_diag_arg); + + maybe_describe_array_bounds (rich_loc->get_loc ()); } return warned; @@ -297,8 +329,11 @@ public: { diagnostic_metadata m; m.add_cwe (124); - return warning_meta (rich_loc, m, get_controlling_option (), - "buffer underflow"); + bool warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer underflow"); + if (warned) + maybe_describe_array_bounds (rich_loc->get_loc ()); + return warned; } label_text describe_final_event (const evdesc::final_event &ev) @@ -346,8 +381,11 @@ public: { diagnostic_metadata m; m.add_cwe (127); - return warning_meta (rich_loc, m, get_controlling_option (), - "buffer underread"); + bool warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer underread"); + if (warned) + maybe_describe_array_bounds (rich_loc->get_loc ()); + return warned; } label_text describe_final_event (const evdesc::final_event &ev) diff --git a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c index 953cbd32f5a..a7a17dbd358 100644 --- a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c @@ -330,6 +330,7 @@ int test_returns_element_ptr (int j) __analyzer_eval (*returns_element_ptr (1) == 8); /* { dg-warning "TRUE" } */ __analyzer_eval (*returns_element_ptr (2) == 9); /* { dg-warning "TRUE" } */ return *returns_element_ptr (3); /* { dg-warning "buffer overread" } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } .-1 } */ } int returns_offset (int arr[3], int i) diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c index 9f3cda6e02b..dc4de9b28a6 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c @@ -25,8 +25,9 @@ void test1 (void) id_sequence[2] = 345; id_sequence[3] = 456; /* { dg-line test1 } */ - /* { dg-warning "overflow" "warning" { target *-*-* } test1 } */ - /* { dg-message "" "note" { target *-*-* } test1 } */ + /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test1 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'id_sequence'" "num bad bytes note" { target *-*-* } test1 } */ + /* { dg-message "valid subscripts for 'id_sequence' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } test1 } */ } void test2 (void) @@ -46,8 +47,9 @@ void test3 (void) for (int i = n; i >= 0; i--) arr[i] = i; /* { dg-line test3 } */ - /* { dg-warning "overflow" "warning" { target *-*-* } test3 } */ - /* { dg-message "" "note" { target *-*-* } test3 } */ + /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test3 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } test3 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test3 } */ } void test4 (void) @@ -72,7 +74,7 @@ void test5 (void) *last_el = 4; /* { dg-line test5 } */ free (arr); - /* { dg-warning "overflow" "warning" { target *-*-* } test5 } */ + /* { dg-warning "heap-based buffer overflow" "warning" { target *-*-* } test5 } */ /* { dg-message "" "note" { target *-*-* } test5 } */ } @@ -89,9 +91,9 @@ void test6 (void) printf ("x=%d y=%d *p=%d *q=%d\n" , x, y, *p, *q); /* { dg-line test6c } */ } - /* { dg-warning "overflow" "warning" { target *-*-* } test6b } */ + /* { dg-warning "buffer overflow" "warning" { target *-*-* } test6b } */ /* { dg-message "" "note" { target *-*-* } test6b } */ - /* { dg-warning "overread" "warning" { target *-*-* } test6c } */ + /* { dg-warning "buffer overread" "warning" { target *-*-* } test6c } */ /* { dg-message "" "note" { target *-*-* } test6c } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c index 3564476c322..739ebb11590 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c @@ -4,18 +4,21 @@ void int_arr_write_element_before_start_far(char x) { arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_near(char x) { arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_off_by_one(char x) { arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_at_start(char x) @@ -33,6 +36,7 @@ void int_arr_write_element_after_end_off_by_one(char x) arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } void int_arr_write_element_after_end_near(char x) @@ -40,6 +44,7 @@ void int_arr_write_element_after_end_near(char x) arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } void int_arr_write_element_after_end_far(char x) @@ -47,4 +52,5 @@ void int_arr_write_element_after_end_far(char x) arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 1 byte to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c index 24a9a6bfa18..b2b37b92e01 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c @@ -6,18 +6,21 @@ void int_arr_write_element_before_start_far(int32_t x) { arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_near(int32_t x) { arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_off_by_one(int32_t x) { arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */ /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_at_start(int32_t x) @@ -35,6 +38,7 @@ void int_arr_write_element_after_end_off_by_one(int32_t x) arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } void int_arr_write_element_after_end_near(int32_t x) @@ -42,6 +46,7 @@ void int_arr_write_element_after_end_near(int32_t x) arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } void int_arr_write_element_after_end_far(int32_t x) @@ -49,4 +54,5 @@ void int_arr_write_element_after_end_far(int32_t x) arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } From patchwork Thu Dec 1 02:41:57 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61292 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 92D6938518B8 for ; Thu, 1 Dec 2022 02:42:56 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 92D6938518B8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862576; bh=bdT6X24X6JPqR57KyA1y7O3kjDZf66sTtMy3J2Uxeu0=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=kSAuNMUO0Sp6wLw9FHQv8rNpzPcLECUsl9hmVbmYN/tDlytjZwcNIG/WdCG6FYNxm +7jMtEBMkRBhYzPFHSbNREkRZyEst7/8XBnvhakI33g3hhDfLnLu8dYmPFpjrzXnMF jzgbHAfYwPoMML1GGuXXpVB9p6I7rmj1+yRl8s5M= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 0C68A385842E for ; Thu, 1 Dec 2022 02:42:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0C68A385842E Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-194-iegmrZF7MgedTp0R4eHZew-1; Wed, 30 Nov 2022 21:42:06 -0500 X-MC-Unique: iegmrZF7MgedTp0R4eHZew-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DE313811E81 for ; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id B3FDA2024CBE; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 4/7] analyzer: more bounds-checking wording tweaks [PR106626] Date: Wed, 30 Nov 2022 21:41:57 -0500 Message-Id: <20221201024200.3722982-4-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" This patch tweaks the wording of -Wanalyzer-out-of-bounds: * use the spellings/terminology of CWE: * replace "underread" with "under-read", as per: https://cwe.mitre.org/data/definitions/127.html * replace "overread" with "over-read" as per: https://cwe.mitre.org/data/definitions/126.html * replace "underflow" with "underwrite" as per: https://cwe.mitre.org/data/definitions/124.html * wherever known, specify the memory region of the bad access, so that it says e.g. "heap-based buffer over-read" or "stack-based buffer over-read" Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4428-gdf460cf51b2586. gcc/analyzer/ChangeLog: PR analyzer/106626 * bounds-checking.cc (out_of_bounds::get_memory_space): New. (buffer_overflow::emit): Use it. (class buffer_overread): Rename to... (class buffer_over_read): ...this. (buffer_over_read::emit): Specify which memory space the read is from, where known. Change "overread" to "over-read". (class buffer_underflow): Rename to... (class buffer_underwrite): ...this. (buffer_underwrite::emit): Specify which memory space the write is to, where known. Change "underflow" to "underwrite". (class buffer_underread): Rename to... (class buffer_under_read): Rename to... (buffer_under_read::emit): Specify which memory space the read is from, where known. Change "underread" to "under-read". (symbolic_past_the_end::get_memory_space): New. (symbolic_buffer_overflow::emit): Use it. (class symbolic_buffer_overread): Rename to... (class symbolic_buffer_over_read): ...this. (symbolic_buffer_over_read::emit): Specify which memory space the read is from, where known. Change "overread" to "over-read". (region_model::check_symbolic_bounds): Update for class renaming. (region_model::check_region_bounds): Likewise. gcc/testsuite/ChangeLog: PR analyzer/106626 * gcc.dg/analyzer/call-summaries-2.c: Update expected results. * gcc.dg/analyzer/out-of-bounds-1.c: Likewise. * gcc.dg/analyzer/out-of-bounds-2.c: Likewise. * gcc.dg/analyzer/out-of-bounds-3.c: Likewise. * gcc.dg/analyzer/out-of-bounds-4.c: Likewise. * gcc.dg/analyzer/out-of-bounds-5.c: Likewise. * gcc.dg/analyzer/out-of-bounds-container_of.c: Likewise. * gcc.dg/analyzer/out-of-bounds-read-char-arr.c: Likewise. Rename functions from "int_arr_" to "char_arr_". * gcc.dg/analyzer/out-of-bounds-read-int-arr.c: Update expected results. * gcc.dg/analyzer/out-of-bounds-read-struct-arr.c: New test. * gcc.dg/analyzer/out-of-bounds-write-char-arr.c: Update expected results. Rename functions from "int_arr_" to "char_arr_". * gcc.dg/analyzer/out-of-bounds-write-int-arr.c: Update expected results. * gcc.dg/analyzer/out-of-bounds-write-struct-arr.c: New test. * gcc.dg/analyzer/pr101962.c: Update expected results. * gcc.dg/analyzer/realloc-5.c: Update expected results. * gcc.dg/analyzer/zlib-3.c: Update expected results. Signed-off-by: David Malcolm --- gcc/analyzer/bounds-checking.cc | 133 +++++++++++++----- .../gcc.dg/analyzer/call-summaries-2.c | 2 +- .../gcc.dg/analyzer/out-of-bounds-1.c | 4 +- .../gcc.dg/analyzer/out-of-bounds-2.c | 15 +- .../gcc.dg/analyzer/out-of-bounds-3.c | 27 ++-- .../gcc.dg/analyzer/out-of-bounds-4.c | 15 +- .../gcc.dg/analyzer/out-of-bounds-5.c | 20 +-- .../analyzer/out-of-bounds-container_of.c | 4 +- .../analyzer/out-of-bounds-read-char-arr.c | 34 +++-- .../analyzer/out-of-bounds-read-int-arr.c | 18 ++- .../analyzer/out-of-bounds-read-struct-arr.c | 65 +++++++++ .../analyzer/out-of-bounds-write-char-arr.c | 22 +-- .../analyzer/out-of-bounds-write-int-arr.c | 6 +- .../analyzer/out-of-bounds-write-struct-arr.c | 65 +++++++++ gcc/testsuite/gcc.dg/analyzer/pr101962.c | 2 +- gcc/testsuite/gcc.dg/analyzer/realloc-5.c | 2 +- gcc/testsuite/gcc.dg/analyzer/zlib-3.c | 2 +- 17 files changed, 327 insertions(+), 109 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index b02bc79a926..bc7d2dd17ae 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -71,6 +71,11 @@ public: } protected: + enum memory_space get_memory_space () const + { + return m_reg->get_memory_space (); + } + /* Potentially add a note about valid ways to index this array, such as (given "int arr[10];"): note: valid subscripts for 'arr' are '[0]' to '[9]' @@ -150,7 +155,7 @@ public: { diagnostic_metadata m; bool warned; - switch (m_reg->get_memory_space ()) + switch (get_memory_space ()) { default: m.add_cwe (787); @@ -234,22 +239,36 @@ public: } }; -/* Concrete subclass to complain about buffer overreads. */ +/* Concrete subclass to complain about buffer over-reads. */ -class buffer_overread : public past_the_end +class buffer_over_read : public past_the_end { public: - buffer_overread (const region *reg, tree diag_arg, - byte_range range, tree byte_bound) + buffer_over_read (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; m.add_cwe (126); - bool warned = warning_meta (rich_loc, m, get_controlling_option (), - "buffer overread"); + switch (get_memory_space ()) + { + default: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer over-read"); + break; + case MEMSPACE_STACK: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer over-read"); + break; + case MEMSPACE_HEAP: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer over-read"); + break; + } if (warned) { @@ -316,21 +335,35 @@ public: } }; -/* Concrete subclass to complain about buffer underflows. */ +/* Concrete subclass to complain about buffer underwrites. */ -class buffer_underflow : public out_of_bounds +class buffer_underwrite : public out_of_bounds { public: - buffer_underflow (const region *reg, tree diag_arg, byte_range range) + buffer_underwrite (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; + bool warned; m.add_cwe (124); - bool warned = warning_meta (rich_loc, m, get_controlling_option (), - "buffer underflow"); + switch (get_memory_space ()) + { + default: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer underwrite"); + break; + case MEMSPACE_STACK: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer underwrite"); + break; + case MEMSPACE_HEAP: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer underwrite"); + break; + } if (warned) maybe_describe_array_bounds (rich_loc->get_loc ()); return warned; @@ -368,21 +401,35 @@ public: } }; -/* Concrete subclass to complain about buffer underreads. */ +/* Concrete subclass to complain about buffer under-reads. */ -class buffer_underread : public out_of_bounds +class buffer_under_read : public out_of_bounds { public: - buffer_underread (const region *reg, tree diag_arg, byte_range range) + buffer_under_read (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; + bool warned; m.add_cwe (127); - bool warned = warning_meta (rich_loc, m, get_controlling_option (), - "buffer underread"); + switch (get_memory_space ()) + { + default: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "buffer under-read"); + break; + case MEMSPACE_STACK: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer under-read"); + break; + case MEMSPACE_HEAP: + warned = warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer under-read"); + break; + } if (warned) maybe_describe_array_bounds (rich_loc->get_loc ()); return warned; @@ -519,6 +566,11 @@ public: } protected: + enum memory_space get_memory_space () const + { + return m_reg->get_memory_space (); + } + const region *m_reg; tree m_diag_arg; tree m_offset; @@ -542,7 +594,7 @@ public: bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; - switch (m_reg->get_memory_space ()) + switch (get_memory_space ()) { default: m.add_cwe (787); @@ -560,13 +612,13 @@ public: } }; -/* Concrete subclass to complain about overreads with symbolic values. */ +/* Concrete subclass to complain about over-reads with symbolic values. */ -class symbolic_buffer_overread : public symbolic_past_the_end +class symbolic_buffer_over_read : public symbolic_past_the_end { public: - symbolic_buffer_overread (const region *reg, tree diag_arg, tree offset, - tree num_bytes, tree capacity) + symbolic_buffer_over_read (const region *reg, tree diag_arg, tree offset, + tree num_bytes, tree capacity) : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) { m_dir_str = "read"; @@ -576,8 +628,21 @@ public: { diagnostic_metadata m; m.add_cwe (126); - return warning_meta (rich_loc, m, get_controlling_option (), - "buffer overread"); + switch (get_memory_space ()) + { + default: + m.add_cwe (787); + return warning_meta (rich_loc, m, get_controlling_option (), + "buffer over-read"); + case MEMSPACE_STACK: + m.add_cwe (121); + return warning_meta (rich_loc, m, get_controlling_option (), + "stack-based buffer over-read"); + case MEMSPACE_HEAP: + m.add_cwe (122); + return warning_meta (rich_loc, m, get_controlling_option (), + "heap-based buffer over-read"); + } } }; @@ -609,11 +674,11 @@ region_model::check_symbolic_bounds (const region *base_reg, gcc_unreachable (); break; case DIR_READ: - ctxt->warn (make_unique (base_reg, - diag_arg, - offset_tree, - num_bytes_tree, - capacity_tree)); + ctxt->warn (make_unique (base_reg, + diag_arg, + offset_tree, + num_bytes_tree, + capacity_tree)); break; case DIR_WRITE: ctxt->warn (make_unique (base_reg, @@ -701,7 +766,7 @@ region_model::check_region_bounds (const region *reg, /* NUM_BYTES_TREE should always be interpreted as unsigned. */ byte_offset_t num_bytes_unsigned = wi::to_offset (num_bytes_tree); byte_range read_bytes (offset, num_bytes_unsigned); - /* If read_bytes has a subset < 0, we do have an underflow. */ + /* If read_bytes has a subset < 0, we do have an underwrite. */ if (read_bytes.falls_short_of_p (0, &out)) { tree diag_arg = get_representative_tree (base_reg); @@ -711,10 +776,10 @@ region_model::check_region_bounds (const region *reg, gcc_unreachable (); break; case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, out)); + ctxt->warn (make_unique (reg, diag_arg, out)); break; case DIR_WRITE: - ctxt->warn (make_unique (reg, diag_arg, out)); + ctxt->warn (make_unique (reg, diag_arg, out)); break; } } @@ -739,8 +804,8 @@ region_model::check_region_bounds (const region *reg, gcc_unreachable (); break; case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, - out, byte_bound)); + ctxt->warn (make_unique (reg, diag_arg, + out, byte_bound)); break; case DIR_WRITE: ctxt->warn (make_unique (reg, diag_arg, diff --git a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c index a7a17dbd358..22ca475b2ed 100644 --- a/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/call-summaries-2.c @@ -329,7 +329,7 @@ int test_returns_element_ptr (int j) __analyzer_eval (*returns_element_ptr (0) == 7); /* { dg-warning "TRUE" } */ __analyzer_eval (*returns_element_ptr (1) == 8); /* { dg-warning "TRUE" } */ __analyzer_eval (*returns_element_ptr (2) == 9); /* { dg-warning "TRUE" } */ - return *returns_element_ptr (3); /* { dg-warning "buffer overread" } */ + return *returns_element_ptr (3); /* { dg-warning "buffer over-read" } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } .-1 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c index dc4de9b28a6..977476ed2fb 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c @@ -93,7 +93,7 @@ void test6 (void) /* { dg-warning "buffer overflow" "warning" { target *-*-* } test6b } */ /* { dg-message "" "note" { target *-*-* } test6b } */ - /* { dg-warning "buffer overread" "warning" { target *-*-* } test6c } */ + /* { dg-warning "buffer over-read" "warning" { target *-*-* } test6c } */ /* { dg-message "" "note" { target *-*-* } test6c } */ } @@ -116,7 +116,7 @@ void test7 (void) 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 "over-read" "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 index 0df9364c5c1..1330090f348 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-2.c @@ -3,7 +3,7 @@ #include #include -/* Wanalyzer-out-of-bounds tests for buffer overreads. */ +/* Wanalyzer-out-of-bounds tests for buffer over-reads. */ /* Avoid folding of memcpy. */ typedef void * (*memcpy_t) (void *dst, const void *src, size_t n); @@ -21,8 +21,9 @@ void test1 (void) 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 } */ + /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test1 } */ + /* { dg-message "read of 4 bytes from after the end of 'id_sequence'" "num bad bytes note" { target *-*-* } test1 } */ + /* { dg-message "valid subscripts for 'id_sequence' are '\\\[0\\\]' to '\\\[2\\\]'" "valid subscript note" { target *-*-* } test1 } */ } void test2 (void) @@ -46,7 +47,7 @@ void test3 (void) for (int i = n; i > 0; i--) sum += arr[i]; /* { dg-line test3 } */ - /* { dg-warning "overread" "warning" { target *-*-* } test3 } */ + /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test3 } */ /* { dg-message "" "note" { target *-*-* } test3 } */ } @@ -78,6 +79,8 @@ void test5 (void) sum += *(arr + i); /* { dg-line test5 } */ free (arr); - /* { dg-warning "overread" "warning" { target *-*-* } test5 } */ - /* { dg-message "" "note" { target *-*-* } test5 } */ + /* { dg-warning "heap-based buffer over-read" "bounds warning" { target *-*-* } test5 } */ + /* { dg-message "read of 4 bytes from after the end of the region" "num bad bytes note" { target *-*-* } test5 } */ + + /* { dg-warning "use of uninitialized value" "uninit warning" { target *-*-* } test5 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c index 7446b182e48..5fd9cc3c6b1 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-3.c @@ -2,7 +2,7 @@ #include #include -/* Wanalyzer-out-of-bounds tests for buffer underreads and writes. */ +/* Wanalyzer-out-of-bounds tests for buffer under-reads and underwrites. */ /* Avoid folding of memcpy. */ typedef void * (*memcpy_t) (void *dst, const void *src, size_t n); @@ -19,8 +19,9 @@ void test1 (void) int *e = buf - 1; *e = 42; /* { dg-line test1 } */ - /* { dg-warning "underflow" "warning" { target *-*-* } test1 } */ - /* { dg-message "" "note" { target *-*-* } test1 } */ + /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test1 } */ + /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'buf' starts at byte 0" "final event" { target *-*-* } test1 } */ + /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test1 } */ } void test2 (void) @@ -38,8 +39,9 @@ void test3 (void) *e = 123; *(e - 2) = 321; /* { dg-line test3 } */ - /* { dg-warning "underflow" "warning" { target *-*-* } test3 } */ - /* { dg-message "" "note" { target *-*-* } test3 } */ + /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test3 } */ + /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'buf' starts at byte 0" "final event" { target *-*-* } test3 } */ + /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test3 } */ } void test4 (void) @@ -50,8 +52,9 @@ void test4 (void) int n = -4; fn (&(buf[n]), buf, sizeof (int)); /* { dg-line test4 } */ - /* { dg-warning "underflow" "warning" { target *-*-* } test4 } */ - /* { dg-message "" "note" { target *-*-* } test4 } */ + /* { dg-warning "stack-based buffer underwrite" "warning" { target *-*-* } test4 } */ + /* { dg-message "out-of-bounds write from byte -16 till byte -13 but 'buf' starts at byte 0" "final event" { target *-*-* } test4 } */ + /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test4 } */ } void test5 (void) @@ -63,8 +66,9 @@ void test5 (void) for (int i = 4; i >= 0; i++) sum += *(buf - i); /* { dg-line test5 } */ - /* { dg-warning "underread" "warning" { target *-*-* } test5 } */ - /* { dg-message "" "note" { target *-*-* } test5 } */ + /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } test5 } */ + /* { dg-message "out-of-bounds read from byte -16 till byte -13 but 'buf' starts at byte 0" "final event" { target *-*-* } test5 } */ + /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test5 } */ } void test6 (void) @@ -86,6 +90,7 @@ void test8 (void) int n = -4; fn (buf, &(buf[n]), sizeof (int)); /* { dg-line test8 } */ - /* { dg-warning "underread" "warning" { target *-*-* } test8 } */ - /* { dg-message "" "note" { target *-*-* } test8 } */ + /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } test8 } */ + /* { dg-message "out-of-bounds read from byte -16 till byte -13 but 'buf' starts at byte 0" "note" { target *-*-* } test8 } */ + /* { dg-message "valid subscripts for 'buf' are '\\\[0\\\]' to '\\\[3\\\]'" "valid subscript note" { target *-*-* } test8 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c index 46f600de658..9cd8bda76c3 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-4.c @@ -11,8 +11,9 @@ void test1 (void) char dst[5]; strcpy (dst, "Hello"); /* { dg-line test1 } */ - /* { dg-warning "overflow" "warning" { target *-*-* } test1 } */ - /* { dg-message "dst" "note" { target *-*-* } test1 } */ + /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test1 } */ + /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test1 } */ + /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test1 } */ } void test2 (void) @@ -27,8 +28,9 @@ void test3 (void) char dst[5]; strcpy (dst, src); /* { dg-line test3 } */ - /* { dg-warning "overflow" "warning" { target *-*-* } test3 } */ - /* { dg-message "dst" "note" { target *-*-* } test3 } */ + /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test3 } */ + /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test3 } */ + /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test3 } */ } void test4 (void) @@ -51,8 +53,9 @@ void test5 (void) char dst[5]; strcpy (dst, str); /* { dg-line test5 } */ - /* { dg-warning "overflow" "warning" { target *-*-* } test5 } */ - /* { dg-message "dst" "note" { target *-*-* } test5 } */ + /* { dg-warning "stack-based buffer overflow" "warning" { target *-*-* } test5 } */ + /* { dg-message "write of 1 byte to beyond the end of 'dst'" "num bad bytes note" { target *-*-* } test5 } */ + /* { dg-message "valid subscripts for 'dst' are '\\\[0\\\]' to '\\\[4\\\]'" "valid subscript note" { target *-*-* } test5 } */ } void test6 (void) diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c index 7dc0bc5bf18..52fea79152e 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c @@ -13,7 +13,7 @@ void test1 (size_t size) char *buf = __builtin_malloc (size); if (!buf) return; - buf[size] = '\0'; /* { dg-warning "overflow" } */ + buf[size] = '\0'; /* { dg-warning "heap-based buffer overflow" } */ free (buf); } @@ -22,7 +22,7 @@ void test2 (size_t size) char *buf = __builtin_malloc (size); if (!buf) return; - buf[size + 1] = '\0'; /* { dg-warning "overflow" } */ + buf[size + 1] = '\0'; /* { dg-warning "heap-based buffer overflow" } */ free (buf); } @@ -31,33 +31,33 @@ void test3 (size_t size, size_t op) char *buf = __builtin_malloc (size); if (!buf) return; - buf[size + op] = '\0'; /* { dg-warning "overflow" } */ + buf[size + op] = '\0'; /* { dg-warning "heap-based buffer overflow" } */ free (buf); } void test4 (size_t size, unsigned short s) { char *buf = __builtin_alloca (size); - buf[size + s] = '\0'; /* { dg-warning "overflow" } */ + buf[size + s] = '\0'; /* { dg-warning "stack-based buffer overflow" } */ } void test5 (size_t size) { int32_t *buf = __builtin_alloca (4 * size); - buf[size] = 42; /* { dg-warning "overflow" } */ + buf[size] = 42; /* { dg-warning "stack-based buffer overflow" } */ } void test6 (size_t size) { int32_t *buf = __builtin_alloca (4 * size); memset (buf, 0, 4 * size); - int32_t last = *(buf + 4 * size); /* { dg-warning "overread" } */ + int32_t last = *(buf + 4 * size); /* { dg-warning "stack-based buffer over-read" } */ } void test7 (size_t size) { int32_t *buf = __builtin_alloca (4 * size + 3); /* { dg-warning "allocated buffer size is not a multiple of the pointee's size" } */ - buf[size] = 42; /* { dg-warning "overflow" } */ + buf[size] = 42; /* { dg-warning "stack-based buffer overflow" } */ } /* Test where the offset itself is not out-of-bounds @@ -68,7 +68,7 @@ void test8 (size_t size, size_t offset) char src[size]; char dst[size]; memcpy (dst, src, size + offset); /* { dg-line test8 } */ - /* { dg-warning "overread" "warning" { target *-*-* } test8 } */ + /* { dg-warning "over-read" "warning" { target *-*-* } test8 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test8 } */ } @@ -77,7 +77,7 @@ void test9 (size_t size, size_t offset) int32_t src[size]; int32_t dst[size]; memcpy (dst, src, 4 * size + 1); /* { dg-line test9 } */ - /* { dg-warning "overread" "warning" { target *-*-* } test9 } */ + /* { dg-warning "over-read" "warning" { target *-*-* } test9 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test9 } */ } @@ -151,6 +151,6 @@ char *test99 (const char *x, const char *y) __builtin_memcpy (result, x, len_x); __builtin_memcpy (result + len_x, y, len_y); /* BUG (symptom): off-by-one out-of-bounds write to heap. */ - result[len_x + len_y] = '\0'; /* { dg-warning "overflow" } */ + result[len_x + len_y] = '\0'; /* { dg-warning "heap-based buffer overflow" } */ return result; } 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 index 172ec474c42..ef460f4e772 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-container_of.c @@ -44,8 +44,8 @@ int test (struct outer *outer_p, struct inner *inner_p) sum += o->i; /* { dg-line testB } */ return sum; - /* { dg-warning "underread" "warning" { target *-*-* } testA } */ + /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } testA } */ /* { dg-message "" "note" { target *-*-* } testA } */ - /* { dg-warning "underread" "warning" { target *-*-* } testB } */ + /* { dg-warning "stack-based buffer under-read" "warning" { target *-*-* } testB } */ /* { dg-message "" "note" { target *-*-* } testB } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c index 2b43000f833..f6d0dda9fb9 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-char-arr.c @@ -1,50 +1,56 @@ char arr[10]; /* { dg-message "capacity is 10 bytes" } */ -char int_arr_read_element_before_start_far(void) +char char_arr_read_element_before_start_far(void) { - return arr[-100]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-100]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -char int_arr_read_element_before_start_near(void) +char char_arr_read_element_before_start_near(void) { - return arr[-2]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-2]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -char int_arr_read_element_before_start_off_by_one(void) +char char_arr_read_element_before_start_off_by_one(void) { - return arr[-1]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-1]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -char int_arr_read_element_at_start(void) +char char_arr_read_element_at_start(void) { return arr[0]; } -char int_arr_read_element_at_end(void) +char char_arr_read_element_at_end(void) { return arr[9]; } -char int_arr_read_element_after_end_off_by_one(void) +char char_arr_read_element_after_end_off_by_one(void) { - return arr[10]; /* { dg-warning "buffer overread" "warning" } */ + return arr[10]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } -char int_arr_read_element_after_end_near(void) +char char_arr_read_element_after_end_near(void) { - return arr[11]; /* { dg-warning "buffer overread" "warning" } */ + return arr[11]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } -char int_arr_read_element_after_end_far(void) +char char_arr_read_element_after_end_far(void) { - return arr[100]; /* { dg-warning "buffer overread" "warning" } */ + return arr[100]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 1 byte from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c index 0dc95563620..f1b6e119777 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-int-arr.c @@ -4,20 +4,23 @@ int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */ int32_t int_arr_read_element_before_start_far(void) { - return arr[-100]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-100]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } int32_t int_arr_read_element_before_start_near(void) { - return arr[-2]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-2]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } int32_t int_arr_read_element_before_start_off_by_one(void) { - return arr[-1]; /* { dg-warning "buffer underread" "warning" } */ + return arr[-1]; /* { dg-warning "buffer under-read" "warning" } */ /* { dg-message "out-of-bounds read from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } int32_t int_arr_read_element_at_start(void) @@ -32,21 +35,24 @@ int32_t int_arr_read_element_at_end(void) int32_t int_arr_read_element_after_end_off_by_one(void) { - return arr[10]; /* { dg-warning "buffer overread" "warning" } */ + return arr[10]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read from byte 40 till byte 43 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } int32_t int_arr_read_element_after_end_near(void) { - return arr[11]; /* { dg-warning "buffer overread" "warning" } */ + return arr[11]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read from byte 44 till byte 47 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } int32_t int_arr_read_element_after_end_far(void) { - return arr[100]; /* { dg-warning "buffer overread" "warning" } */ + return arr[100]; /* { dg-warning "buffer over-read" "warning" } */ /* { dg-message "out-of-bounds read from byte 400 till byte 403 but 'arr' ends at byte 40" "final event" { target *-*-* } .-1 } */ /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c new file mode 100644 index 00000000000..0f50bb92290 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-read-struct-arr.c @@ -0,0 +1,65 @@ +#include + +struct st +{ + char buf[16]; + int32_t x; + int32_t y; +}; + +struct st arr[10]; + +int32_t struct_arr_read_x_element_before_start_far(void) +{ + return arr[-100].x; /* { dg-warning "buffer under-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte -2384 till byte -2381 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +int32_t struct_arr_read_x_element_before_start_near(void) +{ + return arr[-2].x; /* { dg-warning "buffer under-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte -32 till byte -29 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +int32_t struct_arr_read_x_element_before_start_off_by_one(void) +{ + return arr[-1].x; /* { dg-warning "buffer under-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +int32_t struct_arr_read_x_element_at_start(void) +{ + return arr[0].x; +} + +int32_t struct_arr_read_x_element_at_end(void) +{ + return arr[9].x; +} + +int32_t struct_arr_read_x_element_after_end_off_by_one(void) +{ + return arr[10].x; /* { dg-warning "buffer over-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte 256 till byte 259 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} + +int32_t struct_arr_read_x_element_after_end_near(void) +{ + return arr[11].x; /* { dg-warning "buffer over-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte 280 till byte 283 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} + +int32_t struct_arr_read_x_element_after_end_far(void) +{ + return arr[100].x; /* { dg-warning "buffer over-read" "warning" } */ + /* { dg-message "out-of-bounds read from byte 2416 till byte 2419 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "read of 4 bytes from after the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c index 739ebb11590..2f3bbfa2dc2 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-char-arr.c @@ -1,37 +1,37 @@ char arr[10]; /* { dg-message "capacity is 10 bytes" } */ -void int_arr_write_element_before_start_far(char x) +void char_arr_write_element_before_start_far(char x) { - arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-100] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write at byte -100 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -void int_arr_write_element_before_start_near(char x) +void char_arr_write_element_before_start_near(char x) { - arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-2] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write at byte -2 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -void int_arr_write_element_before_start_off_by_one(char x) +void char_arr_write_element_before_start_off_by_one(char x) { - arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-1] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write at byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } -void int_arr_write_element_at_start(char x) +void char_arr_write_element_at_start(char x) { arr[0] = x; } -void int_arr_write_element_at_end(char x) +void char_arr_write_element_at_end(char x) { arr[9] = x; } -void int_arr_write_element_after_end_off_by_one(char x) +void char_arr_write_element_after_end_off_by_one(char x) { arr[10] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 10 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ @@ -39,7 +39,7 @@ void int_arr_write_element_after_end_off_by_one(char x) /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } -void int_arr_write_element_after_end_near(char x) +void char_arr_write_element_after_end_near(char x) { arr[11] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 11 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ @@ -47,7 +47,7 @@ void int_arr_write_element_after_end_near(char x) /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ } -void int_arr_write_element_after_end_far(char x) +void char_arr_write_element_after_end_far(char x) { arr[100] = x; /* { dg-warning "buffer overflow" "warning" } */ /* { dg-message "out-of-bounds write at byte 100 but 'arr' ends at byte 10" "final event" { target *-*-* } .-1 } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c index b2b37b92e01..0adb7899641 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-int-arr.c @@ -4,21 +4,21 @@ int32_t arr[10]; /* { dg-message "capacity is 40 bytes" } */ void int_arr_write_element_before_start_far(int32_t x) { - arr[-100] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-100] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write from byte -400 till byte -397 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_near(int32_t x) { - arr[-2] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-2] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } void int_arr_write_element_before_start_off_by_one(int32_t x) { - arr[-1] = x; /* { dg-warning "buffer underflow" "warning" } */ + arr[-1] = x; /* { dg-warning "buffer underwrite" "warning" } */ /* { dg-message "out-of-bounds write from byte -4 till byte -1 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c new file mode 100644 index 00000000000..cf6b458f44b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-write-struct-arr.c @@ -0,0 +1,65 @@ +#include + +struct st +{ + char buf[16]; + int32_t x; + int32_t y; +}; + +struct st arr[10]; + +void struct_arr_write_x_element_before_start_far(int32_t x) +{ + arr[-100].x = x; /* { dg-warning "buffer underwrite" "warning" } */ + /* { dg-message "out-of-bounds write from byte -2384 till byte -2381 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +void struct_arr_write_x_element_before_start_near(int32_t x) +{ + arr[-2].x = x; /* { dg-warning "buffer underwrite" "warning" } */ + /* { dg-message "out-of-bounds write from byte -32 till byte -29 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +void struct_arr_write_x_element_before_start_off_by_one(int32_t x) +{ + arr[-1].x = x; /* { dg-warning "buffer underwrite" "warning" } */ + /* { dg-message "out-of-bounds write from byte -8 till byte -5 but 'arr' starts at byte 0" "final event" { target *-*-* } .-1 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-2 } */ +} + +void struct_arr_write_x_element_at_start(int32_t x) +{ + arr[0].x = x; +} + +void struct_arr_write_x_element_at_end(int32_t x) +{ + arr[9].x = x; +} + +void struct_arr_write_x_element_after_end_off_by_one(int32_t x) +{ + arr[10].x = x; /* { dg-warning "buffer overflow" "warning" } */ + /* { dg-message "out-of-bounds write from byte 256 till byte 259 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} + +void struct_arr_write_x_element_after_end_near(int32_t x) +{ + arr[11].x = x; /* { dg-warning "buffer overflow" "warning" } */ + /* { dg-message "out-of-bounds write from byte 280 till byte 283 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} + +void struct_arr_write_x_element_after_end_far(int32_t x) +{ + arr[100].x = x; /* { dg-warning "buffer overflow" "warning" } */ + /* { dg-message "out-of-bounds write from byte 2416 till byte 2419 but 'arr' ends at byte 240" "final event" { target *-*-* } .-1 } */ + /* { dg-message "write of 4 bytes to beyond the end of 'arr'" "num bad bytes note" { target *-*-* } .-2 } */ + /* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } .-3 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/pr101962.c b/gcc/testsuite/gcc.dg/analyzer/pr101962.c index cf0041b2fcd..08c0aba5cbf 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr101962.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr101962.c @@ -25,7 +25,7 @@ test_1 (void) return *a; /* { dg-line test_1 } */ /* { dg-warning "use of uninitialized value '\\*a'" "warning" { target *-*-* } test_1 } */ - /* { dg-warning "overread" "warning" { target *-*-* } test_1 } */ + /* { dg-warning "stack-based buffer over-read" "warning" { target *-*-* } test_1 } */ } static const char * __attribute__((noinline)) diff --git a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c index 2efe3371e12..75f0b70a996 100644 --- a/gcc/testsuite/gcc.dg/analyzer/realloc-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/realloc-5.c @@ -37,7 +37,7 @@ void test_1 () __analyzer_eval (q[8] == 1); /* { dg-line eval } */ /* { dg-warning "UNKNOWN" "warning" { target *-*-* } eval } */ - /* { dg-warning "overread" "warning" { target *-*-* } eval } */ + /* { dg-warning "heap-based buffer over-read" "warning" { target *-*-* } eval } */ /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } eval } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c index b05b8629a7f..def83006a71 100644 --- a/gcc/testsuite/gcc.dg/analyzer/zlib-3.c +++ b/gcc/testsuite/gcc.dg/analyzer/zlib-3.c @@ -184,7 +184,7 @@ static int huft_build(uInt *b, uInt n, uInt s, const uInt *d, const uInt *e, mask = (1 << w) - 1; /* The analyzer thinks that h can be -1 here. This is probably a false positive. */ - while ((i & mask) != x[h]) { /* { dg-bogus "underread" "" { xfail *-*-* } } */ + while ((i & mask) != x[h]) { /* { dg-bogus "under-read" "" { xfail *-*-* } } */ h--; w -= l; mask = (1 << w) - 1; From patchwork Thu Dec 1 02:41:58 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61293 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 E8967384F494 for ; Thu, 1 Dec 2022 02:43:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E8967384F494 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862624; bh=HT7dBvmtwLm5uEXN425VW0YbErnHYl1RjQUmLJthLsU=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=sOYIf6cuZoJjx7tPvKS+LV7yTSM52+dJvX/NhK+TZZZrOpJx/lwyCvmyvy2p+zEYG XjDgZ1fOQMIakpz9x28nsp40vtEFkqv+/HG7COF8tquJWWRRdtBR+zXnpaoKJXboGX MNEOZBFWTp0Hs6oxIkPKrQcsBSJOHnpcWGT2XgXU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 1B9063858434 for ; Thu, 1 Dec 2022 02:42:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B9063858434 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-625-BGvFz16eOT6vc-w6iAsuNA-1; Wed, 30 Nov 2022 21:42:06 -0500 X-MC-Unique: BGvFz16eOT6vc-w6iAsuNA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1BC0E29AB3FC for ; Thu, 1 Dec 2022 02:42:06 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id EC6512024CBE; Thu, 1 Dec 2022 02:42:05 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 5/7] diagnostics: tweak diagnostic_path::interprocedural_p [PR106626] Date: Wed, 30 Nov 2022 21:41:58 -0500 Message-Id: <20221201024200.3722982-5-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" The region-creation event at the start of... : In function 'int_arr_write_element_after_end_off_by_one': :14:11: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds] 14 | arr[10] = x; | ~~~~~~~~^~~ event 1 | | 10 | int32_t arr[10]; | | ^~~ | | | | | (1) capacity is 40 bytes | +--> 'int_arr_write_element_after_end_off_by_one': events 2-3 | | 12 | void int_arr_write_element_after_end_off_by_one(int32_t x) | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | | | | (2) entry to 'int_arr_write_element_after_end_off_by_one' | 13 | { | 14 | arr[10] = x; /* { dg-line line } */ | | ~~~~~~~~~~~ | | | | | (3) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40 | :14:11: note: write of 4 bytes to beyond the end of 'arr' 14 | arr[10] = x; | ~~~~~~~~^~~ :14:11: note: valid subscripts for 'arr' are '[0]' to '[9]' ...makes diagnostic_manager::finish_pruning consider the path to be interprocedural, and so it doesn't prune the function entry event. This patch tweaks diagnostic_path::interprocedural_p to ignore leading events outside of any function, so that it considers the path to be intraprocedural, and thus diagnostic_manager::finish_pruning prunes the function entry event, leading to this simpler output: : In function 'int_arr_write_element_after_end_off_by_one': :14:11: warning: buffer overflow [CWE-787] [-Wanalyzer-out-of-bounds] 14 | arr[10] = x; | ~~~~~~~~^~~ event 1 | | 10 | int32_t arr[10]; | | ^~~ | | | | | (1) capacity is 40 bytes | +--> 'int_arr_write_element_after_end_off_by_one': event 2 | | 14 | arr[10] = x; | | ~~~~~~~~^~~ | | | | | (2) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40 | :14:11: note: write of 4 bytes to beyond the end of 'arr' :14:11: note: valid subscripts for 'arr' are '[0]' to '[9]' Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4429-g1d86af242bc4a8. gcc/ChangeLog: PR analyzer/106626 * diagnostic-path.h (diagnostic_path::get_first_event_in_a_function): New decl. * diagnostic.cc (diagnostic_path::get_first_event_in_a_function): New. (diagnostic_path::interprocedural_p): Ignore leading events that are outside of any function. gcc/testsuite/ChangeLog: PR analyzer/106626 * gcc.dg/analyzer/out-of-bounds-multiline-1.c: New test. Signed-off-by: David Malcolm --- gcc/diagnostic-path.h | 3 ++ gcc/diagnostic.cc | 37 +++++++++++++++++-- .../analyzer/out-of-bounds-multiline-1.c | 37 +++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c diff --git a/gcc/diagnostic-path.h b/gcc/diagnostic-path.h index 8ce4ff763d4..aa5cda8c23a 100644 --- a/gcc/diagnostic-path.h +++ b/gcc/diagnostic-path.h @@ -167,6 +167,9 @@ class diagnostic_path virtual const diagnostic_event & get_event (int idx) const = 0; bool interprocedural_p () const; + +private: + bool get_first_event_in_a_function (unsigned *out_idx) const; }; /* Concrete subclasses. */ diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc index a9562a815b1..322515b3242 100644 --- a/gcc/diagnostic.cc +++ b/gcc/diagnostic.cc @@ -939,18 +939,49 @@ diagnostic_event::meaning::maybe_get_property_str (enum property p) /* class diagnostic_path. */ +/* Subroutint of diagnostic_path::interprocedural_p. + Look for the first event in this path that is within a function + i.e. has a non-NULL fndecl, and a non-zero stack depth. + If found, write its index to *OUT_IDX and return true. + Otherwise return false. */ + +bool +diagnostic_path::get_first_event_in_a_function (unsigned *out_idx) const +{ + const unsigned num = num_events (); + for (unsigned i = 0; i < num; i++) + { + if (!(get_event (i).get_fndecl () == NULL + && get_event (i).get_stack_depth () == 0)) + { + *out_idx = i; + return true; + } + } + return false; +} + /* Return true if the events in this path involve more than one function, or false if it is purely intraprocedural. */ bool diagnostic_path::interprocedural_p () const { + /* Ignore leading events that are outside of any function. */ + unsigned first_fn_event_idx; + if (!get_first_event_in_a_function (&first_fn_event_idx)) + return false; + + const diagnostic_event &first_fn_event = get_event (first_fn_event_idx); + tree first_fndecl = first_fn_event.get_fndecl (); + int first_fn_stack_depth = first_fn_event.get_stack_depth (); + const unsigned num = num_events (); - for (unsigned i = 0; i < num; i++) + for (unsigned i = first_fn_event_idx + 1; i < num; i++) { - if (get_event (i).get_fndecl () != get_event (0).get_fndecl ()) + if (get_event (i).get_fndecl () != first_fndecl) return true; - if (get_event (i).get_stack_depth () != get_event (0).get_stack_depth ()) + if (get_event (i).get_stack_depth () != first_fn_stack_depth) return true; } return false; diff --git a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c new file mode 100644 index 00000000000..25301e9e2ff --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-multiline-1.c @@ -0,0 +1,37 @@ +/* Integration test of how the execution path looks for + -Wanalyzer-out-of-bounds. */ + +/* { dg-additional-options "-fdiagnostics-show-path-depths" } */ +/* { dg-additional-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */ + + +#include + +int32_t arr[10]; + +void int_arr_write_element_after_end_off_by_one(int32_t x) +{ + arr[10] = x; /* { dg-line line } */ +} +/* { dg-warning "buffer overflow" "warning" { target *-*-* } line } */ +/* { dg-message "valid subscripts for 'arr' are '\\\[0\\\]' to '\\\[9\\\]'" "valid subscript note" { target *-*-* } line } */ + + +/* { dg-begin-multiline-output "" } + arr[10] = x; + ~~~~~~~~^~~ + event 1 (depth 0) + | + | int32_t arr[10]; + | ^~~ + | | + | (1) capacity is 40 bytes + | + +--> 'int_arr_write_element_after_end_off_by_one': event 2 (depth 1) + | + | arr[10] = x; + | ~~~~~~~~^~~ + | | + | (2) out-of-bounds write from byte 40 till byte 43 but 'arr' ends at byte 40 + | + { dg-end-multiline-output "" } */ From patchwork Thu Dec 1 02:41:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61291 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 B866A3861875 for ; Thu, 1 Dec 2022 02:42:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B866A3861875 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862572; bh=u+dbsADotQfxrDaM+xHPt8z27v4Yip8akmlZsAi8Mn0=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=o25IkuSmvnfgMm3hc00UZPJIFvaJRBpwQTZXTeIVSbspEDj/vFRfU9d3Qg3XyyUlB uwwuMGUjBxn6XtnZzbQOoVk+9PArVFT0NAATDL4XhQGEZbFz/yeli8MJBF7bWlcXSP qWXQPW7bhPmhNKTfQZV8pDN2aTWG3ZNWNz5bu+p4= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 68CC03858281 for ; Thu, 1 Dec 2022 02:42:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 68CC03858281 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-118-KpReLODxOVm1G7U-qX6KWw-1; Wed, 30 Nov 2022 21:42:06 -0500 X-MC-Unique: KpReLODxOVm1G7U-qX6KWw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 4EF07101A52A for ; Thu, 1 Dec 2022 02:42:06 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A270200D8C3; Thu, 1 Dec 2022 02:42:06 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 6/7] analyzer: unify bounds-checking class hierarchies Date: Wed, 30 Nov 2022 21:41:59 -0500 Message-Id: <20221201024200.3722982-6-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Convert out-of-bounds class hierarchy from: pending_diagnostic out_of_bounds past_the_end buffer_overflow (*) buffer_over_read (*) buffer_underwrite (*) buffer_under_read (*) symbolic_past_the_end symbolic_buffer_overflow (*) symbolic_buffer_over_read (*) to: pending_diagnostic out_of_bounds concrete_out_of_bounds concrete_past_the_end concrete_buffer_overflow (*) concrete_buffer_over_read (*) concrete_buffer_underwrite (*) concrete_buffer_under_read (*) symbolic_past_the_end symbolic_buffer_overflow (*) symbolic_buffer_over_read (*) where the concrete classes (i.e. the instantiable ones) are marked with a (*). Doing so undercovered a bug where, for CWE-131-examples.c, we were emitting an extra: warning: heap-based buffer over-read [CWE-122] [-Wanalyzer-out-of-bounds] at the: WidgetList[numWidgets] = NULL; The issue was that within set_next_state we get the rvalue for the LHS, which looks like a read to the bounds-checker. The patch fixes this by passing NULL as the region_model_context * for such accesses. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4430-g8bc9e4ee874ea3. gcc/analyzer/ChangeLog: * bounds-checking.cc (class out_of_bounds): Split out from... (class concrete_out_of_bounds): New abstract subclass. (class past_the_end): Rename to... (class concrete_past_the_end): ...this, and make a subclass of concrete_out_of_bounds. (class buffer_overflow): Rename to... (class concrete_buffer_overflow): ...this, and make a subclass of concrete_past_the_end. (class buffer_over_read): Rename to... (class concrete_buffer_over_read): ...this, and make a subclass of concrete_past_the_end. (class buffer_underwrite): Rename to... (class concrete_buffer_underwrite): ...this, and make a subclass of concrete_out_of_bounds. (class buffer_under_read): Rename to... (class concrete_buffer_under_read): ...this, and make a subclass of concrete_out_of_bounds. (class symbolic_past_the_end): Convert to a subclass of out_of_bounds. (symbolic_buffer_overflow::get_kind): New. (symbolic_buffer_over_read::get_kind): New. (region_model::check_region_bounds): Update for renamings. * engine.cc (impl_sm_context::set_next_state): Eliminate "new_ctxt", passing NULL to get_rvalue instead. (impl_sm_context::warn): Likewise. Signed-off-by: David Malcolm --- gcc/analyzer/bounds-checking.cc | 185 +++++++++++++++++++------------- gcc/analyzer/engine.cc | 24 +---- 2 files changed, 115 insertions(+), 94 deletions(-) diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index bc7d2dd17ae..aaf3f22109b 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -37,27 +37,21 @@ along with GCC; see the file COPYING3. If not see namespace ana { -/* Abstract base class for all out-of-bounds warnings with concrete values. */ +/* Abstract base class for all out-of-bounds warnings. */ -class out_of_bounds : public pending_diagnostic_subclass +class out_of_bounds : public pending_diagnostic { 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) + out_of_bounds (const region *reg, tree diag_arg) + : m_reg (reg), m_diag_arg (diag_arg) {} - const char *get_kind () const final override - { - return "out_of_bounds_diagnostic"; - } - - bool operator== (const out_of_bounds &other) const + bool subclass_equal_p (const pending_diagnostic &base_other) const override { - 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); + const out_of_bounds &other + (static_cast (base_other)); + return (m_reg == other.m_reg + && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg)); } int get_controlling_option () const final override @@ -106,25 +100,51 @@ protected: const region *m_reg; tree m_diag_arg; +}; + +/* Abstract base class for all out-of-bounds warnings where the + out-of-bounds range is concrete. */ + +class concrete_out_of_bounds : public out_of_bounds +{ +public: + concrete_out_of_bounds (const region *reg, tree diag_arg, + byte_range out_of_bounds_range) + : out_of_bounds (reg, diag_arg), + m_out_of_bounds_range (out_of_bounds_range) + {} + + bool subclass_equal_p (const pending_diagnostic &base_other) const override + { + const concrete_out_of_bounds &other + (static_cast (base_other)); + return (out_of_bounds::subclass_equal_p (other) + && m_out_of_bounds_range == other.m_out_of_bounds_range); + } + +protected: byte_range m_out_of_bounds_range; }; -/* Abstract subclass to complaing about out-of-bounds +/* Abstract subclass to complaing about concrete out-of-bounds past the end of the buffer. */ -class past_the_end : public out_of_bounds +class concrete_past_the_end : public concrete_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) + concrete_past_the_end (const region *reg, tree diag_arg, byte_range range, + tree byte_bound) + : concrete_out_of_bounds (reg, diag_arg, range), m_byte_bound (byte_bound) {} - bool operator== (const past_the_end &other) const + bool + subclass_equal_p (const pending_diagnostic &base_other) const final override { - return out_of_bounds::operator== (other) - && pending_diagnostic::same_tree_p (m_byte_bound, - other.m_byte_bound); + const concrete_past_the_end &other + (static_cast (base_other)); + return (concrete_out_of_bounds::subclass_equal_p (other) + && pending_diagnostic::same_tree_p (m_byte_bound, + other.m_byte_bound)); } label_text @@ -143,14 +163,19 @@ protected: /* Concrete subclass to complain about buffer overflows. */ -class buffer_overflow : public past_the_end +class concrete_buffer_overflow : public concrete_past_the_end { public: - buffer_overflow (const region *reg, tree diag_arg, + concrete_buffer_overflow (const region *reg, tree diag_arg, byte_range range, tree byte_bound) - : past_the_end (reg, diag_arg, range, byte_bound) + : concrete_past_the_end (reg, diag_arg, range, byte_bound) {} + const char *get_kind () const final override + { + return "concrete_buffer_overflow"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -241,14 +266,19 @@ public: /* Concrete subclass to complain about buffer over-reads. */ -class buffer_over_read : public past_the_end +class concrete_buffer_over_read : public concrete_past_the_end { public: - buffer_over_read (const region *reg, tree diag_arg, - byte_range range, tree byte_bound) - : past_the_end (reg, diag_arg, range, byte_bound) + concrete_buffer_over_read (const region *reg, tree diag_arg, + byte_range range, tree byte_bound) + : concrete_past_the_end (reg, diag_arg, range, byte_bound) {} + const char *get_kind () const final override + { + return "concrete_buffer_over_read"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -337,13 +367,19 @@ public: /* Concrete subclass to complain about buffer underwrites. */ -class buffer_underwrite : public out_of_bounds +class concrete_buffer_underwrite : public concrete_out_of_bounds { public: - buffer_underwrite (const region *reg, tree diag_arg, byte_range range) - : out_of_bounds (reg, diag_arg, range) + concrete_buffer_underwrite (const region *reg, tree diag_arg, + byte_range range) + : concrete_out_of_bounds (reg, diag_arg, range) {} + const char *get_kind () const final override + { + return "concrete_buffer_underwrite"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -403,13 +439,19 @@ public: /* Concrete subclass to complain about buffer under-reads. */ -class buffer_under_read : public out_of_bounds +class concrete_buffer_under_read : public concrete_out_of_bounds { public: - buffer_under_read (const region *reg, tree diag_arg, byte_range range) - : out_of_bounds (reg, diag_arg, range) + concrete_buffer_under_read (const region *reg, tree diag_arg, + byte_range range) + : concrete_out_of_bounds (reg, diag_arg, range) {} + const char *get_kind () const final override + { + return "concrete_buffer_under_read"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -470,38 +512,26 @@ public: /* Abstract class to complain about out-of-bounds read/writes where the values are symbolic. */ -class symbolic_past_the_end - : public pending_diagnostic_subclass +class symbolic_past_the_end : public out_of_bounds { public: symbolic_past_the_end (const region *reg, tree diag_arg, tree offset, tree num_bytes, tree capacity) - : m_reg (reg), m_diag_arg (diag_arg), m_offset (offset), - m_num_bytes (num_bytes), m_capacity (capacity) + : out_of_bounds (reg, diag_arg), + m_offset (offset), + m_num_bytes (num_bytes), + m_capacity (capacity) {} - const char *get_kind () const final override - { - return "symbolic_past_the_end"; - } - - bool operator== (const symbolic_past_the_end &other) const - { - return m_reg == other.m_reg - && pending_diagnostic::same_tree_p (m_diag_arg, other.m_diag_arg) - && pending_diagnostic::same_tree_p (m_offset, other.m_offset) - && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes) - && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity); - } - - int get_controlling_option () const final override - { - return OPT_Wanalyzer_out_of_bounds; - } - - void mark_interesting_stuff (interesting_t *interest) final override + bool + subclass_equal_p (const pending_diagnostic &base_other) const final override { - interest->add_region_creation (m_reg); + const symbolic_past_the_end &other + (static_cast (base_other)); + return (out_of_bounds::subclass_equal_p (other) + && pending_diagnostic::same_tree_p (m_offset, other.m_offset) + && pending_diagnostic::same_tree_p (m_num_bytes, other.m_num_bytes) + && pending_diagnostic::same_tree_p (m_capacity, other.m_capacity)); } label_text @@ -566,13 +596,6 @@ public: } protected: - enum memory_space get_memory_space () const - { - return m_reg->get_memory_space (); - } - - const region *m_reg; - tree m_diag_arg; tree m_offset; tree m_num_bytes; tree m_capacity; @@ -591,6 +614,11 @@ public: m_dir_str = "write"; } + const char *get_kind () const final override + { + return "symbolic_buffer_overflow"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -624,6 +652,11 @@ public: m_dir_str = "read"; } + const char *get_kind () const final override + { + return "symbolic_buffer_over_read"; + } + bool emit (rich_location *rich_loc) final override { diagnostic_metadata m; @@ -776,10 +809,12 @@ region_model::check_region_bounds (const region *reg, gcc_unreachable (); break; case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, out)); + ctxt->warn (make_unique (reg, diag_arg, + out)); break; case DIR_WRITE: - ctxt->warn (make_unique (reg, diag_arg, out)); + ctxt->warn (make_unique (reg, diag_arg, + out)); break; } } @@ -804,12 +839,12 @@ region_model::check_region_bounds (const region *reg, gcc_unreachable (); break; case DIR_READ: - ctxt->warn (make_unique (reg, diag_arg, - out, byte_bound)); + ctxt->warn (make_unique (reg, diag_arg, + out, byte_bound)); break; case DIR_WRITE: - ctxt->warn (make_unique (reg, diag_arg, - out, byte_bound)); + ctxt->warn (make_unique (reg, diag_arg, + out, byte_bound)); break; } } diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 0c49bb26872..991b592b828 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -310,21 +310,17 @@ public: } - void set_next_state (const gimple *stmt, + void set_next_state (const gimple *, tree var, state_machine::state_t to, tree origin) final override { logger * const logger = get_logger (); LOG_FUNC (logger); - impl_region_model_context new_ctxt (m_eg, m_enode_for_diag, - m_old_state, m_new_state, - NULL, NULL, - stmt); const svalue *var_new_sval - = m_new_state->m_region_model->get_rvalue (var, &new_ctxt); + = m_new_state->m_region_model->get_rvalue (var, NULL); const svalue *origin_new_sval - = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt); + = m_new_state->m_region_model->get_rvalue (origin, NULL); /* We use the new sval here to avoid issues with uninitialized values. */ state_machine::state_t current @@ -350,12 +346,8 @@ public: (m_eg, m_enode_for_diag, NULL, NULL, NULL/*m_enode->get_state ()*/, NULL, stmt); - impl_region_model_context new_ctxt (m_eg, m_enode_for_diag, - m_old_state, m_new_state, - NULL, NULL, - stmt); const svalue *origin_new_sval - = m_new_state->m_region_model->get_rvalue (origin, &new_ctxt); + = m_new_state->m_region_model->get_rvalue (origin, NULL); state_machine::state_t current = m_old_smap->get_state (sval, m_eg.get_ext_state ()); @@ -380,11 +372,8 @@ public: { LOG_FUNC (get_logger ()); gcc_assert (d); - impl_region_model_context old_ctxt - (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL); - const svalue *var_old_sval - = m_old_state->m_region_model->get_rvalue (var, &old_ctxt); + = m_old_state->m_region_model->get_rvalue (var, NULL); state_machine::state_t current = (var ? m_old_smap->get_state (var_old_sval, m_eg.get_ext_state ()) @@ -400,9 +389,6 @@ public: { LOG_FUNC (get_logger ()); gcc_assert (d); - impl_region_model_context old_ctxt - (m_eg, m_enode_for_diag, m_old_state, m_new_state, NULL, NULL, NULL); - state_machine::state_t current = (sval ? m_old_smap->get_state (sval, m_eg.get_ext_state ()) From patchwork Thu Dec 1 02:42:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 61295 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 04E26386EC0C for ; Thu, 1 Dec 2022 02:44:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 04E26386EC0C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669862687; bh=grftwHGIFosY4NUhXGi7ht+WbNepJiRRUvyx5Id0h5Q=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BPPh/RVSpmaJGWYauorlTQOFB1FgVBFmf0OBuGyrCQje3Wjd3QLHOgavSMKMCUBvR 6wZgKQgS/d/Je47DUIIWQLoBppadiERrxion9ciKiYrSzpl2JdKu/HMX4mTsgt1alp rNGuw7hBuc4CQhxXCei/sNr5ibvhixFSX587glbU= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 7E6E73858288 for ; Thu, 1 Dec 2022 02:42:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7E6E73858288 Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-47-iqoDkCXnPIuCtCkyPv24rQ-1; Wed, 30 Nov 2022 21:42:06 -0500 X-MC-Unique: iqoDkCXnPIuCtCkyPv24rQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 83779101A528 for ; Thu, 1 Dec 2022 02:42:06 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.16.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F24A2024CC5; Thu, 1 Dec 2022 02:42:06 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed 7/7] analyzer: fix i18n issues in symbolic out-of-bounds [PR106626] Date: Wed, 30 Nov 2022 21:42:00 -0500 Message-Id: <20221201024200.3722982-7-dmalcolm@redhat.com> In-Reply-To: <20221201024200.3722982-1-dmalcolm@redhat.com> References: <20221201024200.3722982-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: David Malcolm via Gcc-patches From: David Malcolm Reply-To: David Malcolm Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-4431-geaaf97b6147095. gcc/analyzer/ChangeLog: PR analyzer/106626 * bounds-checking.cc (symbolic_past_the_end::describe_final_event): Delete, moving to symbolic_buffer_overflow::describe_final_event and symbolic_buffer_over_read::describe_final_event, eliminating composition of text strings via "byte_str" and "m_dir_str". (symbolic_past_the_end::m_dir_str): Delete field. (symbolic_buffer_overflow::symbolic_buffer_overflow): Drop m_dir_str. (symbolic_buffer_overflow::describe_final_event): New, as noted above. (symbolic_buffer_over_read::symbolic_buffer_overflow): Drop m_dir_str. (symbolic_buffer_over_read::describe_final_event): New, as noted above. Signed-off-by: David Malcolm --- gcc/analyzer/bounds-checking.cc | 192 +++++++++++++++++++++++--------- 1 file changed, 138 insertions(+), 54 deletions(-) diff --git a/gcc/analyzer/bounds-checking.cc b/gcc/analyzer/bounds-checking.cc index aaf3f22109b..1c44790f86d 100644 --- a/gcc/analyzer/bounds-checking.cc +++ b/gcc/analyzer/bounds-checking.cc @@ -544,62 +544,10 @@ public: return label_text (); } - label_text - describe_final_event (const evdesc::final_event &ev) final override - { - const char *byte_str; - if (pending_diagnostic::same_tree_p (m_num_bytes, integer_one_node)) - byte_str = "byte"; - else - byte_str = "bytes"; - - if (m_offset) - { - if (m_num_bytes && TREE_CODE (m_num_bytes) == INTEGER_CST) - { - if (m_diag_arg) - return ev.formatted_print ("%s of %E %s at offset %qE" - " exceeds %qE", m_dir_str, - m_num_bytes, byte_str, - m_offset, m_diag_arg); - else - return ev.formatted_print ("%s of %E %s at offset %qE" - " exceeds the buffer", m_dir_str, - m_num_bytes, byte_str, m_offset); - } - else if (m_num_bytes) - { - if (m_diag_arg) - return ev.formatted_print ("%s of %qE %s at offset %qE" - " exceeds %qE", m_dir_str, - m_num_bytes, byte_str, - m_offset, m_diag_arg); - else - return ev.formatted_print ("%s of %qE %s at offset %qE" - " exceeds the buffer", m_dir_str, - m_num_bytes, byte_str, m_offset); - } - else - { - if (m_diag_arg) - return ev.formatted_print ("%s at offset %qE exceeds %qE", - m_dir_str, m_offset, m_diag_arg); - else - return ev.formatted_print ("%s at offset %qE exceeds the" - " buffer", m_dir_str, m_offset); - } - } - if (m_diag_arg) - return ev.formatted_print ("out-of-bounds %s on %qE", - m_dir_str, m_diag_arg); - return ev.formatted_print ("out-of-bounds %s", m_dir_str); - } - protected: tree m_offset; tree m_num_bytes; tree m_capacity; - const char *m_dir_str; }; /* Concrete subclass to complain about overflows with symbolic values. */ @@ -611,7 +559,6 @@ public: tree num_bytes, tree capacity) : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) { - m_dir_str = "write"; } const char *get_kind () const final override @@ -638,6 +585,75 @@ public: "heap-based buffer overflow"); } } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + if (m_offset) + { + /* Known offset. */ + if (m_num_bytes) + { + /* Known offset, known size. */ + if (TREE_CODE (m_num_bytes) == INTEGER_CST) + { + /* Known offset, known constant size. */ + if (pending_diagnostic::same_tree_p (m_num_bytes, + integer_one_node)) + { + /* Singular m_num_bytes. */ + if (m_diag_arg) + return ev.formatted_print + ("write of %E byte at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("write of %E byte at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + else + { + /* Plural m_num_bytes. */ + if (m_diag_arg) + return ev.formatted_print + ("write of %E bytes at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("write of %E bytes at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + } + else + { + /* Known offset, known symbolic size. */ + if (m_diag_arg) + return ev.formatted_print + ("write of %qE bytes at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("write of %qE bytes at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + } + else + { + /* Known offset, unknown size. */ + if (m_diag_arg) + return ev.formatted_print ("write at offset %qE exceeds %qE", + m_offset, m_diag_arg); + else + return ev.formatted_print ("write at offset %qE exceeds the" + " buffer", m_offset); + } + } + /* Unknown offset. */ + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds write on %qE", + m_diag_arg); + return ev.formatted_print ("out-of-bounds write"); + } }; /* Concrete subclass to complain about over-reads with symbolic values. */ @@ -649,7 +665,6 @@ public: tree num_bytes, tree capacity) : symbolic_past_the_end (reg, diag_arg, offset, num_bytes, capacity) { - m_dir_str = "read"; } const char *get_kind () const final override @@ -677,6 +692,75 @@ public: "heap-based buffer over-read"); } } + + label_text + describe_final_event (const evdesc::final_event &ev) final override + { + if (m_offset) + { + /* Known offset. */ + if (m_num_bytes) + { + /* Known offset, known size. */ + if (TREE_CODE (m_num_bytes) == INTEGER_CST) + { + /* Known offset, known constant size. */ + if (pending_diagnostic::same_tree_p (m_num_bytes, + integer_one_node)) + { + /* Singular m_num_bytes. */ + if (m_diag_arg) + return ev.formatted_print + ("read of %E byte at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("read of %E byte at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + else + { + /* Plural m_num_bytes. */ + if (m_diag_arg) + return ev.formatted_print + ("read of %E bytes at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("read of %E bytes at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + } + else + { + /* Known offset, known symbolic size. */ + if (m_diag_arg) + return ev.formatted_print + ("read of %qE bytes at offset %qE exceeds %qE", + m_num_bytes, m_offset, m_diag_arg); + else + return ev.formatted_print + ("read of %qE bytes at offset %qE exceeds the buffer", + m_num_bytes, m_offset); + } + } + else + { + /* Known offset, unknown size. */ + if (m_diag_arg) + return ev.formatted_print ("read at offset %qE exceeds %qE", + m_offset, m_diag_arg); + else + return ev.formatted_print ("read at offset %qE exceeds the" + " buffer", m_offset); + } + } + /* Unknown offset. */ + if (m_diag_arg) + return ev.formatted_print ("out-of-bounds read on %qE", + m_diag_arg); + return ev.formatted_print ("out-of-bounds read"); + } }; /* Check whether an access is past the end of the BASE_REG. */