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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJUgMy+jTTAQ+=F=ddryLrftyB0h==pezZdvnZAT-UHmSU994w@mail.gmail.com>
Date: Wed, 3 Sep 2025 17:23:01 -0400
From: Zecheng Li <zecheng@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
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 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. 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?

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

> > -                     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.