Re: [PATCH v2 4/6] Use remote register numbers in tracepoint mask

Pedro Franco de Carvalho
2018-08-03T22:09:53Z

Ulrich Weigand <uweigand@de.ibm.com> writes:

> The alternative I guess would be to have collect_symbol fall back to
> the "treat_as_expr" case for collecting pseudo registers; and then
> possibly implementing as optimization a check whether the agent
> expression is actually empty except for the register mask ...
> Something similar would then also have to be done in encode_actions_1.
>
> Do you think this would be doable?

I tried doing something like this but in add_local_register (this way
even if add_memranges uses a pseudoregister for some reason, it should
also work). See the next message for the updated patch.

I just check if ax_regs_mask adds anything to the expression other than
the mask, and add the expression to the collection_list.

I'm not sure that this is the best approach, would it make sense to
create a full expression, and generate an ax from that?

This means that if a pseudo-register does something more complicated
like reading memory, the ax it generates through
gdbarch_ax_pseudo_register_collect should include the trace bytecodes
for the memory it needs.

I don't think this was the original intent, and it isn't clear that the
rest of the ax code was made to work with that, but I tested this with a
pseudoreg that reads the backchain from r1 (in powerpc), and it works.

Alternatively, I can just raise an error in add_local_register if the
length of the ax after ax_regs_mask is non-zero.

I also refactored the lines that check if an ax is valid into a
function, which is now also called from add_local_register too, since a
full ax might be added.

Previously only validate_actionline checked that the expression was not
too long, now every place that generates an ax does this as well.

Thanks for the review!

--
Pedro Franco de Carvalho