Fix pass49-frag (mudflap support for varargs)

Fix pass49-frag (mudflap support for varargs)

[Michael Matz]

Hi,

for some reasons the FAIL of pass49-frag now annoyed me enough to look 
into it, where it didn't do so for the last 50 years it's failing (give or 
take a few).  mudflap has a mean to mark some expressions as not 
interesting to generate checks for, which is used to disable this for the 
indirect refs that are generated for the varargs accesses (naturally these 
can't be checked because nobody registers these stack slots).  That is 
done via build_va_arg_indirect_ref calling mf_mark on the generated 
indirect_ref.

Now, our gimplifier changes this indirect_ref into a mem_ref, making the 
marking (via a hash-table marked_trees in tree-mudflap.c) obsolete.  
Instead of amending the gimplifier to copy the mark whenever changing a 
marked tree, I chose to instead simply generate a memref right from the 
start, which isn't gimplified further, hence the mark stays okay.

Yes, that's somewhat fragile, but so is the whole mudflap code generation.  
And in the abstract it seems better to generate more natural gimple code 
to begin with (of course the operand still is gimplified when necessary, 
but that doesn't destroy the mark as it's done in place).

Why pass49-frag also failed before mem-ref I don't know.  I don't want to 
know.  I'd speculate that it's similar reasons.  Or maybe it wasn't 
failing and I confuse it with some other pass4x-frag that was failing for 
years where nobody cared.

In any case, patch fixes this mudflap testcase and for x86_64-linux that 
was the last failing one.  Regstrapping on x86_64-linux in progress.  Okay 
for trunk?


Ciao,
Michael.
PS: Btw, that varargs at least for x86_64 didn't work with mudflap makes 
me somewhat suspicious about the existence of people using -fmudflap.  
Perhaps we should just remove the whole mudflap code for 4.7.
-- 
	* builtins.c (build_va_arg_indirect_ref): Use
	build_simple_mem_ref_loc.

Index: builtins.c
===================================================================
--- builtins.c	(revision 171675)
+++ builtins.c	(working copy)
@@ -4748,7 +4748,7 @@ std_gimplify_va_arg_expr (tree valist, t
 tree
 build_va_arg_indirect_ref (tree addr)
 {
-  addr = build_fold_indirect_ref_loc (EXPR_LOCATION (addr), addr);
+  addr = build_simple_mem_ref_loc (EXPR_LOCATION (addr), addr);
 
   if (flag_mudflap) /* Don't instrument va_arg INDIRECT_REF.  */
     mf_mark (addr);

Re: Fix pass49-frag (mudflap support for varargs)

[Richard Guenther]

On Wed, Mar 30, 2011 at 11:17 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> for some reasons the FAIL of pass49-frag now annoyed me enough to look
> into it, where it didn't do so for the last 50 years it's failing (give or
> take a few).  mudflap has a mean to mark some expressions as not
> interesting to generate checks for, which is used to disable this for the
> indirect refs that are generated for the varargs accesses (naturally these
> can't be checked because nobody registers these stack slots).  That is
> done via build_va_arg_indirect_ref calling mf_mark on the generated
> indirect_ref.
>
> Now, our gimplifier changes this indirect_ref into a mem_ref, making the
> marking (via a hash-table marked_trees in tree-mudflap.c) obsolete.
> Instead of amending the gimplifier to copy the mark whenever changing a
> marked tree, I chose to instead simply generate a memref right from the
> start, which isn't gimplified further, hence the mark stays okay.
>
> Yes, that's somewhat fragile, but so is the whole mudflap code generation.
> And in the abstract it seems better to generate more natural gimple code
> to begin with (of course the operand still is gimplified when necessary,
> but that doesn't destroy the mark as it's done in place).
>
> Why pass49-frag also failed before mem-ref I don't know.  I don't want to
> know.  I'd speculate that it's similar reasons.  Or maybe it wasn't
> failing and I confuse it with some other pass4x-frag that was failing for
> years where nobody cared.
>
> In any case, patch fixes this mudflap testcase and for x86_64-linux that
> was the last failing one.  Regstrapping on x86_64-linux in progress.  Okay
> for trunk?

Ok.

Thanks,
Richard.

>
> Ciao,
> Michael.
> PS: Btw, that varargs at least for x86_64 didn't work with mudflap makes
> me somewhat suspicious about the existence of people using -fmudflap.
> Perhaps we should just remove the whole mudflap code for 4.7.

Count me in ;)

(it surely needs a rewrite)

Richard.

> --
>        * builtins.c (build_va_arg_indirect_ref): Use
>        build_simple_mem_ref_loc.
>
> Index: builtins.c
> ===================================================================
> --- builtins.c  (revision 171675)
> +++ builtins.c  (working copy)
> @@ -4748,7 +4748,7 @@ std_gimplify_va_arg_expr (tree valist, t
>  tree
>  build_va_arg_indirect_ref (tree addr)
>  {
> -  addr = build_fold_indirect_ref_loc (EXPR_LOCATION (addr), addr);
> +  addr = build_simple_mem_ref_loc (EXPR_LOCATION (addr), addr);
>
>   if (flag_mudflap) /* Don't instrument va_arg INDIRECT_REF.  */
>     mf_mark (addr);
>