From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2153) id A7839383B783; Mon, 30 May 2022 03:36:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A7839383B783 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jakub Jelinek To: gcc-cvs@gcc.gnu.org Subject: [gcc r12-8433] asan: Fix up instrumentation of assignments which are both loads and stores [PR105714] X-Act-Checkin: gcc X-Git-Author: Jakub Jelinek X-Git-Refname: refs/heads/releases/gcc-12 X-Git-Oldrev: 7b88f9b8881b7653b5a834da1b2b8fda463e61c0 X-Git-Newrev: 120d99a3ec3f13766e13b40e48b0614242447fea Message-Id: <20220530033648.A7839383B783@sourceware.org> Date: Mon, 30 May 2022 03:36:48 +0000 (GMT) X-BeenThere: gcc-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 May 2022 03:36:48 -0000 https://gcc.gnu.org/g:120d99a3ec3f13766e13b40e48b0614242447fea commit r12-8433-g120d99a3ec3f13766e13b40e48b0614242447fea Author: Jakub Jelinek Date: Wed May 25 12:05:08 2022 +0200 asan: Fix up instrumentation of assignments which are both loads and stores [PR105714] 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. 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. (cherry picked from commit af02daff557a0abbf5521c143f1cdda406848a9b) Diff: --- gcc/asan.cc | 15 ++++++++++++++- gcc/testsuite/gcc.dg/asan/pr105714.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/gcc/asan.cc b/gcc/asan.cc index ef59b77ebc2..bacf890af18 100644 --- a/gcc/asan.cc +++ b/gcc/asan.cc @@ -1285,7 +1285,20 @@ has_stmt_been_instrumented_p (gimple *stmt) 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)) { diff --git a/gcc/testsuite/gcc.dg/asan/pr105714.c b/gcc/testsuite/gcc.dg/asan/pr105714.c new file mode 100644 index 00000000000..d378b8ab0c4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/asan/pr105714.c @@ -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.*" } */