linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Douglas Raillard <douglas.raillard@arm.com>
Cc: linux-trace-devel@vger.kernel.org,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Takaya Saeki <takayas@google.com>,
	Ian Rogers <irogers@google.com>,
	aahringo@redhat.com
Subject: Re: [PATCH 1/2] libtraceevent: Add loading of BTF to the tep handle
Date: Mon, 4 Aug 2025 08:43:22 -0400	[thread overview]
Message-ID: <20250804084322.15b8c321@gandalf.local.home> (raw)
In-Reply-To: <e74be108-7ca5-466e-a89a-512a80a11beb@arm.com>

On Mon, 4 Aug 2025 12:47:31 +0100
Douglas Raillard <douglas.raillard@arm.com> wrote:

> > +int tep_btf_print_args(struct tep_handle *tep, struct trace_seq *s, void *args,
> > +		       int nmem, int size, const char *func)
> > +{
> > +	struct tep_btf *btf = tep->btf;
> > +	struct btf_type *type = tep_btf_find_func(btf, func);
> > +	struct btf_param *param;
> > +	unsigned long long arg;
> > +	unsigned int encode;
> > +	const char *param_name;
> > +	int a, p, x, nr;
> > +
> > +	if (size != 4 && size != 8)
> > +		return -1;
> > +
> > +	if (!type) {
> > +		for (int i = 0; i < nmem; i++) {
> > +			assign_arg(&arg, args, size, i);
> > +			trace_seq_printf(s, "%llx", arg);
> > +			if (i + 1 < nmem)
> > +				trace_seq_puts(s, ", ");
> > +		}
> > +		return 0;
> > +	}
> > +
> > +	if (BTF_INFO_KIND(type->info) != BTF_KIND_FUNC) {
> > +		printf("Invalid func type %d %s\n", BTF_INFO_KIND(type->info),
> > +		       btf_type_str(type));
> > +		return -1;
> > +	}
> > +
> > +	/* Get the function proto */
> > +	type = btf_get_type(btf, type->type);
> > +
> > +	/* No proto means "()" ? */
> > +	if (!type)
> > +		return 0;
> > +
> > +	if (BTF_INFO_KIND(type->info) != BTF_KIND_FUNC_PROTO) {
> > +		printf("Invalid func proto type %d %s\n", BTF_INFO_KIND(type->info),
> > +		       btf_type_str(type));
> > +		return -1;
> > +	}
> > +
> > +	/* Get the number of parameters */
> > +	nr = BTF_INFO_VLEN(type->info);
> > +
> > +	/* The parameters are right after the FUNC_PROTO type */
> > +	param = ((void *)type) + sizeof(*type);
> > +
> > +	for (a = 0, p = 0; p < nr; a++, p++) {
> > +		struct btf_type *t;
> > +
> > +		if (p)
> > +			trace_seq_puts(s, ", ");
> > +
> > +		if (a == nmem) {
> > +			trace_seq_puts(s, "...");
> > +			break;
> > +		}
> > +
> > +		assign_arg(&arg, args, size, a);
> > +
> > +		param_name = btf_name(btf, param[p].name_off);
> > +		if (param_name)
> > +			trace_seq_printf(s, "%s=", param_name);
> > +
> > +		t = btf_skip_modifiers(btf, param[p].type);
> > +
> > +		switch (t ? BTF_INFO_KIND(t->info) : BTF_KIND_UNKN) {
> > +		case BTF_KIND_UNKN:
> > +			trace_seq_putc(s, '?');
> > +			/* Still print unknown type values */
> > +			/* fallthough */
> > +		case BTF_KIND_PTR:
> > +			trace_seq_printf(s, "0x%llx", arg);
> > +			break;
> > +		case BTF_KIND_INT:
> > +			encode = *(int *)((void *)t + sizeof(*t));
> > +			/* Print unsigned ints as hex */
> > +			if (BTF_INT_ENCODING(encode) & BTF_INT_SIGNED)  
> 
> BTF_INT_OFFSET() and BTF_INT_VAL() values should also be used to shift and mask
> appropriately.

You meant to the "arg" passed in?

> 
> I assume that regardless of BTF_INT_CHAR and BTF_INT_BOOL value, BTF_INT_SIGNED is
> set appropriately.
> 
> > +				trace_seq_printf(s, "%lld", arg);
> > +			else
> > +				trace_seq_printf(s, "0x%llx", arg);
> > +			break;
> > +		case BTF_KIND_ENUM:  
> 
> Could add as well: case BTF_KIND_ENUM64:

Of course then we would need to check if this is a 32 bit infrastructure.
I'm assuming it would be treated differently. More like a struct?

> 
> > +			trace_seq_printf(s, "%lld", arg);  
> 
> This could probably be improved to display the enum variant name, with the caveats that:

If btf has it, sure! But that can come later.

Thanks,

-- Steve


> 
> 1. C allows values that are not that of any enumerator AFAIR, so you'd need the
>     raw int display fallback.
> 2. GNU C allows enum forward declaration. This is not ISO C and BTF has no specific
>     representation for that. As a result, last time I checked you end up with
>     BTF_KIND_ENUM and info.vlen == 0. Another entry with info.vlen != 0 should
>     also exist and give the actual enumerators list. Those forward decl provide an
>     incomplete type though, so you should not encounter it directly in function
>     parameters.
> 
> > +			break;  
> 
> > +		default:
> > +			/* This does not handle complex arguments */
> > +			trace_seq_printf(s, "(%s)[0x%llx", btf_type_str(t), arg);
> > +			for (x = sizeof(long); x < t->size; x += sizeof(long)) {
> > +				trace_seq_putc(s, ':');
> > +				if (++a == nmem) {
> > +					trace_seq_puts(s, "...]");
> > +					return 0;
> > +				}
> > +				assign_arg(&arg, args, size, a);
> > +				trace_seq_printf(s, "0x%llx", arg);
> > +			}
> > +			trace_seq_putc(s, ']');
> > +			break;
> > +		}
> > +	}
> > +	return 0;
> > +}  


  reply	other threads:[~2025-08-04 12:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-31 18:52 [PATCH 0/2] libtraceevent: Add BTF parsing for function arguments Steven Rostedt
2025-07-31 18:52 ` [PATCH 1/2] libtraceevent: Add loading of BTF to the tep handle Steven Rostedt
2025-08-04 11:47   ` Douglas Raillard
2025-08-04 12:43     ` Steven Rostedt [this message]
2025-08-04 14:18       ` Douglas Raillard
2025-08-04 14:41         ` Steven Rostedt
2025-08-04 15:01           ` Douglas Raillard
2025-08-04 16:07   ` Douglas Raillard
2025-08-04 16:26     ` Douglas Raillard
2025-08-04 17:34     ` Steven Rostedt
2025-07-31 18:52 ` [PATCH 2/2] libtraceevent: Add man page for the new BTF functions Steven Rostedt
2025-08-04 11:09   ` Douglas Raillard
2025-08-04 12:32     ` Steven Rostedt
2025-08-04 13:19       ` Douglas Raillard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250804084322.15b8c321@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=aahringo@redhat.com \
    --cc=douglas.raillard@arm.com \
    --cc=irogers@google.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=takayas@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).