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.129.124]) by sourceware.org (Postfix) with ESMTPS id 713B23839C6A for ; Wed, 25 May 2022 09:32:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 713B23839C6A 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-351-AK0PGxH1NheJZTY5U94Zlw-1; Wed, 25 May 2022 05:32:36 -0400 X-MC-Unique: AK0PGxH1NheJZTY5U94Zlw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2BFF7811E7A; Wed, 25 May 2022 09:32:36 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.106]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DA5B6112131B; Wed, 25 May 2022 09:32:35 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 24P9WXfm030495 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 25 May 2022 11:32:33 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 24P9WWE7030494; Wed, 25 May 2022 11:32:32 +0200 Date: Wed, 25 May 2022 11:32:32 +0200 From: Jakub Jelinek To: Richard Biener Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] asan: Fix up instrumentation of assignments which are both loads and stores [PR105714] Message-ID: Reply-To: Jakub Jelinek MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 May 2022 09:32:39 -0000 Hi! On the following testcase with -Os asan pass sees: [local count: 354334800]: # h_21 = PHI *c.3_5 = *d.2_4; h_15 = h_21 + 1; if (h_15 != 3) goto ; [75.00%] else goto ; [25.00%] [local count: 118111600]: *c.3_5 = MEM[(struct a *)&b + 12B]; _13 = c.3_5->x; return _13; It instruments the *c.3_5 = *d.2_4; assignment by adding .ASAN_CHECK (7, c.3_5, 4, 4); .ASAN_CHECK (6, d.2_4, 4, 4); before it (which later lowers to checking the corresponding shadow memory). But when considering instrumentation of *c.3_5 = MEM[(struct a *)&b + 12B]; it doesn't instrument anything, because it sees that *c.3_5 store is already instrumented in a dominating block and so there is no need to instrument *c.3_5 store again (i.e. add another .ASAN_CHECK (7, c.3_5, 4, 4); ). That is true, but misses the fact that we still want to instrument the MEM[(struct a *)&b + 12B] load. The following patch fixes that by changing has_stmt_been_instrumented_p to consider both store and load in the assignment if it does both (returning true iff both have been instrumented). That matches how we handle e.g. builtin calls, where we also perform AND of all the memory locs involved in the call. I've verified that we still don't add the redundant .ASAN_CHECK (7, c.3_5, 4, 4); call but just add _18 = &MEM[(struct a *)&b + 12B]; .ASAN_CHECK (6, _18, 4, 4); to instrument the load. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2022-05-25 Jakub Jelinek PR sanitizer/105714 * asan.cc (has_stmt_been_instrumented_p): For assignments which are both stores and loads, return true only if both destination and source have been instrumented. * gcc.dg/asan/pr105714.c: New test. --- gcc/asan.cc.jj 2022-05-12 08:27:56.923834018 +0200 +++ gcc/asan.cc 2022-05-24 11:39:28.527258357 +0200 @@ -1285,7 +1285,20 @@ has_stmt_been_instrumented_p (gimple *st if (get_mem_ref_of_assignment (as_a (stmt), &r, &r_is_store)) - return has_mem_ref_been_instrumented (&r); + { + if (!has_mem_ref_been_instrumented (&r)) + return false; + if (r_is_store && gimple_assign_load_p (stmt)) + { + asan_mem_ref src; + asan_mem_ref_init (&src, NULL, 1); + src.start = gimple_assign_rhs1 (stmt); + src.access_size = int_size_in_bytes (TREE_TYPE (src.start)); + if (!has_mem_ref_been_instrumented (&src)) + return false; + } + return true; + } } else if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)) { --- gcc/testsuite/gcc.dg/asan/pr105714.c.jj 2022-05-24 11:50:26.753570348 +0200 +++ gcc/testsuite/gcc.dg/asan/pr105714.c 2022-05-24 11:51:01.074225766 +0200 @@ -0,0 +1,33 @@ +/* PR sanitizer/105714 */ +/* { dg-do run } */ +/* { dg-skip-if "" { *-*-* } { "*" } { "-Os" } } */ +/* { dg-shouldfail "asan" } */ + +struct A { int x; }; +struct A b[2]; +struct A *c = b, *d = b; +int e; + +int +foo () +{ + for (e = 0; e < 1; e++) + { + int i[1]; + i; + } + for (int h = 0; h < 3; h++) + *c = *d; + *c = *(b + 3); + return c->x; +} + +int +main () +{ + foo (); + return 0; +} + +/* { dg-output "ERROR: AddressSanitizer: global-buffer-overflow on address.*(\n|\r\n|\r)" } */ +/* { dg-output "READ of size.*" } */ Jakub