Message ID | 20240131033759.2236614-1-jason@redhat.com |
---|---|
State | Accepted |
Headers |
Return-Path: <gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:2087:b0:106:209c:c626 with SMTP id gs7csp1651420dyb; Tue, 30 Jan 2024 19:38:54 -0800 (PST) X-Google-Smtp-Source: AGHT+IEcE3ZpYsX9fk2C3Bx5TMSfFnPEG6dAZ3Nxsgy65aMedbjKsC9kMbbnkT9MEF0mW0sixw6x X-Received: by 2002:a05:6214:e4e:b0:686:2b63:16a4 with SMTP id o14-20020a0562140e4e00b006862b6316a4mr522981qvc.48.1706672334554; Tue, 30 Jan 2024 19:38:54 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1706672334; cv=pass; d=google.com; s=arc-20160816; b=q0VFFbRurBf8VSwg5RxGgGTOLg59NUaIAPQC5eSQxwasZCcNtHU99b1C+XFN0GO+3r qmBZXr3tiD9Wp246E1A4llHtjEn/B97lWxZTPUf+GViOfARIvuMUUmTYnkwyWMwcL0pG 3bIW1inta02MeDokIY7y7pl3HPm4yS8vMczTh0RAJetJJph9ihMzt5BeQdG7RY7//jvx vg3MWnNaiK/Sn6tr4s7A8IRNCnI9repq5QZpiintG9OsdoiFFoIIFp14Xrxvw6vGq7Os xHr0tqjJt0lsfhUM97W0jfhb1AZNM59OJ5lpdhmZFoZlJRSNby0+gZQ1J5yGGze4jB2a VHHg== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :arc-filter:dmarc-filter:delivered-to; bh=8QwVqCroVVpzHekINVFxucqygBRAkl6d435o22lm3O0=; fh=icyMVknwVzwe5fGxphHiUgnWqkxEV1uqj56u7ATjt8E=; b=NFexBZNHSXKgMQNkREEaT69LmoCYjSNi5DOvaE3sPeWSZHOrmabcxhtORDrTJB0dbJ 3xeY0Ui4lHVlFSRWRi4jrTXuwHW5uenDDDD6SeEm98GM3itj1L271vPPMEVRzukbQoJ7 KZrO8aoye9l6BY7LrKfoH3OTuGujNo/eEH3cwO87Wa/AOeT1w1Z7fBSrpyytIcyQkhCs xXZvDZ+O0k9gLxwCRAiwIfg8k7iDbx066JS/3oWel3ERz6SIiJ74xamCgGZUYCcKuEC5 6nZKktUCgwI+HT29VMRaEO4QdjmFc5hRZpnuExBm9zV4axr1qrIF9WPYnIjg1vk/z0r4 3GMA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fjHI4ptQ; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com X-Forwarded-Encrypted: i=1; AJvYcCWyCS2YFO8emB6WYnX3mzfHUXyJSySrXe4KZ1PCb4ztZtdeV8DOYV4Pb+EQNkEOsO7kh/ODeYOgWzJ9KtH3Fs4boNwp8A== Received: from server2.sourceware.org (server2.sourceware.org. [2620:52:3:1:0:246e:9693:128c]) by mx.google.com with ESMTPS id gf2-20020a056214250200b0067f9b223bfbsi6563053qvb.294.2024.01.30.19.38.54 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jan 2024 19:38:54 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) client-ip=2620:52:3:1:0:246e:9693:128c; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=fjHI4ptQ; arc=pass (i=1); spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 2620:52:3:1:0:246e:9693:128c as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4332D38582A5 for <ouuuleilei@gmail.com>; Wed, 31 Jan 2024 03:38:54 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org 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 464DA3858D1E for <gcc-patches@gcc.gnu.org>; Wed, 31 Jan 2024 03:38:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 464DA3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 464DA3858D1E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706672289; cv=none; b=dQNGBaScz9i8ZcwUiFltxO+alYGKcm7Np0YpiprCCSYSxcqEJ1norAL/hdeNTHpCqDZF2v+k1UCgiBTg9RGobFyNdn+QRB6ayFJqgM0pUdObOwwT6i1kMW7ZFWjlOBX5oi8ufWNt4+g7ZTFsvp0DA9K9Z5jQW/qzDVXzuadl6bg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1706672289; c=relaxed/simple; bh=TRiensm6KEOzEGjk0c76ts2MdVmx2ZdR2vkBynMCmoQ=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=B56H7E4et7G7RpWDVZ3TLF5dnpsIaMKVzPqTuDappjzsxN0mJ0XSLeNNE6yyTDVKnnc47jLhsZaiMGE8ZIKz28Lk0AsKWUP2O82pf1HVAeR6ziW7DxXXZmmaGBx6Fs6UuTs1iGowHKoiCjkJafbHfLtStn6fdVUdsDdRk1ls3es= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1706672287; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=8QwVqCroVVpzHekINVFxucqygBRAkl6d435o22lm3O0=; b=fjHI4ptQfeqQHu86GQMTwE5Fu4BoUXFys3ExaRGVjaIcCIJi6Y5UsKAcx+ce2zh4T+b9ri zkeGSd87Xw8WHkkLMRPVFaiwrfZlBuhKgufQ8reHi4pJZlH5JuHbq2YxT+UvRQcSUeBc4m hZoMSLnj7+H5i3Bs5LYoOBbZuSRG2us= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-253-Lquu8V9HNV-tn50qykOZXA-1; Tue, 30 Jan 2024 22:38:03 -0500 X-MC-Unique: Lquu8V9HNV-tn50qykOZXA-1 Received: by mail-qk1-f200.google.com with SMTP id af79cd13be357-7853c364e60so17397685a.2 for <gcc-patches@gcc.gnu.org>; Tue, 30 Jan 2024 19:38:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1706672283; x=1707277083; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=8QwVqCroVVpzHekINVFxucqygBRAkl6d435o22lm3O0=; b=SAjCStKM+9cJPhqF4Eqm7KjQ2teQ2sYqjPZQHITyWw/pR2FlJ9ZuewuLaGKUC7CsKa /o7Qm+37sltrL7xz66b6wjANWneUYtQfHHuAKWEDc3HBdy5HDsCn8IYvhgabYalUR6Cb lit3dBxk7VshcQwgI4i1Iu981g9B0Bf1wL7NVfk3jyXNdDBI/ZADc4vXADokHJG6zUsO XwRNsaBtjit/bNnxCHM+/e2Keolg5mBoL4JAAv/bW+tNd1JDIc943chWUQN2gI81/Lgi eCHyVoxmFG2i2lkY5H+/LQbO4F08GaOu0LCBfyrBf8XXipR+CWO8xdjHl/uVZrDyfI6U RvpA== X-Gm-Message-State: AOJu0YxW1GQo24ZgTIEUw86cEQ98QIliSPCbZILwHFHWTH4QmqYPypXf nzlVATJDeIew8pwvIAjIqhAGGdaN5YANRfhPrOOADZ1dpZce873DbMCg9XOEPrmwbhmTP3i6Ov0 /Xd0GgK75cgG2jPNcbXoBevd9KEsv3G5fs3Gnq7xe8roszKbDqJSlBdrtJj/K+5wXQ5/+l99KxW wLDWLcSAUyx3+qA5IFSOopwoXjLxjcb/Z4VMg= X-Received: by 2002:a05:620a:558c:b0:783:fbc1:9c2a with SMTP id vq12-20020a05620a558c00b00783fbc19c2amr362129qkn.29.1706672282657; Tue, 30 Jan 2024 19:38:02 -0800 (PST) X-Received: by 2002:a05:620a:558c:b0:783:fbc1:9c2a with SMTP id vq12-20020a05620a558c00b00783fbc19c2amr362107qkn.29.1706672282065; Tue, 30 Jan 2024 19:38:02 -0800 (PST) Received: from jason.com (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id g25-20020a37e219000000b007836647671fsm2870353qki.89.2024.01.30.19.38.01 for <gcc-patches@gcc.gnu.org> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 30 Jan 2024 19:38:01 -0800 (PST) From: Jason Merrill <jason@redhat.com> To: gcc-patches@gcc.gnu.org Subject: [PATCH RFA] asan: poisoning promoted statics [PR113531] Date: Tue, 30 Jan 2024 22:37:59 -0500 Message-Id: <20240131033759.2236614-1-jason@redhat.com> X-Mailer: git-send-email 2.39.3 MIME-Version: 1.0 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=-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_H4, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1789575649894531393 X-GMAIL-MSGID: 1789575649894531393 |
Series |
[RFA] asan: poisoning promoted statics [PR113531]
|
|
Checks
Context | Check | Description |
---|---|---|
snail/gcc-patch-check | success | Github commit url |
Commit Message
Jason Merrill
Jan. 31, 2024, 3:37 a.m. UTC
Tested x86_64-pc-linux-gnu, OK for trunk? -- 8< -- Since my r14-1500-g4d935f52b0d5c0 we promote an initializer_list backing array to static storage where appropriate, but this happens after we decided to add it to asan_poisoned_variables. As a result we add unpoison/poison for it to the gimple. But then sanopt removes the unpoison. So the second time we call the function and want to load from the array asan still considers it poisoned. A simple fix seems to be to not expand unpoison/poison for such a variable, since by that time we know it's static. PR c++/113531 gcc/ChangeLog: * asan.cc (asan_expand_mark_ifn): Check TREE_STATIC. gcc/testsuite/ChangeLog: * g++.dg/asan/initlist1.C: New test. --- gcc/asan.cc | 8 ++++++++ gcc/testsuite/g++.dg/asan/initlist1.C | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 gcc/testsuite/g++.dg/asan/initlist1.C base-commit: 209fc1e5f6c67e55e579b69f617b0b678b1bfdf0
Comments
On Wed, Jan 31, 2024 at 4:38 AM Jason Merrill <jason@redhat.com> wrote: > > Tested x86_64-pc-linux-gnu, OK for trunk? It's a quite "late" fixup, I suppose you have tried to avoid marking it during gimplification? I see we do parts of this during BIND_EXPR processing which is indeed a bit early but possibly difficult to rectify. So, OK if you think fixing during gimplification is overly messy. Richard. > -- 8< -- > > Since my r14-1500-g4d935f52b0d5c0 we promote an initializer_list backing > array to static storage where appropriate, but this happens after we decided > to add it to asan_poisoned_variables. As a result we add unpoison/poison > for it to the gimple. But then sanopt removes the unpoison. So the second > time we call the function and want to load from the array asan still > considers it poisoned. > > A simple fix seems to be to not expand unpoison/poison for such a variable, > since by that time we know it's static. > > PR c++/113531 > > gcc/ChangeLog: > > * asan.cc (asan_expand_mark_ifn): Check TREE_STATIC. > > gcc/testsuite/ChangeLog: > > * g++.dg/asan/initlist1.C: New test. > --- > gcc/asan.cc | 8 ++++++++ > gcc/testsuite/g++.dg/asan/initlist1.C | 20 ++++++++++++++++++++ > 2 files changed, 28 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/asan/initlist1.C > > diff --git a/gcc/asan.cc b/gcc/asan.cc > index 0fd7dd1f3ed..efecac2ea2b 100644 > --- a/gcc/asan.cc > +++ b/gcc/asan.cc > @@ -3762,6 +3762,14 @@ asan_expand_mark_ifn (gimple_stmt_iterator *iter) > > gcc_checking_assert (TREE_CODE (decl) == VAR_DECL); > > + if (TREE_STATIC (decl)) > + { > + /* Don't poison a variable with static storage; it might have gotten > + marked before gimplify_init_constructor promoted it to static. */ > + gsi_remove (iter, true); > + return false; > + } > + > if (hwasan_sanitize_p ()) > { > gcc_assert (param_hwasan_instrument_stack); > diff --git a/gcc/testsuite/g++.dg/asan/initlist1.C b/gcc/testsuite/g++.dg/asan/initlist1.C > new file mode 100644 > index 00000000000..6cd5b7d3aba > --- /dev/null > +++ b/gcc/testsuite/g++.dg/asan/initlist1.C > @@ -0,0 +1,20 @@ > +// PR c++/113531 > +// { dg-do run { target c++11 } } > +// { dg-additional-options "-fsanitize=address" } > + > +#include <initializer_list> > + > +void f(int) { } > + > +void g() > +{ > + for (auto i : { 1, 2, 3 }) > + f (i); > + f(42); > +} > + > +int main() > +{ > + g(); > + g(); > +} > > base-commit: 209fc1e5f6c67e55e579b69f617b0b678b1bfdf0 > -- > 2.39.3 >
On Wed, Jan 31, 2024 at 09:51:05AM +0100, Richard Biener wrote: > On Wed, Jan 31, 2024 at 4:38 AM Jason Merrill <jason@redhat.com> wrote: > > > > Tested x86_64-pc-linux-gnu, OK for trunk? > > It's a quite "late" fixup, I suppose you have tried to avoid marking it > during gimplification? I see we do parts of this during BIND_EXPR > processing which is indeed a bit early but possibly difficult to rectify. Indeed. But what we could do is try to fold_stmt those .ASAN_MARK calls away earlier (but sure, the asan.cc change would be still required because that would be just an optimization). But that can be handled incrementally, so I think the patch is ok as is (and I can handle the incremental part myself). Note, the handling of global vars in asan is done only at the end (asan_finish_file), so I think such late TREE_STATIC marked vars will still be correctly treated as global vars if varpool knows about them (and if varpool doesn't, then lots of other things would break). Jakub
On 1/31/24 03:51, Richard Biener wrote: > On Wed, Jan 31, 2024 at 4:38 AM Jason Merrill <jason@redhat.com> wrote: >> >> Tested x86_64-pc-linux-gnu, OK for trunk? > > It's a quite "late" fixup, I suppose you have tried to avoid marking it > during gimplification? I see we do parts of this during BIND_EXPR > processing which is indeed a bit early but possibly difficult to rectify. I also considered > diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc > index 7f79b3cc7e6..c906d927a09 100644 > --- a/gcc/gimplify.cc > +++ b/gcc/gimplify.cc > @@ -1249,6 +1249,10 @@ asan_poison_variable (tree decl, bool poison, gimple_stmt_iterator *it, > if (zerop (unit_size)) > return; > > + /* Or variables in static storage. */ > + if (TREE_STATIC (decl)) > + return; > + > /* It's necessary to have all stack variables aligned to ASAN granularity > bytes. */ > gcc_assert (!hwasan_sanitize_p () || hwasan_sanitize_stack_p ()); which fixes the bug by avoiding the poison mark, but it's too late to avoid the unpoison mark--though the unpoison is still removed by sanopt, so the end result is the same. I decided to send the other patch because it applies to both, but I'm happy with either approach. Jason
diff --git a/gcc/asan.cc b/gcc/asan.cc index 0fd7dd1f3ed..efecac2ea2b 100644 --- a/gcc/asan.cc +++ b/gcc/asan.cc @@ -3762,6 +3762,14 @@ asan_expand_mark_ifn (gimple_stmt_iterator *iter) gcc_checking_assert (TREE_CODE (decl) == VAR_DECL); + if (TREE_STATIC (decl)) + { + /* Don't poison a variable with static storage; it might have gotten + marked before gimplify_init_constructor promoted it to static. */ + gsi_remove (iter, true); + return false; + } + if (hwasan_sanitize_p ()) { gcc_assert (param_hwasan_instrument_stack); diff --git a/gcc/testsuite/g++.dg/asan/initlist1.C b/gcc/testsuite/g++.dg/asan/initlist1.C new file mode 100644 index 00000000000..6cd5b7d3aba --- /dev/null +++ b/gcc/testsuite/g++.dg/asan/initlist1.C @@ -0,0 +1,20 @@ +// PR c++/113531 +// { dg-do run { target c++11 } } +// { dg-additional-options "-fsanitize=address" } + +#include <initializer_list> + +void f(int) { } + +void g() +{ + for (auto i : { 1, 2, 3 }) + f (i); + f(42); +} + +int main() +{ + g(); + g(); +}