From patchwork Fri Aug 11 22:12:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Malcolm X-Patchwork-Id: 134733 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp1385403vqi; Fri, 11 Aug 2023 15:13:11 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEF91yLkb9XavmXyfO1Q2Kr4lU6NnybTVE8wiZRDMEtyi5W/XKuUboQFJTNbbb6gWZk+Lu0 X-Received: by 2002:a5d:69ce:0:b0:317:5579:2520 with SMTP id s14-20020a5d69ce000000b0031755792520mr2068417wrw.1.1691791991410; Fri, 11 Aug 2023 15:13:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1691791991; cv=none; d=google.com; s=arc-20160816; b=iqzoCy2wv0W48HfZFrwnw3Y/UTRZupnNbe041EUXjyI8o+W834SSCYwCExlm9HhXRK Dj6Zx/dwdux9DVEshZuGfdLlYHe+0IFPLW8oOq9J0EdIwTSA2yCdRcu05aKjjpQPeucW vdcOPWJOth+YWwNz8/Xruva/eyIRKPz4UGvGMly3/wTS8lCFc7YPAxm9THoEdSYsJ4os 7CuWFMZIO11l3ODHlZut9rfiBE65BaSruW0J97pfnYphCVU6JK8BzdejegR+kiCuNqWd 86vGAqncn77UxSSV4elXcbzgu1JmmSeoTvMjKSbxCKwOdi/alO6ER2yD3Gc9cQ95Gld8 xfuw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:cc :to:dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=h2A6RSTAAT5lPpPk+1GHnRqdJ3cV+qIyaHXLL+CH8C0=; fh=fa0YJSH7y4k0snNuxsKgOI6vGP7j2f0jR9F1WHC95UA=; b=u9wG9T5yA7kq5FU74jImA47LNvb+VIw5Ac/Iy08i9C4RdpdpzHYguQTFpHEyeIvPpp XsxdoXIh/V+rT6b4cdDLDnmsSWF+P7BQD4roXtPTpero6/PGTpQP8Q2qhPaTIhDXXHUz R/N4dYB2a5WKz9oebeTqX7NiokGIssekjqRYzpzlAzsKbA5DgXyZvVPYLhbamBlNj3Zr AQlgw/ErGmlC1+pNO8q90/sGqCmRNGJE6FVYbVCJ6n7z+XFJuRXYe73mwRf5sR1flcFh LpxD0plA1lp5NH9lS2FaZi56HWYpV9Q5OvHINk624VWsGKogx0zgh1kBuNfvX+mXdCb4 B6Jg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="c/KxXTg8"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id m8-20020a056402050800b0051df5f4e88csi4120234edv.48.2023.08.11.15.13.11 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Aug 2023 15:13:11 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="c/KxXTg8"; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 488733858D28 for ; Fri, 11 Aug 2023 22:13:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 488733858D28 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1691791990; bh=h2A6RSTAAT5lPpPk+1GHnRqdJ3cV+qIyaHXLL+CH8C0=; h=To:Cc:Subject:Date:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=c/KxXTg85DWSCQGmYNfwsT5Xyvdw7HewD21t4cnJCQkC+mp8JMxLXhhn81rJHuPHc tsSTdQZsW9w38tNlMVh2IojvI+6LAg2sgXxjEW2s242wxyfzOof4Vt+Ay5mzEOLRY7 FqCSsIP+xIWa3gBQoZGwh3bVdr7RqWvLDK/0LX+s= 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 3F1BD3858004 for ; Fri, 11 Aug 2023 22:12:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3F1BD3858004 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-260-1WGNYskxOfeH9okGvNHauA-1; Fri, 11 Aug 2023 18:12:23 -0400 X-MC-Unique: 1WGNYskxOfeH9okGvNHauA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id CE028858EED for ; Fri, 11 Aug 2023 22:12:22 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.22.32.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id 985F640C2063; Fri, 11 Aug 2023 22:12:22 +0000 (UTC) To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [pushed] analyzer: new warning: -Wanalyzer-unterminated-string [PR105899] Date: Fri, 11 Aug 2023 18:12:12 -0400 Message-Id: <20230811221212.1959872-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 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_H4, RCVD_IN_MSPIKE_WL, 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+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1773972479186543599 X-GMAIL-MSGID: 1773972479186543599 This patch adds new functions to the analyzer for checking that an argument at a callsite is a pointer to a valid null-terminated string, and uses this for the following known functions: - error (param 3, the format string) - error_at_line (param 5, the format string) - putenv - strchr (1st param) - strcpy (2nd param) - strdup Currently the check merely detects pointers to unterminated string constants, and adds a new -Wanalyzer-unterminated-string to complain about that. I'm experimenting with detecting other ways in which a buffer can fail to be null-terminated, and for other problems with such buffers, but this patch at least adds the framework for wiring up the check to specific parameters of known_functions. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r14-3169-g325f9e88802daa. gcc/analyzer/ChangeLog: PR analyzer/105899 * analyzer.opt (Wanalyzer-unterminated-string): New. * call-details.cc (call_details::check_for_null_terminated_string_arg): New. * call-details.h (call_details::check_for_null_terminated_string_arg): New decl. * kf-analyzer.cc (class kf_analyzer_get_strlen): New. (register_known_analyzer_functions): Register it. * kf.cc (kf_error::impl_call_pre): Check that format arg is a valid null-terminated string. (kf_putenv::impl_call_pre): Likewise for the sole param. (kf_strchr::impl_call_pre): Likewise for the first param. (kf_strcpy::impl_call_pre): Likewise for the second param. (kf_strdup::impl_call_pre): Likewise for the sole param. * region-model.cc (get_strlen): New. (struct call_arg_details): New. (inform_about_expected_null_terminated_string_arg): New. (class unterminated_string_arg): New. (region_model::check_for_null_terminated_string_arg): New. * region-model.h (region_model::check_for_null_terminated_string_arg): New decl. gcc/ChangeLog: PR analyzer/105899 * doc/analyzer.texi (__analyzer_get_strlen): New. * doc/invoke.texi: Add -Wanalyzer-unterminated-string. gcc/testsuite/ChangeLog: PR analyzer/105899 * gcc.dg/analyzer/analyzer-decls.h (__analyzer_get_strlen): New. * gcc.dg/analyzer/error-1.c (test_error_unterminated): New. (test_error_at_line_unterminated): New. * gcc.dg/analyzer/null-terminated-strings-1.c: New test. * gcc.dg/analyzer/putenv-1.c (test_unterminated): New. * gcc.dg/analyzer/strchr-1.c (test_unterminated): New. * gcc.dg/analyzer/strcpy-1.c (test_unterminated): New. * gcc.dg/analyzer/strdup-1.c (test_unterminated): New. --- gcc/analyzer/analyzer.opt | 4 + gcc/analyzer/call-details.cc | 7 + gcc/analyzer/call-details.h | 2 + gcc/analyzer/kf-analyzer.cc | 18 ++ gcc/analyzer/kf.cc | 11 ++ gcc/analyzer/region-model.cc | 163 ++++++++++++++++++ gcc/analyzer/region-model.h | 3 + gcc/doc/analyzer.texi | 8 + gcc/doc/invoke.texi | 13 ++ .../gcc.dg/analyzer/analyzer-decls.h | 5 + gcc/testsuite/gcc.dg/analyzer/error-1.c | 12 ++ .../analyzer/null-terminated-strings-1.c | 30 ++++ gcc/testsuite/gcc.dg/analyzer/putenv-1.c | 7 + gcc/testsuite/gcc.dg/analyzer/strchr-1.c | 6 + gcc/testsuite/gcc.dg/analyzer/strcpy-1.c | 6 + gcc/testsuite/gcc.dg/analyzer/strdup-1.c | 6 + 16 files changed, 301 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index 2760aaa8151..a9bac400ca3 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -214,6 +214,10 @@ Wanalyzer-tainted-size Common Var(warn_analyzer_tainted_size) Init(1) Warning Warn about code paths in which an unsanitized value is used as a size. +Wanalyzer-unterminated-string +Common Var(warn_analyzer_unterminated_string) Init(1) Warning +Warn about code paths which attempt to find the length of an unterminated string. + Wanalyzer-use-after-free Common Var(warn_analyzer_use_after_free) Init(1) Warning Warn about code paths in which a freed value is used. diff --git a/gcc/analyzer/call-details.cc b/gcc/analyzer/call-details.cc index 93f4846f674..fa86f55177a 100644 --- a/gcc/analyzer/call-details.cc +++ b/gcc/analyzer/call-details.cc @@ -376,6 +376,13 @@ call_details::lookup_function_attribute (const char *attr_name) const return lookup_attribute (attr_name, TYPE_ATTRIBUTES (allocfntype)); } +void +call_details::check_for_null_terminated_string_arg (unsigned arg_idx) const +{ + region_model *model = get_model (); + model->check_for_null_terminated_string_arg (*this, arg_idx); +} + } // namespace ana #endif /* #if ENABLE_ANALYZER */ diff --git a/gcc/analyzer/call-details.h b/gcc/analyzer/call-details.h index bf2601151ea..0622cab7856 100644 --- a/gcc/analyzer/call-details.h +++ b/gcc/analyzer/call-details.h @@ -71,6 +71,8 @@ public: tree lookup_function_attribute (const char *attr_name) const; + void check_for_null_terminated_string_arg (unsigned arg_idx) const; + private: const gcall *m_call; region_model *m_model; diff --git a/gcc/analyzer/kf-analyzer.cc b/gcc/analyzer/kf-analyzer.cc index 5aed007d6ea..1a0c94089ac 100644 --- a/gcc/analyzer/kf-analyzer.cc +++ b/gcc/analyzer/kf-analyzer.cc @@ -358,6 +358,22 @@ public: } }; +/* Handler for "__analyzer_get_strlen". */ + +class kf_analyzer_get_strlen : public known_function +{ +public: + bool matches_call_types_p (const call_details &cd) const final override + { + return cd.num_args () == 1 && cd.arg_is_pointer_p (0); + } + void impl_call_pre (const call_details &cd) const final override + { + cd.check_for_null_terminated_string_arg (0); + cd.set_any_lhs_with_defaults (); + } +}; + /* Populate KFM with instances of known functions used for debugging the analyzer and for writing DejaGnu tests, all with a "__analyzer_" prefix. */ @@ -379,6 +395,8 @@ register_known_analyzer_functions (known_function_manager &kfm) kfm.add ("__analyzer_eval", make_unique ()); kfm.add ("__analyzer_get_unknown_ptr", make_unique ()); + kfm.add ("__analyzer_get_strlen", + make_unique ()); } } // namespace ana diff --git a/gcc/analyzer/kf.cc b/gcc/analyzer/kf.cc index b9ee2e45c86..6b2db861376 100644 --- a/gcc/analyzer/kf.cc +++ b/gcc/analyzer/kf.cc @@ -414,6 +414,10 @@ kf_error::impl_call_pre (const call_details &cd) const if (!model->add_constraint (status, EQ_EXPR, integer_zero_node, ctxt)) if (ctxt) ctxt->terminate_path (); + + /* Check "format" arg. */ + const int fmt_arg_idx = (m_min_args == 3) ? 2 : 4; + model->check_for_null_terminated_string_arg (cd, fmt_arg_idx); } /* Handler for "free", after sm-handling. @@ -674,6 +678,7 @@ public: gcc_assert (fndecl); region_model_context *ctxt = cd.get_ctxt (); region_model *model = cd.get_model (); + model->check_for_null_terminated_string_arg (cd, 0); const svalue *ptr_sval = cd.get_arg_svalue (0); const region *reg = model->deref_rvalue (ptr_sval, cd.get_arg_tree (0), ctxt); @@ -949,6 +954,10 @@ public: { return (cd.num_args () == 2 && cd.arg_is_pointer_p (0)); } + void impl_call_pre (const call_details &cd) const final override + { + cd.check_for_null_terminated_string_arg (0); + } void impl_call_post (const call_details &cd) const final override; }; @@ -1109,6 +1118,7 @@ kf_strcpy::impl_call_pre (const call_details &cd) const cd.get_ctxt ()); const svalue *src_contents_sval = model->get_store_value (src_reg, cd.get_ctxt ()); + cd.check_for_null_terminated_string_arg (1); cd.maybe_set_lhs (dest_sval); @@ -1136,6 +1146,7 @@ public: { region_model *model = cd.get_model (); region_model_manager *mgr = cd.get_manager (); + cd.check_for_null_terminated_string_arg (0); /* Ideally we'd get the size here, and simulate copying the bytes. */ const region *new_reg = model->get_or_create_region_for_heap_alloc (NULL, cd.get_ctxt ()); diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index aa9fe008b9d..494a9cdf149 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -3175,6 +3175,169 @@ region_model::set_value (tree lhs, tree rhs, region_model_context *ctxt) set_value (lhs_reg, rhs_sval, ctxt); } +/* Look for the first 0 byte within STRING_CST. + If there is one, write its index to *OUT and return true. + Otherwise, return false. */ + +static bool +get_strlen (tree string_cst, int *out) +{ + gcc_assert (TREE_CODE (string_cst) == STRING_CST); + + if (const void *p = memchr (TREE_STRING_POINTER (string_cst), + 0, + TREE_STRING_LENGTH (string_cst))) + { + *out = (const char *)p - TREE_STRING_POINTER (string_cst); + return true; + } + else + return false; +} + +/* A bundle of information about a problematic argument at a callsite + for use by pending_diagnostic subclasses for reporting and + for deduplication. */ + +struct call_arg_details +{ +public: + call_arg_details (const call_details &cd, unsigned arg_idx) + : m_call (cd.get_call_stmt ()), + m_called_fndecl (cd.get_fndecl_for_call ()), + m_arg_idx (arg_idx), + m_arg_expr (cd.get_arg_tree (arg_idx)) + { + } + + bool operator== (const call_arg_details &other) const + { + return (m_call == other.m_call + && m_called_fndecl == other.m_called_fndecl + && m_arg_idx == other.m_arg_idx + && pending_diagnostic::same_tree_p (m_arg_expr, other.m_arg_expr)); + } + + const gcall *m_call; + tree m_called_fndecl; + unsigned m_arg_idx; // 0-based + tree m_arg_expr; +}; + +/* Issue a note specifying that a particular function parameter is expected + to be a valid null-terminated string. */ + +static void +inform_about_expected_null_terminated_string_arg (const call_arg_details &ad) +{ + // TODO: ideally we'd underline the param here + inform (DECL_SOURCE_LOCATION (ad.m_called_fndecl), + "argument %d of %qD must be a pointer to a null-terminated string", + ad.m_arg_idx + 1, ad.m_called_fndecl); +} + +/* A subclass of pending_diagnostic for complaining about uses + of unterminated strings (thus accessing beyond the bounds + of a buffer). */ + +class unterminated_string_arg +: public pending_diagnostic_subclass +{ +public: + unterminated_string_arg (const call_arg_details arg_details) + : m_arg_details (arg_details) + { + gcc_assert (m_arg_details.m_called_fndecl); + } + + const char *get_kind () const final override + { + return "unterminated_string_arg"; + } + + bool operator== (const unterminated_string_arg &other) const + { + return m_arg_details == other.m_arg_details; + } + + int get_controlling_option () const final override + { + return OPT_Wanalyzer_unterminated_string; + } + + bool emit (rich_location *rich_loc, logger *) final override + { + auto_diagnostic_group d; + bool warned; + if (m_arg_details.m_arg_expr) + warned = warning_at (rich_loc, get_controlling_option (), + "passing pointer to unterminated string %qE" + " as argument %i of %qE", + m_arg_details.m_arg_expr, + m_arg_details.m_arg_idx + 1, + m_arg_details.m_called_fndecl); + else + warned = warning_at (rich_loc, get_controlling_option (), + "passing pointer to unterminated string" + " as argument %i of %qE", + m_arg_details.m_arg_idx + 1, + m_arg_details.m_called_fndecl); + if (warned) + inform_about_expected_null_terminated_string_arg (m_arg_details); + return warned; + } + + label_text describe_final_event (const evdesc::final_event &ev) final override + { + return ev.formatted_print + ("passing pointer to unterminated buffer as argument %i of %qE" + " would lead to read past the end of the buffer", + m_arg_details.m_arg_idx + 1, + m_arg_details.m_called_fndecl); + } + +private: + const call_arg_details m_arg_details; +}; + +/* Check that argument ARG_IDX (0-based) to the call described by CD + is a pointer to a valid null-terminated string. + + Complain if the buffer pointed to isn't null-terminated. + + TODO: we should also complain if: + - the pointer is NULL (or could be) + - the buffer pointed to is uninitalized before any 0-terminator + - the 0-terminator is within the bounds of the underlying base region + + We're checking that the called function could validly iterate through + the buffer reading it until it finds a 0 byte (such as by calling + strlen, or equivalent code). */ + +void +region_model::check_for_null_terminated_string_arg (const call_details &cd, + unsigned arg_idx) +{ + region_model_context *ctxt = cd.get_ctxt (); + + const svalue *arg_sval = cd.get_arg_svalue (arg_idx); + const region *buf_reg + = deref_rvalue (arg_sval, cd.get_arg_tree (arg_idx), ctxt); + + const svalue *contents_sval = get_store_value (buf_reg, ctxt); + + if (tree cst = contents_sval->maybe_get_constant ()) + if (TREE_CODE (cst) == STRING_CST) + { + int cst_strlen; + if (!get_strlen (cst, &cst_strlen)) + { + call_arg_details arg_details (cd, arg_idx); + ctxt->warn (make_unique (arg_details)); + } + } +} + /* Remove all bindings overlapping REG within the store. */ void diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index a8acad8b7b2..4f09f2e585a 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -502,6 +502,9 @@ class region_model const svalue *sval_hint, region_model_context *ctxt) const; + void check_for_null_terminated_string_arg (const call_details &cd, + unsigned idx); + private: const region *get_lvalue_1 (path_var pv, region_model_context *ctxt) const; const svalue *get_rvalue_1 (path_var pv, region_model_context *ctxt) const; diff --git a/gcc/doc/analyzer.texi b/gcc/doc/analyzer.texi index 2692b0e3ece..c50d7eb0ce8 100644 --- a/gcc/doc/analyzer.texi +++ b/gcc/doc/analyzer.texi @@ -652,6 +652,14 @@ __analyzer_get_unknown_ptr (); @end smallexample will obtain an unknown @code{void *}. +@item __analyzer_get_strlen +@smallexample +__analyzer_get_strlen (buf); +@end smallexample +will emit a warning if PTR doesn't point to a null-terminated string. +TODO: eventually get the strlen of the buffer (without the +optimizer touching it). + @end table @subsection Other Debugging Techniques diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 674f956f4b8..18ec61eada8 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -486,6 +486,7 @@ Objective-C and Objective-C++ Dialects}. -Wno-analyzer-tainted-size -Wanalyzer-too-complex -Wno-analyzer-unsafe-call-within-signal-handler +-Wno-analyzer-unterminated-string -Wno-analyzer-use-after-free -Wno-analyzer-use-of-pointer-in-stale-stack-frame -Wno-analyzer-use-of-uninitialized-value @@ -10318,6 +10319,7 @@ Enabling this option effectively enables the following warnings: -Wanalyzer-shift-count-overflow -Wanalyzer-stale-setjmp-buffer -Wanalyzer-unsafe-call-within-signal-handler +-Wanalyzer-unterminated-string -Wanalyzer-use-after-free -Wanalyzer-use-of-pointer-in-stale-stack-frame -Wanalyzer-use-of-uninitialized-value @@ -10907,6 +10909,17 @@ called from a signal handler. See @uref{https://cwe.mitre.org/data/definitions/479.html, CWE-479: Signal Handler Use of a Non-reentrant Function}. +@opindex Wanalyzer-unterminated-string +@opindex Wno-analyzer-unterminated-string +@item -Wno-analyzer-unterminated-string +This warning requires @option{-fanalyzer}, which enables it; use +@option{-Wno-analyzer-unterminated-string} to disable it. + +This diagnostic warns about code paths which attempt to find the length +of an unterminated string. For example, passing a pointer to an unterminated +buffer to @code{strlen} would lead to accesses beyond the end of the buffer +whilst attempting to find the terminating zero character. + @opindex Wanalyzer-use-after-free @opindex Wno-analyzer-use-after-free @item -Wno-analyzer-use-after-free diff --git a/gcc/testsuite/gcc.dg/analyzer/analyzer-decls.h b/gcc/testsuite/gcc.dg/analyzer/analyzer-decls.h index d9a32ed9370..a6267289462 100644 --- a/gcc/testsuite/gcc.dg/analyzer/analyzer-decls.h +++ b/gcc/testsuite/gcc.dg/analyzer/analyzer-decls.h @@ -53,4 +53,9 @@ extern void __analyzer_eval (int); /* Obtain an "unknown" void *. */ extern void *__analyzer_get_unknown_ptr (void); +/* Complain if PTR doesn't point to a null-terminated string. + TODO: eventually get the strlen of the buffer (without the + optimizer touching it). */ +extern __SIZE_TYPE__ __analyzer_get_strlen (const char *ptr); + #endif /* #ifndef ANALYZER_DECLS_H. */ diff --git a/gcc/testsuite/gcc.dg/analyzer/error-1.c b/gcc/testsuite/gcc.dg/analyzer/error-1.c index f82a4cdd3c0..491d615e2cb 100644 --- a/gcc/testsuite/gcc.dg/analyzer/error-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/error-1.c @@ -64,3 +64,15 @@ void test_6 (int st, const char *str) error (st, errno, "test: %s", str); __analyzer_eval (st == 0); /* { dg-warning "TRUE" } */ } + +char *test_error_unterminated (int st) +{ + char fmt[3] = "abc"; + error (st, errno, fmt); /* { dg-warning "passing pointer to unterminated string '&fmt' as argument 3 of 'error'" } */ +} + +char *test_error_at_line_unterminated (int st, int errno) +{ + char fmt[3] = "abc"; + error_at_line (st, errno, __FILE__, __LINE__, fmt); /* { dg-warning "passing pointer to unterminated string '&fmt' as argument 5 of 'error_at_line'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c new file mode 100644 index 00000000000..33798706823 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/null-terminated-strings-1.c @@ -0,0 +1,30 @@ +#include "analyzer-decls.h" + +#define NULL ((void *)0) +typedef __SIZE_TYPE__ size_t; + +void test_terminated (void) +{ + __analyzer_get_strlen ("abc"); /* { dg-bogus "" } */ +} + +void test_unterminated (void) +{ + char buf[3] = "abc"; + __analyzer_get_strlen (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of '__analyzer_get_strlen'" } */ +} + +void test_embedded_nul (void) +{ + char buf[3] = "a\0c"; + __analyzer_get_strlen (buf); /* { dg-bogus "" } */ +} + +void test_fully_initialized_but_unterminated (void) +{ + char buf[3]; + buf[0] = 'a'; + buf[1] = 'b'; + buf[2] = 'c'; + __analyzer_get_strlen (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of '__analyzer_get_strlen'" "" { xfail *-*-* } } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/putenv-1.c b/gcc/testsuite/gcc.dg/analyzer/putenv-1.c index 543121258c8..5fa20334c0a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/putenv-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/putenv-1.c @@ -108,3 +108,10 @@ void test_outer (void) char arr_outer[] = "NAME=VALUE"; /* { dg-message "'arr_outer' declared on stack here" } */ __analyzer_test_inner (arr_outer); } + +void test_unterminated (void) +{ + char buf[3] = "abc"; + putenv (buf); /* { dg-warning "passing pointer to unterminated string" } */ + /* { dg-warning "'putenv' on a pointer to automatic variable 'buf'" "POS34-C" { target *-*-* } .-1 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strchr-1.c b/gcc/testsuite/gcc.dg/analyzer/strchr-1.c index bfa48916ca2..2fb6c76797e 100644 --- a/gcc/testsuite/gcc.dg/analyzer/strchr-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/strchr-1.c @@ -25,3 +25,9 @@ void test_3 (const char *s, int c) char *p = strchr (s, c); /* { dg-message "when 'strchr' returns NULL"} */ *p = 'A'; /* { dg-warning "dereference of NULL 'p'" "null deref" } */ } + +void test_unterminated (int c) +{ + char buf[3] = "abc"; + strchr (buf, c); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of 'strchr'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c index ed5bab98e4b..f23dd69bfb6 100644 --- a/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c @@ -16,3 +16,9 @@ test_1a (char *dst, char *src) __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ return result; } + +char *test_unterminated (char *dst) +{ + char buf[3] = "abc"; + return strcpy (dst, buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 2 of 'strcpy'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strdup-1.c b/gcc/testsuite/gcc.dg/analyzer/strdup-1.c index 9ac3921af21..682bfb90176 100644 --- a/gcc/testsuite/gcc.dg/analyzer/strdup-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/strdup-1.c @@ -38,3 +38,9 @@ void test_6 (const char *s) char *p = __builtin_strdup (s); /* { dg-message "this call could return NULL" } */ requires_nonnull (p); /* { dg-warning "use of possibly-NULL 'p'" } */ } + +char *test_unterminated (void) +{ + char buf[3] = "abc"; + return strdup (buf); /* { dg-warning "passing pointer to unterminated string '&buf' as argument 1 of 'strdup'" } */ +}