Message-ID: <CAK7LNATAyCUNkWv+NqKz7HZGRsjByZYsADf=d9-JT_QAh56S3Q@mail.gmail.com>
Date:   Tue, 8 Aug 2023 05:06:08 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Will Deacon <will@...nel.org>
Cc:     linux-kernel@...r.kernel.org, kernel-team@...roid.com,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nicolas Schier <nicolas@...sle.eu>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        John Stultz <jstultz@...gle.com>, linux-kbuild@...r.kernel.org
Subject: Re: [PATCH v3 3/4] scripts/faddr2line: Constrain readelf output to
 symbols from System.map

On Fri, Aug 4, 2023 at 11:30 PM Will Deacon <will@...nel.org> wrote:
>
> On Thu, Aug 03, 2023 at 04:54:37AM +0900, Masahiro Yamada wrote:
> > On Fri, Jul 28, 2023 at 8:34 PM Will Deacon <will@...nel.org> wrote:
> > >
> > > Some symbols emitted in the readelf output but filtered from System.map
> > > can confuse the 'faddr2line' symbol size calculation, resulting in the
> > > erroneous rejection of valid offsets. This is especially prevalent when
> > > building an arm64 kernel with CONFIG_CFI_CLANG=y, where most functions
> > > are prefixed with a 32-bit data value in a '$d.n' section. For example:
> > >
> > > 447538: ffff800080014b80   548 FUNC    GLOBAL DEFAULT    2 do_one_initcall
> > >    104: ffff800080014c74     0 NOTYPE  LOCAL  DEFAULT    2 $x.73
> > >    106: ffff800080014d30     0 NOTYPE  LOCAL  DEFAULT    2 $x.75
> > >    111: ffff800080014da4     0 NOTYPE  LOCAL  DEFAULT    2 $d.78
> > >    112: ffff800080014da8     0 NOTYPE  LOCAL  DEFAULT    2 $x.79
> > >     36: ffff800080014de0   200 FUNC    LOCAL  DEFAULT    2 run_init_process
> > >
> > > Adding a warning to do_one_initcall() results in:
> > >
> > >   | WARNING: CPU: 0 PID: 1 at init/main.c:1236 do_one_initcall+0xf4/0x260
> > >
> > > Which 'faddr2line' refuses to accept:
> > >
> > > $ ./scripts/faddr2line vmlinux do_one_initcall+0xf4/0x260
> > > skipping do_one_initcall address at 0xffff800080014c74 due to size mismatch (0x260 != 0x224)
> > > no match for do_one_initcall+0xf4/0x260
> > >
> > > Filter out entries from readelf using the 'sysmap-ignored-syms.sed'
> > > script used to construct System.map, so that the size of a symbol is
> > > calculated as a delta to the next symbol present in ksymtab.
> >
> >
> > I do not think this patch set is the right approach.
> >
> > I assume faddr2line is meant to work with both vmlinux
> > and modules.
>
> Huh, it seems to be busted for modules :/ I get:
>
>  | error: unknown argument '--section=.text'
>
> with llvm and:
>
>  | addr2line: DWARF error: invalid or unhandled FORM value: 0x25
>
> with binutils.
>
> I'll look into this, as I don't think it's related to symbol filtering.
>
> > A problem is that we have different filtering policies wrt kallsyms.
> >
> > scripts/mksysmap filters symbols in vmlinux,
> > while kernel/module/kallsyms.c filters ones in modules.
>
> I don't understand why we need two different ways of filtering out
> symbols, but it appears that the module case only filters out local
> labels and mapping symbols, both of which are filtered out of vmlinux
> as well. Is that right?


For vmlinux kallsyms, the reason is the annoyance in the build process.
kallsyms requires linking vmlinux three times.
(see scripts/link-vmlinux.sh if you are interested)


[1a] link vmlinux without symbol table
[1b] Generate symbol list from [1a]
[2a] link vmlinux, embedding the temp kallsyms created by [1b]
[2b] Generate symbol list from [2a]
[3a] link vmlinux, embedding the final kallsyms created by [2b]


Our assumption is that [1b] and [2b] have the same number of
symbols, so that [2a] and [3a] can embed the same size kallsyms.


However, the process [2a] inserts the kallsyms table
in the middle of vmlinux.
It may cause the compiler to emit additional symbols
(e.g. ARM veneers) due to the inserted kallsyms.


So, we filter out all compiler-generated symbols so that
[1b] and [2b] have the same number of symbols.


If we do not do it,
we may have more and more kallsyms steps.

Emitted veneers increase kallsyms size
-> Increased kallsyms causes more veneers
-> The new veneers increase kallsyms size
-> Increased kallsyms causes yet more veneers
-> The new veneers increase kallsyms size again
-> (this cycle may continue again and again)






> > This patch tries to get aligned with the stacktrace of vmlinux,
> > but that does not seem optimal to the stacktrace of modules.
> >
> >
> > I have not checked the details, but I guess
> > the module kallsyms filters less symbols.
> >
> > https://github.com/torvalds/linux/blob/v6.5-rc4/kernel/module/kallsyms.c#L288
> >
> > I prefer filtering symbols in the intersection of vmlinux and modules.
>
> I think mksysmap filters out a superset of the symbols which are filtered
> for modules, so why is the intersection the right thing to do? That will
> mean that faddr2line considers a whole bunch of symbols that aren't in
> the ksymtab of vmlinux.



If A is a subset of B,
(A & B) == A.


I meant "the intersection of the filtering rules of vmlinux and modules"
== "the filtering rule of modules".







>
> > is_mapping_symbol() filters symbols you are addressing.
>
> That's a C function and faddr2line is a shell script. What exactly do
> you want me to do? My first hack just matched on symbols starting with
> '$' but I ended up with this after other review feedback.


In linux-next, I see this:

static inline int is_mapping_symbol(const char *str)
{
        if (str[0] == '.' && str[1] == 'L')
                return true;
        if (str[0] == 'L' && str[1] == '0')
                return true;
        return str[0] == '$';
}


It is pretty easy to write a sed script to do the same thing.





Another possibility is to use different filtering rules
between vmlinux and modules.
(but I do not know if this complexity is worthwhile)



[pseudo code]


get_symbol_list () {

    if [ "${is_vmlinux}" = 1 ]; then
            {READELF} --symbols --wide $objfile | sed -f
scripts/sysmap-ignored-syms.sed
    else
            {READELF} --symbols --wide $objfile | sed  'remove ".L", 'L0", "^$"'
    fi
}




>
> Josh -- how do you want to proceed here?
>
> Will



-- 
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists