linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ian Rogers <irogers@google.com>, Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-perf-users@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Stephane Eranian <eranian@google.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	linux-toolchains@vger.kernel.org,
	linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 03/23] perf dwarf-aux: Handle type transfer for memory access
Date: Tue, 19 Mar 2024 10:55:02 -0300	[thread overview]
Message-ID: <ZfmZNp1DaKYZCwyf@x1> (raw)
In-Reply-To: <20240319055115.4063940-4-namhyung@kernel.org>

On Mon, Mar 18, 2024 at 10:50:55PM -0700, Namhyung Kim wrote:
> We want to track type states as instructions are executed.  Each
> instruction can access compound types like struct or union and load/
> store its members to a different location.
> 
> The die_deref_ptr_type() is to find a type of memory access with a
> pointer variable.  If it points to a compound type like struct, the
> target memory is a member in the struct.  The access will happen
> with an offset indicating which member it refers.  Let's follow the
> DWARF info to figure out the type of the pointer target.
> 
> For example, say we have the following code.
> 
>   struct foo {
>     int a;
>     int b;
>   };
> 
>   struct foo *p = malloc(sizeof(*p));
>   p->b = 0;
> 
> The last pointer access should produce x86 asm like below:
> 
>   mov  0x0, 4(%rbx)
> 
> And we know %rbx register has a pointer to struct foo.  Then offset 4
> should return the debug info of member 'b'.
> 
> Also variables of compound types can be accessed directly without a
> pointer.  The die_get_member_type() is to handle a such case.
> 
> Acked-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/dwarf-aux.c | 110 ++++++++++++++++++++++++++++++++++++
>  tools/perf/util/dwarf-aux.h |   6 ++
>  2 files changed, 116 insertions(+)
> 
> diff --git a/tools/perf/util/dwarf-aux.c b/tools/perf/util/dwarf-aux.c
> index 785aa7a3d725..cd9364d296b6 100644
> --- a/tools/perf/util/dwarf-aux.c
> +++ b/tools/perf/util/dwarf-aux.c
> @@ -1838,3 +1838,113 @@ int die_get_scopes(Dwarf_Die *cu_die, Dwarf_Addr pc, Dwarf_Die **scopes)
>  	*scopes = data.scopes;
>  	return data.nr;
>  }
> +
> +static int __die_find_member_offset_cb(Dwarf_Die *die_mem, void *arg)
> +{
> +	Dwarf_Die type_die;
> +	Dwarf_Word size, loc;
> +	Dwarf_Word offset = (long)arg;
> +	int tag = dwarf_tag(die_mem);
> +
> +	if (tag != DW_TAG_member)
> +		return DIE_FIND_CB_SIBLING;
> +
> +	/* Unions might not have location */
> +	if (die_get_data_member_location(die_mem, &loc) < 0)
> +		loc = 0;
> +
> +	if (offset == loc)
> +		return DIE_FIND_CB_END;
> +
> +	die_get_real_type(die_mem, &type_die);

Don't we have to check that the type_die was initialized? From what I
can see there is the possibility it isn't:

  Dwarf_Die type_die;
  die_get_real_type(die_mem, &type_die);
        do {
                vr_die = __die_get_real_type(vr_die, die_mem);
        } while (vr_die && dwarf_tag(vr_die) == DW_TAG_typedef);
        
        return vr_die;

    static Dwarf_Die *__die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)

        do {
                vr_die = die_get_type(vr_die, die_mem);
                if (!vr_die)
                        break;

   Dwarf_Die *die_get_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)

        Dwarf_Attribute attr;

        if (dwarf_attr_integrate(vr_die, DW_AT_type, &attr) &&
            dwarf_formref_die(&attr, die_mem))
                return die_mem;
        else
                return NULL;
  } 

> +	if (dwarf_aggregate_size(&type_die, &size) < 0)
> +		size = 0;
> +
> +	if (loc < offset && offset < (loc + size))
> +		return DIE_FIND_CB_END;
> +
> +	return DIE_FIND_CB_SIBLING;
> +}
> +
> +/**
> + * die_get_member_type - Return type info of struct member
> + * @type_die: a type DIE
> + * @offset: offset in the type
> + * @die_mem: a buffer to save the resulting DIE
> + *
> + * This function returns a type of a member in @type_die where it's located at
> + * @offset if it's a struct.  For now, it just returns the first matching
> + * member in a union.  For other types, it'd return the given type directly
> + * if it's within the size of the type or NULL otherwise.
> + */
> +Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset,
> +			       Dwarf_Die *die_mem)
> +{
> +	Dwarf_Die *member;
> +	Dwarf_Die mb_type;
> +	int tag;
> +
> +	tag = dwarf_tag(type_die);
> +	/* If it's not a compound type, return the type directly */
> +	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
> +		Dwarf_Word size;
> +
> +		if (dwarf_aggregate_size(type_die, &size) < 0)
> +			size = 0;
> +
> +		if ((unsigned)offset >= size)
> +			return NULL;
> +
> +		*die_mem = *type_die;
> +		return die_mem;
> +	}
> +
> +	mb_type = *type_die;
> +	/* TODO: Handle union types better? */
> +	while (tag == DW_TAG_structure_type || tag == DW_TAG_union_type) {
> +		member = die_find_child(&mb_type, __die_find_member_offset_cb,
> +					(void *)(long)offset, die_mem);
> +		if (member == NULL)
> +			return NULL;
> +
> +		if (die_get_real_type(member, &mb_type) == NULL)
> +			return NULL;
> +
> +		tag = dwarf_tag(&mb_type);
> +
> +		if (tag == DW_TAG_structure_type || tag == DW_TAG_union_type) {
> +			Dwarf_Word loc;
> +
> +			/* Update offset for the start of the member struct */
> +			if (die_get_data_member_location(member, &loc) == 0)
> +				offset -= loc;
> +		}
> +	}
> +	*die_mem = mb_type;
> +	return die_mem;
> +}
> +
> +/**
> + * die_deref_ptr_type - Return type info for pointer access
> + * @ptr_die: a pointer type DIE
> + * @offset: access offset for the pointer
> + * @die_mem: a buffer to save the resulting DIE
> + *
> + * This function follows the pointer in @ptr_die with given @offset
> + * and saves the resulting type in @die_mem.  If the pointer points
> + * a struct type, actual member at the offset would be returned.
> + */
> +Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset,
> +			      Dwarf_Die *die_mem)
> +{
> +	Dwarf_Die type_die;
> +
> +	if (dwarf_tag(ptr_die) != DW_TAG_pointer_type)
> +		return NULL;
> +
> +	if (die_get_real_type(ptr_die, &type_die) == NULL)
> +		return NULL;
> +
> +	return die_get_member_type(&type_die, offset, die_mem);
> +}
> diff --git a/tools/perf/util/dwarf-aux.h b/tools/perf/util/dwarf-aux.h
> index cd171b06fd4c..16c916311bc0 100644
> --- a/tools/perf/util/dwarf-aux.h
> +++ b/tools/perf/util/dwarf-aux.h
> @@ -144,6 +144,12 @@ struct die_var_type {
>  	int offset;
>  };
>  
> +/* Return type info of a member at offset */
> +Dwarf_Die *die_get_member_type(Dwarf_Die *type_die, int offset, Dwarf_Die *die_mem);
> +
> +/* Return type info where the pointer and offset point to */
> +Dwarf_Die *die_deref_ptr_type(Dwarf_Die *ptr_die, int offset, Dwarf_Die *die_mem);
> +
>  #ifdef HAVE_DWARF_GETLOCATIONS_SUPPORT
>  
>  /* Get byte offset range of given variable DIE */
> -- 
> 2.44.0.291.gc1ea87d7ee-goog

  reply	other threads:[~2024-03-19 13:55 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-19  5:50 [PATCHSET 00/23] Remaining bits of data type profiling (v7) Namhyung Kim
2024-03-19  5:50 ` [PATCH 01/23] perf dwarf-aux: Remove unused pc argument Namhyung Kim
2024-03-19 13:43   ` Arnaldo Carvalho de Melo
2024-03-19 17:39     ` Namhyung Kim
2024-03-19  5:50 ` [PATCH 02/23] perf dwarf-aux: Add die_collect_vars() Namhyung Kim
2024-03-19 13:45   ` Arnaldo Carvalho de Melo
2024-03-19  5:50 ` [PATCH 03/23] perf dwarf-aux: Handle type transfer for memory access Namhyung Kim
2024-03-19 13:55   ` Arnaldo Carvalho de Melo [this message]
2024-03-19 17:41     ` Namhyung Kim
2024-03-19  5:50 ` [PATCH 04/23] perf dwarf-aux: Add die_find_func_rettype() Namhyung Kim
2024-03-19 13:56   ` Arnaldo Carvalho de Melo
2024-03-19 17:42     ` Namhyung Kim
2024-03-19 18:19       ` Arnaldo Carvalho de Melo
2024-03-19 20:33         ` Namhyung Kim
2024-03-19  5:50 ` [PATCH 05/23] perf map: Add map__objdump_2rip() Namhyung Kim
2024-03-19  5:50 ` [PATCH 06/23] perf annotate-data: Introduce struct data_loc_info Namhyung Kim
2024-03-19  5:50 ` [PATCH 07/23] perf annotate: Add annotate_get_basic_blocks() Namhyung Kim
2024-03-19  5:51 ` [PATCH 08/23] perf annotate-data: Add debug messages Namhyung Kim
2024-03-19 14:05   ` Arnaldo Carvalho de Melo
2024-03-19  5:51 ` [PATCH 09/23] perf annotate-data: Maintain variable type info Namhyung Kim
2024-03-19 14:07   ` Arnaldo Carvalho de Melo
2024-03-19 17:44     ` Namhyung Kim
2024-03-19 18:12       ` Arnaldo Carvalho de Melo
2024-03-19 20:34         ` Namhyung Kim
2024-03-19  5:51 ` [PATCH 10/23] perf annotate-data: Add update_insn_state() Namhyung Kim
2024-03-19  5:51 ` [PATCH 11/23] perf annotate-data: Add get_global_var_type() Namhyung Kim
2024-03-19  5:51 ` [PATCH 12/23] perf annotate-data: Handle global variable access Namhyung Kim
2024-03-19  5:51 ` [PATCH 13/23] perf annotate-data: Handle call instructions Namhyung Kim
2024-03-19  5:51 ` [PATCH 14/23] perf annotate-data: Implement instruction tracking Namhyung Kim
2024-03-19  5:51 ` [PATCH 15/23] perf annotate-data: Check register state for type Namhyung Kim
2024-03-19  5:51 ` [PATCH 16/23] perf annotate: Parse x86 segment register location Namhyung Kim
2024-03-19  5:51 ` [PATCH 17/23] perf annotate-data: Handle this-cpu variables in kernel Namhyung Kim
2024-03-19  5:51 ` [PATCH 18/23] perf annotate-data: Track instructions with a this-cpu variable Namhyung Kim
2024-03-19  5:51 ` [PATCH 19/23] perf annotate-data: Support general per-cpu access Namhyung Kim
2024-03-19  5:51 ` [PATCH 20/23] perf annotate-data: Handle ADD instructions Namhyung Kim
2024-03-19  5:51 ` [PATCH 21/23] perf annotate-data: Add stack canary type Namhyung Kim
2024-03-19  5:51 ` [PATCH 22/23] perf annotate-data: Add a cache for global variable types Namhyung Kim
2024-03-19 18:05   ` Namhyung Kim
2024-03-19 18:07     ` Arnaldo Carvalho de Melo
2024-03-19 18:09       ` Arnaldo Carvalho de Melo
2024-03-19 18:09       ` Namhyung Kim
2024-03-19  5:51 ` [PATCH 23/23] perf annotate-data: Do not retry for invalid types Namhyung Kim
2024-03-19 14:17 ` [PATCHSET 00/23] Remaining bits of data type profiling (v7) Arnaldo Carvalho de Melo

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=ZfmZNp1DaKYZCwyf@x1 \
    --to=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-toolchains@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    /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).