linux-kernel - Re: [PATCH v2 08/10] perf dwarf-aux: Skip check_variable for die_find_variable_by

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aLtDX0W0DMMr6BE4@google.com>
Date: Fri, 5 Sep 2025 13:09:03 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Zecheng Li <zecheng@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"Liang, Kan" <kan.liang@...ux.intel.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Xu Liu <xliuprof@...gle.com>, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 08/10] perf dwarf-aux: Skip check_variable for
 die_find_variable_by_reg

On Wed, Sep 03, 2025 at 05:23:01PM -0400, Zecheng Li wrote:
> On Sat, Aug 30, 2025 at 3:31 AM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > > - In match_var_offset, use __die_get_real_type instead of
> > >   die_get_real_type to preserve typedefs. Move the (offset == 0) branch
> >
> > Why do you want to preserve typedefs?  I think a variable type can be
> > typedef to a pointer then now it won't resolve that target type anymore.
> 
> check_variable preserves the typedefs. It would sometimes resolve to
> an unnamed struct if we remove the typedefs. 

Ah, that's unfortunate.

> Let me test if it will
> affect the dwarf_tag(&data->type) == DW_TAG_pointer_type check. Also I
> found calling dwarf_aggregate_size on typedef'd types gives the
> correct result, so maybe we don't need the sized_type in
> check_variable?

You're right.

> 
> > > - When comparing types from different scopes, first compare their type
> > >   offsets. A larger offset means the field belongs to an outer
> > >   (enclosing) struct. This helps resolve cases where a pointer is found
> > >   in an inner scope, but a struct containing that pointer exists in an
> > >   outer scope. Previously, is_better_type would prefer the pointer type,
> > >   but the struct type is actually more complete and should be chosen.
> >
> > Can we improve is_better_type() then?
> 
> Here we are comparing two types with the extra access offset
> information. In other contexts, the calls to is_better_type compares
> two types only, so I think we don't need to add two new parameters to
> is_better_type?

Right, I meant just about pointer type and struct type.  It compares two
types take the same location so I didn't expect they can be a pointer
and a struct.  My intention was about a pointer and a base type.

Also you may consider typedef and struct.  I think we prefer struct
since it can access the member field.  But as you said we should use
typedef if it's an unnamed struct.  It'd be great if we can get members
even for typedefs (for structs).

> 
> > > -                     if (!found || is_better_type(type_die, &mem_die)) {
> > > +                     if (!found || dloc->type_offset < type_offset ||
> > > +                             (dloc->type_offset == type_offset &&
> > > +                              is_better_type(type_die, &mem_die))) {
> > >                               *type_die = mem_die;
> > >                               dloc->type_offset = type_offset;
> > >                               found = true;
> 
> I find changing the is_better_type call to !is_better_type(&mem_die,
> type_die) would yield better results: prefer types from outer scope if
> the current one is not "better" than the new one.

Ok, sounds good.

Thanks,
Namhyung