From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52d.google.com (mail-pg1-x52d.google.com [IPv6:2607:f8b0:4864:20::52d]) by sourceware.org (Postfix) with ESMTPS id 7A0E23858D28 for ; Thu, 26 Jan 2023 01:49:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7A0E23858D28 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pg1-x52d.google.com with SMTP id 78so217628pgb.8 for ; Wed, 25 Jan 2023 17:49:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=sEZ35Xtt4JYYejXTXcV59IivH3XtNzGPU3vh7YlA6J0=; b=Xzc95WYKpMnxxdWI6pFtq3jKFjAt89rgueSIwgMiFOiWGQd6FgClRH0TgwqadSlbx8 TOXN9+H3XjsTsrpV/ci6iSFmaSzfzrBK4a0WPnVXpGJf0Cax4V2D8XfChGAmy8CrZFCs ygjExtam9sBinDYHSUx7fukyv0Cr5lwksbVHYXAKFNvvnwVxlyH4wd+rzK+rcEfddHmJ OOWQqTxrhnffuTYaNaPf9g2NdEKovBU8brP5u7ErD+a6H3foXY8Ls2wsVk7GEFBdq2nV 3fvlk2wmBtcJETr1Ufp7jLb8Ejgxtd8mOPFFVMl5bnMl1T8ZF6xPUdKB4lJTDU0J4U49 p+XA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=sEZ35Xtt4JYYejXTXcV59IivH3XtNzGPU3vh7YlA6J0=; b=k8VBCQSZlIcUq8BbHhzK1ahSDlceOXUmvHWbqrQXMbavK/XgZKWhZ/meaznp96JD8U EDuMgJsgUuEVOypzNTeIf45R6CAxRLnzdk5XtEykcZg1OINaG5MFr9P6fpUc1oRALja9 rtdtkKPGrT2bqoryHBsUwZItEN6RcdyUu6Jcu+CLJRAa0/yduXvyUIDXhJhKYZvKRbYo 6kjE/qDzU6RqVjeCFqy1u+E/HHv5Ut4QbwHqHiRELreTncVGDcPg+dmNBJe3VzG/XvFG uxIkh7sTquCc56x9eOrSHxs5oGiI7MvVBg0PLMQPmYWvo/UufMHF+vkJHLv9KR37SxeE Pbug== X-Gm-Message-State: AFqh2kr/Uw9Rp13lUVwCT1FoqVcKrZPmFT3GP17XBlXKVkHd+XgGsWbj RgG+tmF63LIPUQA4qSOeU9otUgzS5pOlNn2Pnyo= X-Google-Smtp-Source: AMrXdXustaR9J5Xj5mbbcNWloNUGzHYhyE1QCgkVn4Tbr3IoMK4C5RVcmQ6664Wy2Xp4KhSDFprCGWuxlbHD3EI2EAA= X-Received: by 2002:a62:86c2:0:b0:58d:9d50:67bd with SMTP id x185-20020a6286c2000000b0058d9d5067bdmr4259363pfd.20.1674697773345; Wed, 25 Jan 2023 17:49:33 -0800 (PST) MIME-Version: 1.0 References: <20230125232545.312399-1-polacek@redhat.com> In-Reply-To: <20230125232545.312399-1-polacek@redhat.com> From: Andrew Pinski Date: Wed, 25 Jan 2023 17:49:20 -0800 Message-ID: Subject: Re: [PATCH] opts: SANITIZE_ADDRESS wrongly cleared [PR108543] To: Marek Polacek Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-7.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,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 Wed, Jan 25, 2023 at 3:26 PM Marek Polacek via Gcc-patches wrote: > > Here we crash on a null fndecl ultimately because we haven't defined > the built-ins described in sanitizer.def. So > builtin_decl_explicit (BUILT_IN_ASAN_POINTER_SUBTRACT); > returns NULL_TREE, causing an ICE later. > > DEF_SANITIZER_BUILTIN only actually defines the built-ins when flag_sanitize > has SANITIZE_ADDRESS, or some of the other SANITIZE_*, but it doesn't check > SANITIZE_KERNEL_ADDRESS or SANITIZE_USER_ADDRESS. Unfortunately, with > -fsanitize=address -fno-sanitize=kernel-address > or > -fsanitize=kernel-address -fno-sanitize=address > SANITIZE_ADDRESS ends up being unset from flag_sanitize even though > _USER/_KERNEL are set. That's because -fsanitize=address means > SANITIZE_ADDRESS | SANITIZE_USER_ADDRESS and -fsanitize=kernel-address > is SANITIZE_ADDRESS | SANITIZE_KERNEL_ADDRESS but parse_sanitizer_options > does > flags &= ~sanitizer_opts[i].flag; > so the subsequent -fno- unsets SANITIZE_ADDRESS. Then no sanitizer > built-ins are actually defined. > > I'm not sure why SANITIZE_ADDRESS isn't just SANITIZE_USER_ADDRESS | > SANITIZE_KERNEL_ADDRESS, I don't think we need 3 bits. I am trying to follow the code but I can't tell if -fsanitize=address -fno-sanitize=address still will work? It would make sense to add a testcase for that case too. I suspect it does not work though, because it would still leave SANITIZE_ADDRESS enabled ... Thanks, Andrew > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/12/11? > > PR middle-end/108543 > > gcc/ChangeLog: > > * opts.cc (parse_sanitizer_options): Don't clear SANITIZE_ADDRESS if it > was previously set. > > gcc/testsuite/ChangeLog: > > * c-c++-common/asan/pointer-subtract-5.c: New test. > * c-c++-common/asan/pointer-subtract-6.c: New test. > * c-c++-common/asan/pointer-subtract-7.c: New test. > * c-c++-common/asan/pointer-subtract-8.c: New test. > --- > gcc/opts.cc | 5 ++++- > .../c-c++-common/asan/pointer-subtract-5.c | 15 +++++++++++++++ > .../c-c++-common/asan/pointer-subtract-6.c | 15 +++++++++++++++ > .../c-c++-common/asan/pointer-subtract-7.c | 15 +++++++++++++++ > .../c-c++-common/asan/pointer-subtract-8.c | 15 +++++++++++++++ > 5 files changed, 64 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c > create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c > create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c > create mode 100644 gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c > > diff --git a/gcc/opts.cc b/gcc/opts.cc > index 9ba47d7deaa..c042b03bd9f 100644 > --- a/gcc/opts.cc > +++ b/gcc/opts.cc > @@ -2246,7 +2246,10 @@ parse_sanitizer_options (const char *p, location_t loc, int scode, > flags |= sanitizer_opts[i].flag; > } > else > - flags &= ~sanitizer_opts[i].flag; > + /* Don't clear SANITIZE_ADDRESS if it was previously set; > + -fsanitize=address -fno-sanitize=kernel-address should > + leave SANITIZE_ADDRESS set. */ > + flags &= (~sanitizer_opts[i].flag | SANITIZE_ADDRESS); > found = true; > break; > } > diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c > new file mode 100644 > index 00000000000..867eda0e61e > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-5.c > @@ -0,0 +1,15 @@ > +/* PR middle-end/108543 */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=address -fno-sanitize=kernel-address -fsanitize=pointer-subtract" } */ > + > +struct S { > + long _M_p; > +}; > + > +typedef struct S S; > + > +__PTRDIFF_TYPE__ > +f (S __x, S __y) > +{ > + return &__x._M_p - &__y._M_p; > +} > diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c > new file mode 100644 > index 00000000000..785b90b3d98 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-6.c > @@ -0,0 +1,15 @@ > +/* PR middle-end/108543 */ > +/* { dg-do compile } */ > +/* { dg-options "-fsanitize=kernel-address -fno-sanitize=address -fsanitize=pointer-subtract" } */ > + > +struct S { > + long _M_p; > +}; > + > +typedef struct S S; > + > +__PTRDIFF_TYPE__ > +f (S __x, S __y) > +{ > + return &__x._M_p - &__y._M_p; > +} > diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c > new file mode 100644 > index 00000000000..11b63401b8c > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-7.c > @@ -0,0 +1,15 @@ > +/* PR middle-end/108543 */ > +/* { dg-do compile } */ > +/* { dg-options "-fno-sanitize=kernel-address -fsanitize=address -fsanitize=pointer-subtract" } */ > + > +struct S { > + long _M_p; > +}; > + > +typedef struct S S; > + > +__PTRDIFF_TYPE__ > +f (S __x, S __y) > +{ > + return &__x._M_p - &__y._M_p; > +} > diff --git a/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c b/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c > new file mode 100644 > index 00000000000..ac2b9c3c1d9 > --- /dev/null > +++ b/gcc/testsuite/c-c++-common/asan/pointer-subtract-8.c > @@ -0,0 +1,15 @@ > +/* PR middle-end/108543 */ > +/* { dg-do compile } */ > +/* { dg-options "-fno-sanitize=address -fsanitize=kernel-address -fsanitize=pointer-subtract" } */ > + > +struct S { > + long _M_p; > +}; > + > +typedef struct S S; > + > +__PTRDIFF_TYPE__ > +f (S __x, S __y) > +{ > + return &__x._M_p - &__y._M_p; > +} > > base-commit: 9fb9da3d38513d320bfea72050f7a59688595e0b > -- > 2.39.1 >