Re: [PATCH v7 06/10] tracing/probes: Type casting always involves nested calls

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

Sashiko commented:

> > +	}
> > +	*close = '\0';
> >  
> > -		ctx->offset += 1;	/* for the '(' */
> > -		/* We need to parse the nested one */
> > -		ret = parse_probe_arg(tmp + 1, find_fetch_type(NULL, ctx->flags),
> > -				pcode, end, ctx);
> > -		if (ret < 0)
> > -			return ret;
> > -		ctx->nested_level--;
> > -		clear_struct_btf(ctx);
> > +	/* We need to parse the nested one */
> > +	ret = parse_probe_arg(tmp, find_fetch_type(NULL, ctx->flags),
> > +			      pcode, end, ctx);
> 
> Does routing the nested argument through parse_probe_arg() instead of 
> parse_btf_arg() break the documented eprobe syntax (STRUCT)FIELD->MEMBER?
> Since this patch removes the TPARG_FL_TEVENT handling block from
> parse_btf_arg(), and handle_typecast() now recursively invokes 
> parse_probe_arg() on the extracted FIELD name, a bare eprobe FIELD name 
> that does not start with a '$' prefix will hit the default case in 
> parse_probe_arg().
> This causes it to be unconditionally rejected with -EINVAL (NOSUP_BTFARG) 
> because eprobe flags (TPARG_FL_TEVENT) do not satisfy 
> tparg_is_function_entry() or tparg_is_function_return(). This acts as an 
> ABI breakage for existing user-space scripts relying on this eprobe syntax.

Wait... Did the type-casting support patch accidentally allow access to
event fields without the "$"?

Hmm, if so, it should be documented, and need to support correctly
with/without typecast.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>