Re: [PATCH v4] Introduce strub: machine-independent stack scrubbing

[Richard Biener <richard.guenther@gmail.com>]

On Thu, Nov 30, 2023 at 6:04 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> On Nov 29, 2023, Richard Biener <richard.guenther@gmail.com> wrote:
>
> > On Wed, Nov 29, 2023 at 9:53 AM Alexandre Oliva <oliva@adacore.com> wrote:
>
> >> Because &arg_#(D)[n_#] is good gimple, but &(*byref_arg_#(D))[n_#] isn't.
>
> > 'arg_#(D)' looks like a SSA name, and no, taking the address doesn't work,
> > so I assume it was &MEM[arg_(D)][n_#] which is indeed OK.  But you
> > shouldn't need to change a pointer argument to be passed by reference,
> > do you?  As said, you want to restrict by-reference passing to arguments
> > that are !is_gimple_reg_type ().  Everywhere where a plain PARM_DECL
> > was valid a *ptr indirection is as well.
>
> > Can you check on the pass-by-reference thing again please?
>
> Applying the following patchlet on top of refs/users/aoliva/heads/strub
> (that has a -fstrub=all patchlet in it) I get a build error in libgo
> building golang.org/x/mod/sumdb.o:
>
> In function ‘golang_0org_1x_1mod_1sumdb.Client.checkTrees.strub.0’:
> go1: error: invalid argument to gimple call
> &older_195(D)->Hash
> # VUSE <.MEM_55>
> _16 = __builtin_memcmp (&h, &older_195(D)->Hash, 32);
> during IPA pass: strub
>
> golang.org/x/mod/sumdb.go.057i.remove_symbols:  _5 = __builtin_memcmp (&h, &older.Hash, 32);

Ah, yeah - &older.Hash is considered a constant while &older_195(D)->Hash isn't.

So indeed you are correct - you'll need adjustments, sorry for
misleading you ...

Richard.

> within golang_0org_1x_1mod_1sumdb.Client.checkTrees becomes, in wrapped
> version thereof:
>
> golang.org/x/mod/sumdb.go.058i.strub:  _16 = __builtin_memcmp (&h, &older_195(D)->Hash, 32);
>
> It's not even the case that Hash is at offset 0 into older's type, but I
> suspect the reason why the former is well-formed while the latter is not
> the offset, but the SSA_NAME.
>
>
> diff --git a/gcc/ipa-strub.cc b/gcc/ipa-strub.cc
> index 293bec132b8..515ab9a2ee5 100644
> --- a/gcc/ipa-strub.cc
> +++ b/gcc/ipa-strub.cc
> @@ -1950,7 +1950,7 @@ walk_regimplify_addr_expr (tree *op, int *rec, void *arg)
>    if (!*op || TREE_CODE (*op) != ADDR_EXPR)
>      return NULL_TREE;
>
> -  if (!is_gimple_val (*op))
> +  if (0 && !is_gimple_val (*op))
>      {
>        tree ret = force_gimple_operand_gsi (&gsi, *op, true,
>                                            NULL_TREE, true, GSI_SAME_STMT);
>
> --
> Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
>    Free Software Activist                   GNU Toolchain Engineer
> More tolerance and less prejudice are key for inclusion and diversity
> Excluding neuro-others for not behaving ""normal"" is *not* inclusive