public inbox for gcc-bugs@sourceware.org help / color / mirror / Atom feed
From: "cvs-commit at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org> To: gcc-bugs@gcc.gnu.org Subject: [Bug sanitizer/112709] [13/14 Regression] address sanitize and returns_twice causes an ICE Date: Tue, 12 Mar 2024 10:35:59 +0000 [thread overview] Message-ID: <bug-112709-4-cRxWt75Lhf@http.gcc.gnu.org/bugzilla/> (raw) In-Reply-To: <bug-112709-4@http.gcc.gnu.org/bugzilla/> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112709 --- Comment #7 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>: https://gcc.gnu.org/g:ad860cc27b3312f9119c7fecb8638a7c1f6d77c9 commit r14-9438-gad860cc27b3312f9119c7fecb8638a7c1f6d77c9 Author: Jakub Jelinek <jakub@redhat.com> Date: Tue Mar 12 11:34:50 2024 +0100 asan: Instrument <retval> stores in callees rather than callers [PR112709] asan currently instruments since PR69276 r6-6758 fix calls which store the return value into memory on the caller side, before the call it verifies the memory is writable. Now PR112709 where we ICE on trying to instrument such calls made me think about whether that is what we want to do. There are 3 different cases. One is when a function returns an aggregate which is passed e.g. in registers, say like struct S { int a[4]; }; returning on x86_64. That would be ideally instrumented in between the actual call and storing of the aggregate into memory, but asan currently mostly works as a GIMPLE pass and arranging for the instrumentation to happen at that spot would be really hard. We could diagnose after the call but generally asan attempts to diagnose stuff before something is overwritten rather than after, or keep the current behavior (that is what this patch does, which has the disadvantage that it can complain about UB even for functions which never return and so never actually store, and doesn't check whether the memory wasn't e.g. poisoned during the call) or could e.g. instrument both before and after the call (that would have the disadvantage the current state has but at least would check post-factum the store again afterwards). Another case is when a function returns an aggregate through a hidden reference, struct T { int a[128]; }; on x86_64 or even the above struct S on ia32 as example. In the actual program such stores happen when storing something to <retval> or its parts in the callee, because <retval> there expands to *hidden_retval. So, IMHO we should instrument those in the callee rather than caller, that is where the writes are and we can do that easily. This is what the patch below does. And the last case is for builtins/internal functions. Usually those don't return aggregates, but in case they'd do and can be expanded inline, it is better to instrument them in the caller (as before) rather than not instrumenting the return stores at all. I had to tweak the expected output on the PR69276 testcase, because with the patch it keeps previous behavior on x86_64 (structure returned in registers, stored in the caller, so reported as UB in A::A()), while on i686 it changed the behavior and is reported as UB in the vnull::operator vec which stores the structure, A::A() is then a frame above it in the backtrace. 2024-03-12 Jakub Jelinek <jakub@redhat.com> PR sanitizer/112709 * asan.cc (has_stmt_been_instrumented_p): Don't instrument call stores on the caller side unless it is a call to a builtin or internal function or function doesn't return by hidden reference. (maybe_instrument_call): Likewise. (instrument_derefs): Instrument stores to RESULT_DECL if returning by hidden reference. * gcc.dg/asan/pr112709-1.c: New test. * g++.dg/asan/pr69276.C: Adjust expected output for some targets.
next prev parent reply other threads:[~2024-03-12 10:36 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-11-25 5:43 [Bug tree-optimization/112709] New: ICE verify_flow_info failed during GIMPLE pass: asan0 iamanonymous.cs at gmail dot com 2023-11-25 19:53 ` [Bug sanitizer/112709] [13/14 Regression] address sanitize and returns_twice causes an ICE pinskia at gcc dot gnu.org 2023-11-27 12:46 ` jakub at gcc dot gnu.org 2023-11-27 13:00 ` jakub at gcc dot gnu.org 2024-03-07 21:01 ` law at gcc dot gnu.org 2024-03-08 17:02 ` jakub at gcc dot gnu.org 2024-03-08 17:19 ` jakub at gcc dot gnu.org 2024-03-08 17:20 ` jakub at gcc dot gnu.org 2024-03-12 10:35 ` cvs-commit at gcc dot gnu.org [this message] 2024-03-12 13:22 ` jakub at gcc dot gnu.org 2024-03-13 8:18 ` cvs-commit at gcc dot gnu.org 2024-03-13 8:20 ` cvs-commit at gcc dot gnu.org 2024-03-15 14:15 ` [Bug sanitizer/112709] [13 " jakub at gcc dot gnu.org 2024-03-15 23:29 ` cvs-commit at gcc dot gnu.org 2024-03-15 23:29 ` cvs-commit at gcc dot gnu.org 2024-03-18 14:42 ` jakub at gcc dot gnu.org
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bug-112709-4-cRxWt75Lhf@http.gcc.gnu.org/bugzilla/ \ --to=gcc-bugzilla@gcc.gnu.org \ --cc=gcc-bugs@gcc.gnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).