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 [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id E194B385379E for ; Fri, 9 Dec 2022 02:28:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E194B385379E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1670552936; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=hzLDVHmQfGZgKw5wTf+6bGtnCqGq2rDSga5Wy2LtdaA=; b=UN60+/sjv+1eGvNY3unB+3hGsucPBQbeHtU8nuUUNj84A1RIkQ3Iab7/02b5mxvNqZzvWK Oq2frbe9h4Dp2oHP41m2UNPcrF47zlFEMoh7R9PvqejCJPaoBq5RoCS48hMH+WDRN5vjNp UKwxfg1n2/ZX0yQ9Zb7tV6bZ+vyoeuw= 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-630-xuJygxpCM6u3bGvRodlKsA-1; Thu, 08 Dec 2022 21:28:53 -0500 X-MC-Unique: xuJygxpCM6u3bGvRodlKsA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E2A12801E7C; Fri, 9 Dec 2022 02:28:52 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id A4748492B04; Fri, 9 Dec 2022 02:28:52 +0000 (UTC) From: David Malcolm To: Alexandre Oliva Cc: gcc-patches@gcc.gnu.org, David Malcolm Subject: [committed] analyzer: handle memmove like memcpy Date: Thu, 8 Dec 2022 21:28:45 -0500 Message-Id: <20221209022845.32233-1-dmalcolm@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.5 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 List-Id: On Thu, 2022-12-08 at 07:36 -0300, Alexandre Oliva wrote: > Hello again, David, > > On Dec 2, 2022, David Malcolm wrote: > > > I had a go at porting your patch to trunk; here's the result. > > Oh, wow, nice! Thank you so much. > > I confirm it works on riscv64-elf too. Thanks. When I ran the full test suite (on x86_64) it turned out that the added check_for_poison in memcpy was correctly flagging some uses of uninitialized source buffers in the testsuite. So I added modified the patch further, adding dg-warning directives in a few places, and adding some more test coverage of memcpy/memmove from uninit buffers. Here's what I've pushed to trunk (as r13-4577-gcf80a23e19db83); successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Hopefully this doesn't introduce any further failures on riscv. Dave gcc/analyzer/ChangeLog: * region-model-impl-calls.cc (class kf_memcpy): Rename to... (class kf_memcpy_memmove): ...this. (kf_memcpy::impl_call_pre): Rename to... (kf_memcpy_memmove::impl_call_pre): ...this, and check the src for poison. (register_known_functions): Update for above renaming, and register BUILT_IN_MEMMOVE and BUILT_IN_MEMMOVE_CHK. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/memcpy-1.c (test_8a, test_8b): New tests. * gcc.dg/analyzer/memmove-1.c: New test, based on memcpy-1.c * gcc.dg/analyzer/out-of-bounds-1.c (test7): Update expected result for uninit srcBuf. * gcc.dg/analyzer/out-of-bounds-5.c (test8, test9): Add dg-warnings for memcpy from uninit src vla. * gcc.dg/analyzer/pr104308.c (test_memmove_within_uninit): Expect creation point note to be missing on riscv*-*-*. Signed-off-by: David Malcolm --- gcc/analyzer/region-model-impl-calls.cc | 18 +- gcc/testsuite/gcc.dg/analyzer/memcpy-1.c | 14 ++ gcc/testsuite/gcc.dg/analyzer/memmove-1.c | 182 ++++++++++++++++++ .../gcc.dg/analyzer/out-of-bounds-1.c | 2 +- .../gcc.dg/analyzer/out-of-bounds-5.c | 2 + gcc/testsuite/gcc.dg/analyzer/pr104308.c | 2 +- 6 files changed, 212 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/memmove-1.c diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index 6aeb9281bff..ff2f1b1ef9c 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -246,10 +246,12 @@ kf_malloc::impl_call_pre (const call_details &cd) const } } -/* Handler for "memcpy" and "__builtin_memcpy". */ -// TODO: complain about overlapping src and dest. +/* Handler for "memcpy" and "__builtin_memcpy", + "memmove", and "__builtin_memmove". */ +/* TODO: complain about overlapping src and dest for the memcpy + variants. */ -class kf_memcpy : public known_function +class kf_memcpy_memmove : public known_function { public: bool matches_call_types_p (const call_details &cd) const final override @@ -263,7 +265,7 @@ public: }; void -kf_memcpy::impl_call_pre (const call_details &cd) const +kf_memcpy_memmove::impl_call_pre (const call_details &cd) const { const svalue *dest_ptr_sval = cd.get_arg_svalue (0); const svalue *src_ptr_sval = cd.get_arg_svalue (1); @@ -285,6 +287,8 @@ kf_memcpy::impl_call_pre (const call_details &cd) const = mgr->get_sized_region (dest_reg, NULL_TREE, num_bytes_sval); const svalue *src_contents_sval = model->get_store_value (sized_src_reg, cd.get_ctxt ()); + model->check_for_poison (src_contents_sval, cd.get_arg_tree (1), + cd.get_ctxt ()); model->set_value (sized_dest_reg, src_contents_sval, cd.get_ctxt ()); } @@ -927,8 +931,10 @@ register_known_functions (known_function_manager &kfm) kfm.add (BUILT_IN_EXPECT_WITH_PROBABILITY, make_unique ()); kfm.add (BUILT_IN_FREE, make_unique ()); kfm.add (BUILT_IN_MALLOC, make_unique ()); - kfm.add (BUILT_IN_MEMCPY, make_unique ()); - kfm.add (BUILT_IN_MEMCPY_CHK, make_unique ()); + kfm.add (BUILT_IN_MEMCPY, make_unique ()); + kfm.add (BUILT_IN_MEMCPY_CHK, make_unique ()); + kfm.add (BUILT_IN_MEMMOVE, make_unique ()); + kfm.add (BUILT_IN_MEMMOVE_CHK, make_unique ()); kfm.add (BUILT_IN_MEMSET, make_unique ()); kfm.add (BUILT_IN_MEMSET_CHK, make_unique ()); kfm.add (BUILT_IN_REALLOC, make_unique ()); diff --git a/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c index a9368d3307d..b1ffed0a979 100644 --- a/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/memcpy-1.c @@ -166,3 +166,17 @@ void test_7b (void *src, size_t sz) { memcpy ((void *)"hello world", src, sz); /* { dg-warning "write to string literal" } */ } + +/* memcpy from uninitialized buffer. */ + +void test_8a (void *dst) +{ + char src[16]; + memcpy (dst, src, 16); /* { dg-warning "use of uninitialized value" } */ +} + +void test_8b (void *dst, size_t n) +{ + char src[16]; + memcpy (dst, src, n); /* { dg-warning "use of uninitialized value" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/memmove-1.c b/gcc/testsuite/gcc.dg/analyzer/memmove-1.c new file mode 100644 index 00000000000..06627ede081 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/memmove-1.c @@ -0,0 +1,182 @@ +#include +#include "analyzer-decls.h" + +/* Function for thwarting expansion of memmove by optimizer. */ + +typedef void * (*memmove_t) (void *dst, const void *src, size_t n); + +static memmove_t __attribute__((noinline)) +get_memmove (void) +{ + return memmove; +} + +void *test_1 (void *dst, void *src, size_t n) +{ + void *result = memmove (dst, src, n); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void *test_1a (void *dst, void *src, size_t n) +{ + void *result = __memmove_chk (dst, src, n, -1); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void *test_1b (void *dst, void *src, size_t n) +{ + memmove_t fn = get_memmove (); + void *result = fn (dst, src, n); + __analyzer_eval (result == dst); /* { dg-warning "TRUE" } */ + return result; +} + +void test_2 (int i) +{ + int j; + memmove (&j, &i, sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_2a (int i) +{ + int j; + __memmove_chk (&j, &i, sizeof (int), sizeof (int)); + __analyzer_eval (i == j); /* { dg-warning "TRUE" } */ +} + +void test_2b (int i) +{ + int j; + memmove_t fn = get_memmove (); + fn (&j, &i, 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" } */ + + memmove (buf, src, n); + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ +} + +void test_3b (void *src, size_t n) +{ + char buf[40], other[40]; + memmove_t fn = get_memmove (); + buf[0] = 'a'; + other[0] = 'b'; + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "TRUE" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ + + fn (buf, src, n); + __analyzer_eval (buf[0] == 'a'); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (other[0] == 'b'); /* { dg-warning "TRUE" } */ +} + +/* Overwriting a zeroed buffer, then memmove of the result. */ + +void test_4 (int a, int b) +{ + int src[1024]; + int dst[1024]; + memset (src, 0, sizeof (src)); + src[42] = a; + src[100] = b; + __analyzer_eval (src[0] == 0); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[42] == a); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[100] == b); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[1023] == 0); /* { dg-warning "TRUE" } */ + + memmove (dst, src, sizeof (src)); + __analyzer_eval (dst[0] == 0); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[42] == a); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[100] == b); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[1023] == 0); /* { dg-warning "TRUE" } */ +} + +void test_4b (int a, int b) +{ + int src[1024]; + int dst[1024]; + memmove_t fn = get_memmove (); + memset (src, 0, sizeof (src)); + src[42] = a; + src[100] = b; + __analyzer_eval (src[0] == 0); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[42] == a); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[100] == b); /* { dg-warning "TRUE" } */ + __analyzer_eval (src[1023] == 0); /* { dg-warning "TRUE" } */ + + fn (dst, src, sizeof (src)); + __analyzer_eval (dst[0] == 0); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[42] == a); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[100] == b); /* { dg-warning "TRUE" } */ + __analyzer_eval (dst[1023] == 0); /* { dg-warning "TRUE" } */ +} + +/* Populating a buffer from an unknown buffer. */ + +void test_5 (void *src, size_t sz) +{ + char dst[1024]; + memmove (dst, src, sizeof (dst)); + __analyzer_eval (dst[0] == 0); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (dst[1023] == 0); /* { dg-warning "UNKNOWN" } */ +} + +void test_5b (void *src, size_t sz) +{ + char dst[1024]; + memmove_t fn = get_memmove (); + fn (dst, src, sizeof (dst)); + __analyzer_eval (dst[0] == 0); /* { dg-warning "UNKNOWN" } */ + __analyzer_eval (dst[1023] == 0); /* { dg-warning "UNKNOWN" } */ +} + +/* Zero-sized memmove. */ + +void test_6 (void *dst, void *src) +{ + memmove (dst, src, 0); +} + +void test_6b (void *dst, void *src) +{ + memmove_t fn = get_memmove (); + fn (dst, src, 0); +} + +/* memmove to string literal. */ + +void test_7 (void *src, size_t sz) +{ + memmove ((void *)"hello world", src, sz); /* { dg-warning "write to string literal" } */ +} + +void test_7b (void *src, size_t sz) +{ + memmove ((void *)"hello world", src, sz); /* { dg-warning "write to string literal" } */ +} + +/* memcpy from uninitialized buffer. */ + +void test_8a (void *dst) +{ + char src[16]; + memmove (dst, src, 16); /* { dg-warning "use of uninitialized value" } */ +} + +void test_8b (void *dst, size_t n) +{ + char src[16]; + memmove (dst, src, n); /* { dg-warning "use of uninitialized value" } */ +} 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 977476ed2fb..93b379c173a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-1.c @@ -117,6 +117,6 @@ void test7 (void) // TODO: Should we handle widening_svalues as a follow-up? /* { dg-warning "over-read" "warning" { xfail *-*-* } test7 } */ + /* { dg-warning "use of uninitialized value" "uninit warning" { target *-*-* } test7 } */ /* { dg-warning "overflow" "warning" { xfail *-*-* } test7 } */ - /* { dg-message "" "note" { xfail *-*-* } test7 } */ } 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 52fea79152e..eb6aae0f8cb 100644 --- a/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c +++ b/gcc/testsuite/gcc.dg/analyzer/out-of-bounds-5.c @@ -69,6 +69,7 @@ void test8 (size_t size, size_t offset) char dst[size]; memcpy (dst, src, size + offset); /* { dg-line test8 } */ /* { dg-warning "over-read" "warning" { target *-*-* } test8 } */ + /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } test8 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test8 } */ } @@ -78,6 +79,7 @@ void test9 (size_t size, size_t offset) int32_t dst[size]; memcpy (dst, src, 4 * size + 1); /* { dg-line test9 } */ /* { dg-warning "over-read" "warning" { target *-*-* } test9 } */ + /* { dg-warning "use of uninitialized value" "warning" { target *-*-* } test9 } */ /* { dg-warning "overflow" "warning" { target *-*-* } test9 } */ } diff --git a/gcc/testsuite/gcc.dg/analyzer/pr104308.c b/gcc/testsuite/gcc.dg/analyzer/pr104308.c index a3a0cbb7317..e6a2c8821bf 100644 --- a/gcc/testsuite/gcc.dg/analyzer/pr104308.c +++ b/gcc/testsuite/gcc.dg/analyzer/pr104308.c @@ -6,7 +6,7 @@ int test_memmove_within_uninit (void) { - char s[5]; /* { dg-message "region created on stack here" } */ + char s[5]; /* { dg-message "region created on stack here" "" { xfail riscv*-*-* } } */ memmove(s, s + 1, 2); /* { dg-warning "use of uninitialized value" } */ return 0; } -- 2.26.3