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

[Dave Anderson]

Hi Rabin,

I am still waiting for Fujitsu to name a new maintainer for the trace.c
extension module (given that Qiao Nuohan has apparently abandoned it).  
I have been told by Fujitsu that they would resolve the maintainership
issue some time this week.  

Until such time, there is still this patch proposal outstanding:

  [Crash-utility] [Crash-Utility] extensions/trace: Add support for multiple ftrace buffers
  https://www.redhat.com/archives/crash-utility/2016-August/msg00000.html

And now this one...

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()?

Dave



----- Original Message -----
> From: Rabin Vincent <rabinv at axis.com>
> 
> 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..b5901c4 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(ANON_MEMBER_OFFSET("ftrace_event_call", "tp"),
> +		ANON_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
>