Skip to contentSkip to conversation

gen_compile_commands: fix overlooked module files

PATCH In review John Hubbard 2 participants 3 messages Jun 16, 2022 – Jun 21, 2022

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 gen_compile_commands: fix overlooked module files

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.

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 .

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.

Fixes 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
Cc Masahiro Yamada <[email protected]>
Cc Nick Desaulniers <[email protected]>

---

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(-)
scripts/clang-tools/gen_compile_commands.py +3 -4
index 1d1bde1fd45e..acf4ec28aaf1 100755
@@ -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())
 
 def process_line(root_directory, command_prefix, file_path):
     """Extracts information from a .cmd line and creates an entry from it.
Signature · 1 line
2.36.1
#1 lore raw
Quoted 8 lines · John Hubbard

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".

Quoted 8 lines

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 .

Quoted 4 lines

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.

Quoted 22 lines
Fixes 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
Cc Masahiro Yamada <[email protected]>
Cc Nick Desaulniers <[email protected]>

---

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(-)
scripts/clang-tools/gen_compile_commands.py +0 -0
index 1d1bde1fd45e..acf4ec28aaf1 100755
@@ -157,11 +157,10 @@ def cmdfiles_for_modorder(modorder):
             if ext != '.ko':
                 sys.exit('{}: module path must end with .ko'.format(ko))

scripts/clang-tools/gen_compile_commands.py def cmdfiles_for_modorder(modorder): · line 164

Quoted 10 lines
scripts/clang-tools/gen_compile_commands.py +3 -4
             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.

This file consistently keeps two lines between functions, as suggested by PEP8. [1]

[1] : https://peps.python.org/pep-0008/#blank-lines

--
Best Regards
Masahiro Yamada

#2 lore raw
Quoted 31 lines · Masahiro Yamada
Quoted 8 lines · John Hubbard

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".

Quoted 7 lines

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

Quoted 7 lines
Quoted 1 line

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!

Quoted 7 lines
Quoted 6 lines

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.

Quoted 8 lines

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.

Quoted 29 lines
Quoted 29 lines
Fixes 9413e7640564 ("kbuild: split the second line of *.mod into *.usyms")
Cc Masahiro Yamada <[email protected]>
Cc Nick Desaulniers <[email protected]>

---

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(-)
scripts/clang-tools/gen_compile_commands.py +1 -2
index 1d1bde1fd45e..acf4ec28aaf1 100755
@@ -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():
Quoted 8 lines

scripts/clang-tools/gen_compile_commands.py def cmdfiles_for_modorder(modorder): · line 164

Quoted 5 lines
scripts/clang-tools/gen_compile_commands.py +2 -2
-                    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,

Signature · 2 lines
John Hubbard
NVIDIA