Re: [Linux-kernel-mentees] Fwd: [PATCH v4] x86/entry: emit a symbol for register restoring thunk

[Lukas Bulwahn <lukas.bulwahn@gmail.com>]

Sounds good to me.

On Thu, Jan 14, 2021 at 3:21 PM Aditya <yashsri421@gmail.com> wrote:
>
> On 14/1/21 5:19 pm, Lukas Bulwahn wrote:
> > Dwaipayan and Aditya, here is another candidate for a checkpatch rule.
> >
> > Lukas
> >
> > ---------- Forwarded message ---------
> > From: Mark Brown <broonie@kernel.org>
> > Date: Tue, Jan 12, 2021 at 10:02 PM
> > Subject: Re: [PATCH v4] x86/entry: emit a symbol for register restoring thunk
> > To: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Andy Lutomirski <luto@kernel.org>, Thomas Gleixner
> > <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov
> > <bp@alien8.de>, Fangrui Song <maskray@google.com>, Arnd Bergmann
> > <arnd@arndb.de>, Josh Poimboeuf <jpoimboe@redhat.com>, Jonathan Corbet
> > <corbet@lwn.net>, <x86@kernel.org>, H. Peter Anvin <hpa@zytor.com>,
> > Nathan Chancellor <natechancellor@gmail.com>, Miguel Ojeda
> > <ojeda@kernel.org>, Jiri Slaby <jirislaby@kernel.org>, Joe Perches
> > <joe@perches.com>, <linux-doc@vger.kernel.org>,
> > <linux-kernel@vger.kernel.org>, <clang-built-linux@googlegroups.com>
> >
> >
> > On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:
> >
> > This:
> >
> >> when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
> >> notes:
> >
> >>   So basically, you can use an .L symbol *inside* a function or a code
> >>   segment, you just can't use the .L symbol to contain the code using a
> >>   SYM_*_START/END annotation pair.
> >
> > is a stronger statement than this:
> >
> >> +  Developers should avoid using local symbol names that are prefixed with
> >> +  ``.L``, as this has special meaning for the assembler; a symbol entry will
> >> +  not be emitted into the symbol table. This can prevent ``objtool`` from
> >> +  generating correct unwind info. Symbols with STB_LOCAL binding may still be
> >> +  used, and ``.L`` prefixed local symbol names are still generally useable
> >> +  within a function, but ``.L`` prefixed local symbol names should not be used
> >> +  to denote the beginning or end of code regions via
> >> +  ``SYM_CODE_START_LOCAL``/``SYM_CODE_END``.
> >
> > and seems more what I'd expect - SYM_FUNC* is also affected for example.
> > Even though other usages are probably not very likely it seems better to
> > keep the stronger statement in case someone comes up with one, and to
> > stop anyone spending time wondering why only SYM_CODE_START_LOCAL is
> > affected.
> >
> > This also looks like a good candiate for a checkpatch rule, but that can
> > be done separately of course.
> >
>
> Thanks Lukas!
> I'd like to take a shot at it. This is what I have understood from the
> discussion:
> Introduce a new warning rule for .S files, where ".L" prefixes are
> used in the code, if it is occurring between any kind of START/END pair.
> This is what I am planning to do:
> - Check if the file is ".S" file.
> - If the line contains ".L" prefixed symbol, give user a
> warning/check, so that they can ensure that the line is not inside
> START/END block. (As we may not be able to make sure about the same,
> if the START/END line is not in the patch; otherwise we could run a
> while loop)
>
> What do you think?
>
> Thanks
> Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees