Message ID | Pine.LNX.4.64.1103302249580.19760@wotan.suse.de |
---|---|
State | New |
Headers | show |
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); >
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);