From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id D39DE3985469 for ; Fri, 11 Sep 2020 01:10:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org D39DE3985469 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-19-rAb8H7_POOK7LUhPeoP1UQ-1; Thu, 10 Sep 2020 21:10:53 -0400 X-MC-Unique: rAb8H7_POOK7LUhPeoP1UQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 673F410ABDA2 for ; Fri, 11 Sep 2020 01:10:52 +0000 (UTC) Received: from t470.redhat.com (ovpn-114-91.phx2.redhat.com [10.3.114.91]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0A9E019D6C; Fri, 11 Sep 2020 01:10:51 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Subject: [committed] analyzer: stricter handling of non-pure builtins [PR96798] Date: Thu, 10 Sep 2020 21:10:50 -0400 Message-Id: <20200911011050.30045-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0.001 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.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_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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-List-Received-Date: Fri, 11 Sep 2020 01:10:58 -0000 Amongst other things PR analyzer/96798 notes that region_model::on_call_pre treats any builtin that hasn't been coded yet as a no-op (albeit with an unknown return value), which is wrong for non-pure builtins. This patch updates that function's handling of such builtins so that it instead conservatively assumes that any escaped/reachable regions can be affected by the call, and implements enough handling of specific builtins to avoid regressing the testsuite (I hope). Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to master as r11-3130-gb7028f060c6760b336b416897412e327ded12ab5. gcc/analyzer/ChangeLog: PR analyzer/96798 * region-model-impl-calls.cc (region_model::impl_call_memcpy): New. (region_model::impl_call_strcpy): New. * region-model.cc (region_model::on_call_pre): Flag unhandled builtins that are non-pure as having unknown side-effects. Implement BUILT_IN_MEMCPY, BUILT_IN_MEMCPY_CHK, BUILT_IN_STRCPY, BUILT_IN_STRCPY_CHK, BUILT_IN_FPRINTF, BUILT_IN_FPRINTF_UNLOCKED, BUILT_IN_PUTC, BUILT_IN_PUTC_UNLOCKED, BUILT_IN_FPUTC, BUILT_IN_FPUTC_UNLOCKED, BUILT_IN_FPUTS, BUILT_IN_FPUTS_UNLOCKED, BUILT_IN_FWRITE, BUILT_IN_FWRITE_UNLOCKED, BUILT_IN_PRINTF, BUILT_IN_PRINTF_UNLOCKED, BUILT_IN_PUTCHAR, BUILT_IN_PUTCHAR_UNLOCKED, BUILT_IN_PUTS, BUILT_IN_PUTS_UNLOCKED, BUILT_IN_VFPRINTF, BUILT_IN_VPRINTF. * region-model.h (region_model::impl_call_memcpy): New decl. (region_model::impl_call_strcpy): New decl. gcc/testsuite/ChangeLog: PR analyzer/96798 * gcc.dg/analyzer/memcpy-1.c: New test. * gcc.dg/analyzer/strcpy-1.c: New test. --- gcc/analyzer/region-model-impl-calls.cc | 39 +++++++++++++++++++++ gcc/analyzer/region-model.cc | 36 ++++++++++++++++++++ gcc/analyzer/region-model.h | 2 ++ gcc/testsuite/gcc.dg/analyzer/memcpy-1.c | 43 ++++++++++++++++++++++++ gcc/testsuite/gcc.dg/analyzer/strcpy-1.c | 18 ++++++++++ 5 files changed, 138 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/analyzer/memcpy-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/strcpy-1.c diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index fa85035b22d..6582ffb3c95 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -276,6 +276,30 @@ region_model::impl_call_malloc (const call_details &cd) return true; } +/* Handle the on_call_pre part of "memcpy" and "__builtin_memcpy". */ + +void +region_model::impl_call_memcpy (const call_details &cd) +{ + const svalue *dest_sval = cd.get_arg_svalue (0); + const svalue *num_bytes_sval = cd.get_arg_svalue (2); + + const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0), + cd.get_ctxt ()); + + cd.maybe_set_lhs (dest_sval); + + if (tree num_bytes = num_bytes_sval->maybe_get_constant ()) + { + /* "memcpy" of zero size is a no-op. */ + if (zerop (num_bytes)) + return; + } + + /* Otherwise, mark region's contents as unknown. */ + mark_region_as_unknown (dest_reg); +} + /* Handle the on_call_pre part of "memset" and "__builtin_memset". */ bool @@ -353,6 +377,21 @@ region_model::impl_call_operator_delete (const call_details &cd) return false; } +/* Handle the on_call_pre part of "strcpy" and "__builtin_strcpy_chk". */ + +void +region_model::impl_call_strcpy (const call_details &cd) +{ + const svalue *dest_sval = cd.get_arg_svalue (0); + const region *dest_reg = deref_rvalue (dest_sval, cd.get_arg_tree (0), + cd.get_ctxt ()); + + cd.maybe_set_lhs (dest_sval); + + /* For now, just mark region's contents as unknown. */ + mark_region_as_unknown (dest_reg); +} + /* Handle the on_call_pre part of "strlen". Return true if the LHS is updated. */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 03c7daf0d1e..75f4eae3083 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -658,6 +658,8 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt) switch (DECL_UNCHECKED_FUNCTION_CODE (callee_fndecl)) { default: + if (!DECL_PURE_P (callee_fndecl)) + unknown_side_effects = true; break; case BUILT_IN_ALLOCA: case BUILT_IN_ALLOCA_WITH_ALIGN: @@ -672,20 +674,54 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt) break; case BUILT_IN_MALLOC: return impl_call_malloc (cd); + case BUILT_IN_MEMCPY: + case BUILT_IN_MEMCPY_CHK: + impl_call_memcpy (cd); + return false; case BUILT_IN_MEMSET: case BUILT_IN_MEMSET_CHK: impl_call_memset (cd); return false; break; + case BUILT_IN_STRCPY: + case BUILT_IN_STRCPY_CHK: + impl_call_strcpy (cd); + return false; case BUILT_IN_STRLEN: if (impl_call_strlen (cd)) return false; break; + + /* Stdio builtins. */ + case BUILT_IN_FPRINTF: + case BUILT_IN_FPRINTF_UNLOCKED: + case BUILT_IN_PUTC: + case BUILT_IN_PUTC_UNLOCKED: + case BUILT_IN_FPUTC: + case BUILT_IN_FPUTC_UNLOCKED: + case BUILT_IN_FPUTS: + case BUILT_IN_FPUTS_UNLOCKED: + case BUILT_IN_FWRITE: + case BUILT_IN_FWRITE_UNLOCKED: + case BUILT_IN_PRINTF: + case BUILT_IN_PRINTF_UNLOCKED: + case BUILT_IN_PUTCHAR: + case BUILT_IN_PUTCHAR_UNLOCKED: + case BUILT_IN_PUTS: + case BUILT_IN_PUTS_UNLOCKED: + case BUILT_IN_VFPRINTF: + case BUILT_IN_VPRINTF: + /* These stdio builtins have external effects that are out + of scope for the analyzer: we only want to model the effects + on the return value. */ + break; } else if (gimple_call_internal_p (call)) switch (gimple_call_internal_fn (call)) { default: + if (!DECL_PURE_P (callee_fndecl)) + unknown_side_effects = true; break; case IFN_BUILTIN_EXPECT: return impl_call_builtin_expect (cd); diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 4707293175a..1bb9798ae58 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -2554,7 +2554,9 @@ class region_model bool impl_call_calloc (const call_details &cd); void impl_call_free (const call_details &cd); bool impl_call_malloc (const call_details &cd); + void impl_call_memcpy (const call_details &cd); bool impl_call_memset (const call_details &cd); + void impl_call_strcpy (const call_details &cd); bool impl_call_strlen (const call_details &cd); bool impl_call_operator_new (const call_details &cd); bool impl_call_operator_delete (const call_details &cd); diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c new file mode 100644 index 00000000000..f120eac19b3 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c @@ -0,0 +1,43 @@ +#include +#include "analyzer-decls.h" + +void *test_1 (void *dst, void *src, size_t n) +{ + void *result = memcpy (dst, src, n); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void *test_1a (void *dst, void *src, size_t n) +{ + void *result = __memcpy_chk (dst, src, n, -1); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void test_2 (int i) +{ + int j; + memcpy (&j, &i, sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_2a (int i) +{ + int j; + __memcpy_chk (&j, &i, sizeof (int), sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_3 (void *src, size_t n) +{ + char buf[40], other[40]; + buf[0] = 'a'; + other[0] = 'b'; + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "TRUE" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ + + memcpy (buf, src, n); + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c new file mode 100644 index 00000000000..ed5bab98e4b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/strcpy-1.c @@ -0,0 +1,18 @@ +#include +#include "analyzer-decls.h" + +char * +test_1 (char *dst, char *src) +{ + char *result = strcpy (dst, src); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +char * +test_1a (char *dst, char *src) +{ + char *result = __strcpy_chk (dst, src, -1); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} -- 2.26.2