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 <redacted>
Cc: Andy Lutomirski <luto@kernel.org>, Thomas Gleixner
[off-list ref], Ingo Molnar [off-list ref], Borislav Petkov
[off-list ref], Fangrui Song [off-list ref], Arnd Bergmann
[off-list ref], Josh Poimboeuf [off-list ref], Jonathan Corbet
[off-list ref], [off-list ref], H. Peter Anvin [off-list ref],
Nathan Chancellor [off-list ref], Miguel Ojeda
[off-list ref], Jiri Slaby [off-list ref], Joe Perches
[off-list ref], [off-list ref],
[off-list ref], [off-list ref]


On Tue, Jan 12, 2021 at 11:46:24AM -0800, Nick Desaulniers wrote:

This:

quoted

when building with LLVM_IAS=1 (Clang's integrated assembler). Josh
notes:

quoted

  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:

quoted

+  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