[Crash-utility] [PATCH] extensions/trace.so: fix MEMBER_OFFSET usage

[Dave Anderson]

----- Original Message -----
> 
> 
> ----- Original Message -----
> > On Tue, Sep 27, 2016 at 02:49:56PM -0400, Dave Anderson wrote:
> > > In any case, it seems counter-productive to fix this issue by essentially
> > > going backwards and using ANON_MEMBER_OFFSET(), which was a kludge to
> > > begin with.
> > > 
> > > Would it be possible to fix this in different manner while continuing to
> > > use the new-and-improved MEMBER_OFFSET()?
> > 
> > Just replacing ANON_MEMBER_OFFSET() with MEMBER_OFFSET() in the patch
> > works.  Here's an updated patch:
> 
> Nice! Thanks!
> 
> I'll wait a couple days until I hear something from Fujitsu, but if I don't,
> I'll probably go ahead and check the patch in anyway -- because I'd like to
> release crash-7.1.6 by the end of this week.
> 
> Dave

Rabin,

I still haven't heard anything from Fujitsu, so I've gone ahead and queued
this for crash-7.1.6:

  https://github.com/crash-utility/crash/commit/6997fbec4a548703851af98b2146c9a5a0a01a80

Thanks for catching and fixing this,
  Dave

  
> 
>  
> > 
> > 8<--------------------
> > From 4e1b5c07911e9ef815b36970f4ffea71a52a059d Mon Sep 17 00:00:00 2001
> > From: Rabin Vincent <rabinv at axis.com>
> > Date: Tue, 27 Sep 2016 19:40:39 +0200
> > Subject: [PATCHv2] extensions/trace.so: fix MEMBER_OFFSET usage
> > 
> > In Linux v3.15 the name field of ftrace_event_fall moved into an
> > anonymous union (along with a new tp field).  The trace extension
> > attempt to handle this by first checking if the offset can be
> > determined by MEMBER_OFFSET, and if that's the case, assuming that the
> > name is not in an anonymous union and thus that the kernel is pre-v3.15.
> > 
> > This used to work, but since the following commit, MEMBER_OFFSET seems
> > to even find fields inside anonymous unions, so the code always hits the
> > pre-v3.15 path even on newer kernels, preventing the trace extension
> > from successfully loading.
> > 
> > Fix this by reworking the checks to check for the tp field first and
> > using the presence of that field to identify a post-v3.15 kernel.
> > 
> >  commit 5e6fbde738b827e2575aa9ec9ba4b3eb02ac65c5
> >  Author: Dave Anderson <anderson at redhat.com>
> >  Date:   Thu Aug 25 14:26:58 2016 -0400
> > 
> >      Enhancement to determine structure member data if the member is
> >      contained within an anonymous structure or union.  Without the patch,
> >      it is necessary to parse the output of a discrete gdb "printf"
> >      command to determine the offset of such a structure member.
> >      (Alexandr_Terekhov at epam.com)
> > ---
> >  extensions/trace.c | 39 ++++++++++++++++++---------------------
> >  1 file changed, 18 insertions(+), 21 deletions(-)
> > 
> > diff --git a/extensions/trace.c b/extensions/trace.c
> > index f8ccd91..b226ac0 100644
> > --- a/extensions/trace.c
> > +++ b/extensions/trace.c
> > @@ -1030,30 +1030,27 @@ int ftrace_get_event_type_name(ulong call, char
> > *name, int len)
> >  	if (inited)
> >  		goto work;
> >  
> > -	inited = 1;
> > -	name_offset = MAX(MEMBER_OFFSET("ftrace_event_call", "name"),
> > -		MEMBER_OFFSET("trace_event_call", "name"));
> > -	if (name_offset >= 0)
> > -		goto work;
> > -
> > -	name_offset = MAX(ANON_MEMBER_OFFSET("ftrace_event_call", "name"),
> > -		ANON_MEMBER_OFFSET("trace_event_call", "name"));
> > -	if (name_offset < 0)
> > -		return -1;
> > -
> > -	flags_offset = MAX(MEMBER_OFFSET("ftrace_event_call", "flags"),
> > -		MEMBER_OFFSET("trace_event_call", "flags"));
> > -	if (flags_offset < 0)
> > -		return -1;
> > +	name_offset = MAX(MEMBER_OFFSET("ftrace_event_call", "tp"),
> > +		MEMBER_OFFSET("trace_event_call", "tp"));
> > +	if (name_offset >= 0) {
> > +		flags_offset = MAX(MEMBER_OFFSET("ftrace_event_call", "flags"),
> > +			MEMBER_OFFSET("trace_event_call", "flags"));
> > +		if (flags_offset < 0)
> > +			return -1;
> >  
> > -	tp_name_offset = MEMBER_OFFSET("tracepoint", "name");
> > -	if (tp_name_offset < 0)
> > -		return -1;
> > +		tp_name_offset = MEMBER_OFFSET("tracepoint", "name");
> > +		if (tp_name_offset < 0)
> > +			return -1;
> >  
> > -	if (!enumerator_value("TRACE_EVENT_FL_TRACEPOINT", &tracepoint_flag))
> > -		return -1;
> > +		if (!enumerator_value("TRACE_EVENT_FL_TRACEPOINT", &tracepoint_flag))
> > +			return -1;
> >  
> > -	inited = 2;
> > +		inited = 2;
> > +	} else {
> > +		name_offset = MAX(MEMBER_OFFSET("ftrace_event_call", "name"),
> > +			MEMBER_OFFSET("trace_event_call", "name"));
> > +		inited = 1;
> > +	}
> >  
> >  work:
> >  	if (name_offset < 0)
> > --
> > 2.1.4
> > 
> > 
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>