[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f8dede84-eb74-2cdd-c630-21d57a7b3648@nvidia.com>
Date: Tue, 21 Jun 2022 00:21:45 -0700
From: John Hubbard <jhubbard@...dia.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Nick Desaulniers <ndesaulniers@...gle.com>,
Nathan Chancellor <nathan@...nel.org>,
Tom Rix <trix@...hat.com>, Jason Gunthorpe <jgg@...dia.com>,
LKML <linux-kernel@...r.kernel.org>,
clang-built-linux <llvm@...ts.linux.dev>
Subject: Re: [PATCH v2] gen_compile_commands: fix overlooked module files
On 6/19/22 17:15, Masahiro Yamada wrote:
> On Thu, Jun 16, 2022 at 11:12 AM John Hubbard <jhubbard@...dia.com> wrote:
>>
>> scripts/clang-tools/gen_compile_commands.py incorrectly assumes that
>> each .mod file only contains one line. In fact, such files contain one
>> entry per line, and for some subsystems, there can be many, many lines.
>> For example, Nouveau has 762 entries, but only the first entry was being
>> processed. This problem causes clangd to fail to provide references and
>> definitions for valid files that are part of the current kernel
>> configuration.
>
>
> My previous comment "Can you update the commit log?"
> meant, "Can you rewrite the commit log (in a more concise way)?".
>
>
>
> I am not sure the Nouveau example is needed here, but
> more important information is that
> ecca4fea1ede4 changed the .mod file
> from "all entries at the first line" to "one entry per line".
>
>
>
>
>> This problem only occurs when using Kbuild to generate, like this:
>>
>> make CC=clang compile_commands.json
>>
>> It does not occur if you just run gen_compile_commands.py "bare", like
>> this (below):
>>
>> scripts/clang-tools/gen_compile_commands.py/gen_compile_commands.py .
>
> I am not sure if this is needed, but at least, the command line is wrong.
>
> scripts/clang-tools/gen_compile_commands.py/gen_compile_commands.py .
> -->
> scripts/clang-tools/gen_compile_commands.py .
Good catch, not sure how that typo leaked in!
>
>
>> Fix this by fully processing each .mod file. This fix causes the number
>> of build commands that clangd finds in my kernel build (these numbers
>> are heavily dependent upon .config), from 2848 to 5292, which is an 85%
>> increase.
>
> I am not sure if the second sentence is needed
> because this patch is just an obvious fix.
>
>
>
> Sorry, I missed to adjust this file in ecca4fea1ede4,
> and ended up bothering you too much.
>
I'll have a go at rewriting the commit description, sure.
>
>
>> Fixes: 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
>> Cc: Masahiro Yamada <masahiroy@...nel.org>
>> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
>> Signed-off-by: John Hubbard <jhubbard@...dia.com>
>> ---
>>
>> Changes since v1:
>>
>> Applied changes recommended by Masahiro Yamada (thanks!): corrected the
>> "Fixes" tag, and improved the python code for parsing .mod files.
>>
>>
>> scripts/clang-tools/gen_compile_commands.py | 7 +++----
>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/clang-tools/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
>> index 1d1bde1fd45e..acf4ec28aaf1 100755
>> --- a/scripts/clang-tools/gen_compile_commands.py
>> +++ b/scripts/clang-tools/gen_compile_commands.py
>> @@ -157,11 +157,10 @@ def cmdfiles_for_modorder(modorder):
>> if ext != '.ko':
>> sys.exit('{}: module path must end with .ko'.format(ko))
>> mod = base + '.mod'
>> - # The first line of *.mod lists the objects that compose the module.
>> + # Read from *.mod, to get a list of objects that compose the module.
>> with open(mod) as m:
>> - for obj in m.readline().split():
>> - yield to_cmdfile(obj)
>> -
>> + for mod_line in m:
>> + yield to_cmdfile(mod_line.rstrip())
>>
>
>
> Please do not remove the blank line.
Sure, I didn't really intend to change the vertical whitespace. I'll restore that.
I'll send out a v3 shortly.
thanks,
--
John Hubbard
NVIDIA
Powered by blists - more mailing lists