Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 2 Jan 2020 14:53:14 -0500
From:   Joel Fernandes <joel@joelfernandes.org>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Antonio Borneo <antonio.borneo@st.com>,
        Ingo Molnar <mingo@redhat.com>, linux-kernel@vger.kernel.org,
        linux-stm32@st-md-mailman.stormreply.com
Subject: Re: [PATCH] tracing: Fix printing ptrs in preempt/irq enable/disable
 events
Message-ID: <20200102195314.GA227154@google.com>
References: <20191127154428.191095-1-antonio.borneo@st.com>
 <20191204092115.30ef75c9@gandalf.local.home>
 <20191221234741.GA116648@google.com>
 <20191223151301.20be63f7@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191223151301.20be63f7@gandalf.local.home>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Dec 23, 2019 at 03:13:01PM -0500, Steven Rostedt wrote:
> On Sat, 21 Dec 2019 18:47:41 -0500
> Joel Fernandes <joel@joelfernandes.org> wrote:
> 
> > On Wed, Dec 04, 2019 at 09:21:15AM -0500, Steven Rostedt wrote:
> > > 
> > > Joel,
> > > 
> > > Any comments on this patch?  
> > 
> > Steve, it looks like this issue happens with trace-cmd not knowing what
> > _stext is. If I do cat trace_pipe , then I don't see the issue as _stext is
> > looked up correctly but the reporter of the bug is using trace-cmd. Is there
> > a way to solve this within trace-cmd? Not knowing much about trace-cmd
> > internals, I will have to defer to you on this though..
> > 
> > Other than this, I need to make the offset to _stext as s32 instead of u32
> > type so that the problem of the symbol location being before _stext does not
> > cause overflow.
> > 
> > Lastly, I am not super convinced that we need to store the full pointer just
> > to handle a case where the offset of the symbol might be more than +-2G from
> > _stext. Once we see such issue, then we can handle it. But right now the size
> > of the trace buffer is utilized better by just storing the offset IMHO.
> >
> 
> Does this fix it for you?

I am guessing this question is for Antonio to try Steve's patch with trace-cmd.
Meanwhile I posted a patch to fix the offset issue by changing the u32 to s32.

thanks,

 - Joel

> 
> -- Steve
> 
> diff --git a/lib/traceevent/event-parse.c b/lib/traceevent/event-parse.c
> index 4fd3907e..dc705dd2 100644
> --- a/lib/traceevent/event-parse.c
> +++ b/lib/traceevent/event-parse.c
> @@ -3595,6 +3595,45 @@ tep_find_event_by_name(struct tep_handle *tep,
>  	return event;
>  }
>  
> +static unsigned long long test_for_symbol(struct tep_handle *tep,
> +					  struct tep_print_arg *arg)
> +{
> +	unsigned long long val = 0;
> +	struct func_list *item = tep->funclist;
> +	char *func;
> +	int i;
> +
> +	if (isdigit(arg->atom.atom[0]))
> +		return 0;
> +
> +	for (i = 0; i < (int)tep->func_count; i++) {
> +		unsigned long long addr;
> +		const char *name;
> +
> +		if (tep->func_map) {
> +			addr = tep->func_map[i].addr;
> +			name = tep->func_map[i].func;
> +		} else if (item) {
> +			addr = item->addr;
> +			name = item->func;
> +			item = item->next;
> +		} else
> +			break;
> +
> +		if (strcmp(arg->atom.atom, name) == 0) {
> +			val = addr;
> +			break;
> +		}
> +	}
> +
> +	func = realloc(arg->atom.atom, 32);
> +	if (func) {
> +		snprintf(func, 32, "%lld", val);
> +		arg->atom.atom = func;
> +	}
> +	return val;
> +}
> +
>  static unsigned long long
>  eval_num_arg(void *data, int size, struct tep_event *event, struct tep_print_arg *arg)
>  {
> @@ -3611,7 +3650,10 @@ eval_num_arg(void *data, int size, struct tep_event *event, struct tep_print_arg
>  		/* ?? */
>  		return 0;
>  	case TEP_PRINT_ATOM:
> -		return strtoull(arg->atom.atom, NULL, 0);
> +		val = strtoull(arg->atom.atom, NULL, 0);
> +		if (!val)
> +			val = test_for_symbol(tep, arg);
> +		return val;
>  	case TEP_PRINT_FIELD:
>  		if (!arg->field.field) {
>  			arg->field.field = tep_find_any_field(event, arg->field.name);