On 16/1/21 7:08 pm, Dwaipayan Ray wrote:

On Sat, Jan 16, 2021 at 6:48 PM Aditya [off-list ref] wrote:

quoted

On 16/1/21 6:13 pm, Dwaipayan Ray wrote:

quoted

On Sat, Jan 16, 2021 at 5:52 PM Aditya Srivastava [off-list ref] wrote:

quoted

Local symbols prefixed with '.L' do not emit symbol table entries, as
they have special meaning for the assembler.

'.L' prefixed symbols can be used within a code region, but should be
avoided for denoting a range of code via 'SYM_*_START/END' annotations.

Add a new check to emit warning on finding the usage of '.L' symbols
in '.S' files.

Suggested-by: Mark Brown <broonie@kernel.org>
Link: https://lore.kernel.org/lkml/20210112210154.GI4646@sirena.org.uk/ (local)
Signed-off-by: Aditya Srivastava <redacted>
---
 scripts/checkpatch.pl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7030c4d6d126..87d96a039e64 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -3590,6 +3590,12 @@ sub process {
                        }
                }

+# check for .L prefix local symbols in .S files
+               if ($realfile =~ /\.S$/ && $line =~ /\.L\S+/) {
+                       WARN("AVOID_L_PREFIX",
+                            "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr);
+               }
+
 # check we are in a valid source file C or perl if not then ignore this hunk
                next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);

--

2.17.1

From an earlier conversation:

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.

So this check warns on all uses of the .L prefix
I think that might be incorrect.

Hey Dwaipayan,
I think you missed this:

quoted

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

Then again what percentage of users actually intended to use
it in a local scope? That is actually a correct usage and checkpatch will
keep complaining - there will be no way to remove the warning.

quoted

At best, I think, we could use $context_function, which should work
for patches, but again, this will not work for files.

Do you have any suggestions?

Does $context_function work for .S files? Am not sure.> But again that will not help much.

The main concern is not to allow .L prefix within a
SYM_*_START...SYM_*_END annotation pair.

That means something like this will be disallowed:

SYM_FUNC_START(memset)
    .L ... etc
SYM_FUNC_END(memset)

Yes, $context_function works inside .S files.
So, I think if it is inside one of the SYM_*_START contexts, it should
work for us.

One possible way can be to extract the context after getting
a SYM_*_START. Following which the lines could be checked
for a .L preifx.

quoted

But complexity wise I dont know if it would be worth it.

I think, we could do this for files.

But I am not sure about the complexity part.
Lukas, 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