On Wed, Mar 30, 2011 at 11:17 PM, Michael Matz <m...@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);
>
