This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Don't instrument DECL_INITIAL of statics (PR sanitizer/66190)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 29 May 2015 12:26:39 +0200
- Subject: Re: [PATCH] Don't instrument DECL_INITIAL of statics (PR sanitizer/66190)
- Authentication-results: sourceware.org; auth=none
- References: <20150521193658 dot GQ27320 at redhat dot com> <20150529084134 dot GG27320 at redhat dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Fri, May 29, 2015 at 10:41:34AM +0200, Marek Polacek wrote:
> Ping.
>
> On Thu, May 21, 2015 at 09:36:59PM +0200, Marek Polacek wrote:
> > In this PR, we find ourselves instrumenting a static initializer and
> > then crashing when expanding an unlowered UBSAN_NULL. Jakub suggests
> > to not instrument DECL_INITIAL of a static variable. The following
> > patch is an attempt to do that. Note that we're still able to sanitize
> > similar cases (they don't have DECL_INITIAL but something else).
> >
> > Bootstrap/regtest/bootstrap-ubsan passed on x86_64-linux, ok for trunk?
> >
> > 2015-05-21 Marek Polacek <polacek@redhat.com>
> >
> > PR sanitizer/66190
> > * cp-gimplify.c (struct cp_genericize_data): Add no_sanitize_p.
> > (cp_genericize_r): Don't instrument static initializers.
> > (cp_genericize_tree): Initialize wtd.no_sanitize_p.
This seems strange. Normally DECL_INITIAL of vars isn't walked when
processing DECL_EXPRs, so IMHO you shouldn't either.
I think it would be much better to handle this case where the tree.c
code handles it, thus in cp_genericize_r's BIND_EXPR handling.
Just do there something along the lines:
if (flag_sanitize
& (SANITIZE_NULL | SANITIZE_ALIGNMENT | SANITIZE_VPTR))
{
bool no_sanitize_p = wtd->no_sanitize_p;
wtd->no_sanitize_p = true;
for (tree decl = BIND_EXPR_VARS (*tp); decl; decl = DECL_CHAIN (decl))
if (VAR_P (decl)
&& TREE_STATIC (decl)
&& DECL_INITIAL (decl))
cp_walk_tree (&DECL_INITIAL (decl), cp_genericize_r, data, NULL);
wtd->no_sanitize_p = no_sanitize_p;
}
with some appripriate comments. As cp_genericize_r gives up early for
expressions it has walked already, this should DTRT then.
Jakub