Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210622183858.2962637-1-ndesaulniers@google.com>
 <20210622201822.ayavok3d2fw3u2pl@google.com> <CAKwvOdki=HZh4TYwqwDSo4BWtbGHp6pM_2akA+D3K8JO+dMGoQ@mail.gmail.com>
In-Reply-To: <CAKwvOdki=HZh4TYwqwDSo4BWtbGHp6pM_2akA+D3K8JO+dMGoQ@mail.gmail.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Mon, 28 Jun 2021 11:20:41 -0700
Message-ID: <CAKwvOdnyzRk75+P1Uc=hs8e2aL3oMSDvjy-JeYwXwJwd_G7JRw@mail.gmail.com>
Subject: Re: [PATCH] kallsyms: strip LTO suffixes from static functions
To:     Fangrui Song <maskray@google.com>
Cc:     Kees Cook <keescook@chromium.org>, "KE . LI" <like1@oppo.com>,
        Nathan Chancellor <nathan@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Stephen Rothwell <sfr@canb.auug.org.au>,
        Miroslav Benes <mbenes@suse.cz>,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Stephen Boyd <swboyd@chromium.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        Joe Perches <joe@perches.com>, linux-kernel@vger.kernel.org,
        clang-built-linux@googlegroups.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Jun 28, 2021 at 10:54 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Jun 22, 2021 at 1:18 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On 2021-06-22, 'Nick Desaulniers' via Clang Built Linux wrote:
> > >+/*
> > >+ * LLVM mangles static functions for full LTO so that two static functions with
> > >+ * the same identifier do not collide when all code is combined into one
> > >+ * module. The scheme used converts references to foo into
> > >+ * foo.llvm.974640843467629774, for example. This can break hooking of static
> > >+ * functions with kprobes.
> > >+ */
> >
> > The comment should say ThinLTO instead.
> >
> > The .llvm.123 suffix is for global scope promotion for local linkage
> > symbols. The scheme is ThinLTO specific. This ensures that a local
>
> Oh, boy. Indeed.  I had identified the mangling coming from
> getGlobalNameForLocal(), but looking at the call chain now I see:
>
> FunctionImportGlobalProcessing::processGlobalForThinLTO()
> -> FunctionImportGlobalProcessing::getPromotedName()
>   -> ModuleSummaryIndex::getGlobalNameForLocal()
>
> I'm not sure then how I figured it was specific to full LTO.
>
> Android recently switched from thin LTO to full LTO, which is what I
> assumed was the cause of the bug report. Rereading our internal bug
> report, it was tested against a prior version that did the symbol
> truncation for thinLTO. I then assumed this was full LTO specific for
> whatever reason, and modified the patch to only apply to full LTO.  I
> see via the above call chain that this patch is not correct.  Let me
> send my original patch as a v2. b/189560201 if you're interested.

I can even see the .llvm.<number> symbol names via `llvm-nm` on
vmlinux for thinLTO builds. No such symbols exist for full LTO.

-- 
Thanks,
~Nick Desaulniers