[PATCH v2 0/9] kbuild: clang-tidy

[PATCH v2 0/9] kbuild: clang-tidy

[Masahiro Yamada]

I improved gen_compile_commands.py,
then rebased Nathan's v7 [1] on top of them.
To save time, I modified the Makefile part.
No change for run-clang-tools.py

"make clang-tidy" should work in-tree build,
out-of-tree build (O=), and external module build (M=).

This version keeps the previous work-flow.
You can still manually run scripts/gen_compile_commands.json

'make compile_commands.json' or 'make clang-tidy' is handier
for most cases. As Nick noted, there is 3 % loss of the coverage.

If you need the full compilation database that covers all the
compiled C files, please run the script manually.

[1] https://patchwork.kernel.org/patch/11687833/

Masahiro Yamada (8):
  gen_compile_commands: parse only the first line of .*.cmd files
  gen_compile_commands: use choices for --log_levels option
  gen_compile_commands: do not support .cmd files under tools/ directory
  gen_compile_commands: reword the help message of -d option
  gen_compile_commands: make -o option independent of -d option
  gen_compile_commands: move directory walk to a generator function
  gen_compile_commands: support *.o, *.a, modules.order in positional
    argument
  kbuild: wire up the build rule of compile_commands.json to Makefile

Nathan Huckleberry (1):
  Makefile: Add clang-tidy and static analyzer support to makefile

 MAINTAINERS                                 |   1 +
 Makefile                                    |  45 +++-
 scripts/clang-tools/gen_compile_commands.py | 245 ++++++++++++++++++++
 scripts/clang-tools/run-clang-tools.py      |  74 ++++++
 scripts/gen_compile_commands.py             | 151 ------------
 5 files changed, 361 insertions(+), 155 deletions(-)
 create mode 100755 scripts/clang-tools/gen_compile_commands.py
 create mode 100755 scripts/clang-tools/run-clang-tools.py
 delete mode 100755 scripts/gen_compile_commands.py

-- 
2.25.1

[PATCH v2 1/9] gen_compile_commands: parse only the first line of .*.cmd files

[Masahiro Yamada]

After the allmodconfig build, this script takes about 5 sec on my
machine. Most of the run-time is consumed for needless regex matching.

We know the format of .*.cmd file; the first line is the build command.
There is no need to parse the rest.

With this optimization, now it runs 4 times faster.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
---

Changes in v2:
  - Remove the unneeded variable 'line'

 scripts/gen_compile_commands.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index c458696ef3a7..1bcf33a93cb9 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -125,11 +125,8 @@ def main():
             filepath = os.path.join(dirpath, filename)
 
             with open(filepath, 'rt') as f:
-                for line in f:
-                    result = line_matcher.match(line)
-                    if not result:
-                        continue
-
+                result = line_matcher.match(f.readline())
+                if result:
                     try:
                         entry = process_line(directory, dirpath,
                                              result.group(1), result.group(2))
-- 
2.25.1

[PATCH v2 2/9] gen_compile_commands: use choices for --log_levels option

[Masahiro Yamada]

Use 'choices' instead of the own code to check if the given parameter
is valid.

I also simplified the help message because, with 'choices', --help
shows the list of valid parameters:

  --log_level {DEBUG,INFO,WARNING,ERROR,CRITICAL}

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - New patch

 scripts/gen_compile_commands.py | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 1bcf33a93cb9..535248cf2d7e 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -45,24 +45,18 @@ def parse_arguments():
                    'compile_commands.json in the search directory)')
     parser.add_argument('-o', '--output', type=str, help=output_help)
 
-    log_level_help = ('The level of log messages to produce (one of ' +
-                      ', '.join(_VALID_LOG_LEVELS) + '; defaults to ' +
+    log_level_help = ('the level of log messages to produce (defaults to ' +
                       _DEFAULT_LOG_LEVEL + ')')
-    parser.add_argument(
-        '--log_level', type=str, default=_DEFAULT_LOG_LEVEL,
-        help=log_level_help)
+    parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
+                        default=_DEFAULT_LOG_LEVEL, help=log_level_help)
 
     args = parser.parse_args()
 
-    log_level = args.log_level
-    if log_level not in _VALID_LOG_LEVELS:
-        raise ValueError('%s is not a valid log level' % log_level)
-
     directory = args.directory or os.getcwd()
     output = args.output or os.path.join(directory, _DEFAULT_OUTPUT)
     directory = os.path.abspath(directory)
 
-    return log_level, directory, output
+    return args.log_level, directory, output
 
 
 def process_line(root_directory, file_directory, command_prefix, relative_path):
-- 
2.25.1

[PATCH v2 3/9] gen_compile_commands: do not support .cmd files under tools/ directory

[Masahiro Yamada]

The tools/ directory uses a different build system, and the format of
.cmd files is different because the tools builds run in a different
work directory.

Supporting two formats compilicates the script.

The only loss by this change is objtool.

Also, rename the confusing variable 'relative_path' because it is
not necessarily a relative path. When the output directory is not
the direct child of the source tree (e.g. O=foo/bar), it is an
absolute path. Rename it to 'file_path'.

os.path.join(root_directory, file_path) works whether the file_path
is relative or not. If file_path is already absolute, it returns it
as-is.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - New patch

 scripts/gen_compile_commands.py | 31 +++++++++++--------------------
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 535248cf2d7e..1b9899892d99 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -59,23 +59,21 @@ def parse_arguments():
     return args.log_level, directory, output
 
 
-def process_line(root_directory, file_directory, command_prefix, relative_path):
+def process_line(root_directory, command_prefix, file_path):
     """Extracts information from a .cmd line and creates an entry from it.
 
     Args:
         root_directory: The directory that was searched for .cmd files. Usually
             used directly in the "directory" entry in compile_commands.json.
-        file_directory: The path to the directory the .cmd file was found in.
         command_prefix: The extracted command line, up to the last element.
-        relative_path: The .c file from the end of the extracted command.
-            Usually relative to root_directory, but sometimes relative to
-            file_directory and sometimes neither.
+        file_path: The .c file from the end of the extracted command.
+            Usually relative to root_directory, but sometimes absolute.
 
     Returns:
         An entry to append to compile_commands.
 
     Raises:
-        ValueError: Could not find the extracted file based on relative_path and
+        ValueError: Could not find the extracted file based on file_path and
             root_directory or file_directory.
     """
     # The .cmd files are intended to be included directly by Make, so they
@@ -84,20 +82,13 @@ def process_line(root_directory, file_directory, command_prefix, relative_path):
     # by Make, so this code replaces the escaped version with '#'.
     prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
 
-    cur_dir = root_directory
-    expected_path = os.path.join(cur_dir, relative_path)
-    if not os.path.exists(expected_path):
-        # Try using file_directory instead. Some of the tools have a different
-        # style of .cmd file than the kernel.
-        cur_dir = file_directory
-        expected_path = os.path.join(cur_dir, relative_path)
-        if not os.path.exists(expected_path):
-            raise ValueError('File %s not in %s or %s' %
-                             (relative_path, root_directory, file_directory))
+    abs_path = os.path.abspath(os.path.join(root_directory, file_path))
+    if not os.path.exists(abs_path):
+        raise ValueError('File %s not found' % abs_path)
     return {
-        'directory': cur_dir,
-        'file': relative_path,
-        'command': prefix + relative_path,
+        'directory': root_directory,
+        'file': abs_path,
+        'command': prefix + file_path,
     }
 
 
@@ -122,7 +113,7 @@ def main():
                 result = line_matcher.match(f.readline())
                 if result:
                     try:
-                        entry = process_line(directory, dirpath,
+                        entry = process_line(directory,
                                              result.group(1), result.group(2))
                         compile_commands.append(entry)
                     except ValueError as err:
-- 
2.25.1

[PATCH v2 4/9] gen_compile_commands: reword the help message of -d option

[Masahiro Yamada]

I think the help message of the -d option is somewhat misleading.

  Path to the kernel source directory to search (defaults to the working directory)

The part "kernel source directory" is the source of the confusion.
Some people misunderstand as if this script did not support separate
output directories.

Actually, this script also works for out-of-tree builds. You can
use the -d option to point to the object output directory, not to
the source directory. It should match to the O= option used in the
previous kernel build, and then appears in the "directory" field of
compile_commands.json.

Reword the help message.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - New patch

 scripts/gen_compile_commands.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 1b9899892d99..5f6318da01a2 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -31,13 +31,13 @@ def parse_arguments():
 
     Returns:
         log_level: A logging level to filter log output.
-        directory: The directory to search for .cmd files.
+        directory: The work directory where the objects were built
         output: Where to write the compile-commands JSON file.
     """
     usage = 'Creates a compile_commands.json database from kernel .cmd files'
     parser = argparse.ArgumentParser(description=usage)
 
-    directory_help = ('Path to the kernel source directory to search '
+    directory_help = ('specify the output directory used for the kernel build '
                       '(defaults to the working directory)')
     parser.add_argument('-d', '--directory', type=str, help=directory_help)
 
-- 
2.25.1

[PATCH v2 5/9] gen_compile_commands: make -o option independent of -d option

[Masahiro Yamada]

Change the -o option independent of the -d option, which is I think
clearer behavior. Some people may like to use -d to specify a separate
output directory, but still output the compile_commands.py in the
source directory (unless the source tree is read-only) because it is
the default location Clang Tools search for the compilation database.

Also, move the default parameter to the default= argument of the
.add_argument().

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - New patch

 scripts/gen_compile_commands.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 5f6318da01a2..3ed958b64658 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -39,11 +39,13 @@ def parse_arguments():
 
     directory_help = ('specify the output directory used for the kernel build '
                       '(defaults to the working directory)')
-    parser.add_argument('-d', '--directory', type=str, help=directory_help)
+    parser.add_argument('-d', '--directory', type=str, default='.',
+                        help=directory_help)
 
-    output_help = ('The location to write compile_commands.json (defaults to '
-                   'compile_commands.json in the search directory)')
-    parser.add_argument('-o', '--output', type=str, help=output_help)
+    output_help = ('path to the output command database (defaults to ' +
+                   _DEFAULT_OUTPUT + ')')
+    parser.add_argument('-o', '--output', type=str, default=_DEFAULT_OUTPUT,
+                        help=output_help)
 
     log_level_help = ('the level of log messages to produce (defaults to ' +
                       _DEFAULT_LOG_LEVEL + ')')
@@ -52,11 +54,9 @@ def parse_arguments():
 
     args = parser.parse_args()
 
-    directory = args.directory or os.getcwd()
-    output = args.output or os.path.join(directory, _DEFAULT_OUTPUT)
-    directory = os.path.abspath(directory)
-
-    return args.log_level, directory, output
+    return (args.log_level,
+            os.path.abspath(args.directory),
+            args.output)
 
 
 def process_line(root_directory, command_prefix, file_path):
-- 
2.25.1

[PATCH v2 6/9] gen_compile_commands: move directory walk to a generator function

[Masahiro Yamada]

Currently, this script walks under the specified directory (default to
the current directory), then parses all .cmd files found.

Split it into a separate helper function because the next commit will
add more helpers to pick up .cmd files associated with given file(s).

There is no point to build and return a huge list at once. I used a
generator so it works in the for-loop with less memory.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - New patch

 scripts/gen_compile_commands.py | 44 ++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 3ed958b64658..6dec7e2c4098 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -33,6 +33,7 @@ def parse_arguments():
         log_level: A logging level to filter log output.
         directory: The work directory where the objects were built
         output: Where to write the compile-commands JSON file.
+        paths: The list of directories to handle to find .cmd files
     """
     usage = 'Creates a compile_commands.json database from kernel .cmd files'
     parser = argparse.ArgumentParser(description=usage)
@@ -56,7 +57,28 @@ def parse_arguments():
 
     return (args.log_level,
             os.path.abspath(args.directory),
-            args.output)
+            args.output,
+            [args.directory])
+
+
+def cmdfiles_in_dir(directory):
+    """Generate the iterator of .cmd files found under the directory.
+
+    Walk under the given directory, and yield every .cmd file found.
+
+    Args:
+        directory: The directory to search for .cmd files.
+
+    Yields:
+        The path to a .cmd file.
+    """
+
+    filename_matcher = re.compile(_FILENAME_PATTERN)
+
+    for dirpath, _, filenames in os.walk(directory):
+        for filename in filenames:
+            if filename_matcher.match(filename):
+                yield os.path.join(dirpath, filename)
 
 
 def process_line(root_directory, command_prefix, file_path):
@@ -94,31 +116,29 @@ def process_line(root_directory, command_prefix, file_path):
 
 def main():
     """Walks through the directory and finds and parses .cmd files."""
-    log_level, directory, output = parse_arguments()
+    log_level, directory, output, paths = parse_arguments()
 
     level = getattr(logging, log_level)
     logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
 
-    filename_matcher = re.compile(_FILENAME_PATTERN)
     line_matcher = re.compile(_LINE_PATTERN)
 
     compile_commands = []
-    for dirpath, _, filenames in os.walk(directory):
-        for filename in filenames:
-            if not filename_matcher.match(filename):
-                continue
-            filepath = os.path.join(dirpath, filename)
 
-            with open(filepath, 'rt') as f:
+    for path in paths:
+        cmdfiles = cmdfiles_in_dir(path)
+
+        for cmdfile in cmdfiles:
+            with open(cmdfile, 'rt') as f:
                 result = line_matcher.match(f.readline())
                 if result:
                     try:
-                        entry = process_line(directory,
-                                             result.group(1), result.group(2))
+                        entry = process_line(directory, result.group(1),
+                                             result.group(2))
                         compile_commands.append(entry)
                     except ValueError as err:
                         logging.info('Could not add line from %s: %s',
-                                     filepath, err)
+                                     cmdfile, err)
 
     with open(output, 'wt') as f:
         json.dump(compile_commands, f, indent=2, sort_keys=True)
-- 
2.25.1

[PATCH v2 7/9] gen_compile_commands: support *.o, *.a, modules.order in positional argument

[Masahiro Yamada]

This script currently searches the specified directory for .cmd files.
One drawback is it may contain stale .cmd files after you rebuild the
kernel several times without 'make clean'.

This commit supports *.o, *.a, and modules.order as positional
parameters. If such files are given, they are parsed to collect
associated .cmd files. I added a generator helper for each of them.

This feature is useful to get the list of active .cmd files from the
last build, and will be used by the next commit to wire up the
compile_commands.json rule to the Makefile.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

Changes in v2:
  - Separate the file parser into generator functions
  - Use 'obj' instead of 'object' because 'object' is a built-in function
  - I think using 'file' is OK because it is not a built-in function in Python3
    (https://docs.python.org/3/library/functions.html)
    Anyway, the variable 'file' is no longer used in this version
  - Keep the previous work-flow to allow to search the given directory

 scripts/gen_compile_commands.py | 100 ++++++++++++++++++++++++++++++--
 1 file changed, 96 insertions(+), 4 deletions(-)

diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
index 6dec7e2c4098..65859e6044b5 100755
--- a/scripts/gen_compile_commands.py
+++ b/scripts/gen_compile_commands.py
@@ -12,6 +12,7 @@ import json
 import logging
 import os
 import re
+import subprocess
 
 _DEFAULT_OUTPUT = 'compile_commands.json'
 _DEFAULT_LOG_LEVEL = 'WARNING'
@@ -32,8 +33,9 @@ def parse_arguments():
     Returns:
         log_level: A logging level to filter log output.
         directory: The work directory where the objects were built
+        ar: Command used for parsing .a archives
         output: Where to write the compile-commands JSON file.
-        paths: The list of directories to handle to find .cmd files
+        paths: The list of files/directories to handle to find .cmd files
     """
     usage = 'Creates a compile_commands.json database from kernel .cmd files'
     parser = argparse.ArgumentParser(description=usage)
@@ -53,12 +55,21 @@ def parse_arguments():
     parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
                         default=_DEFAULT_LOG_LEVEL, help=log_level_help)
 
+    ar_help = 'command used for parsing .a archives'
+    parser.add_argument('-a', '--ar', type=str, default='ar', help=ar_help)
+
+    paths_help = ('directories to search or files to parse '
+                  '(files should be *.o, *.a, or modules.order). '
+                  'If nothing is specified, the current directory is searched')
+    parser.add_argument('paths', type=str, nargs='*', help=paths_help)
+
     args = parser.parse_args()
 
     return (args.log_level,
             os.path.abspath(args.directory),
             args.output,
-            [args.directory])
+            args.ar,
+            args.paths if len(args.paths) > 0 else [args.directory])
 
 
 def cmdfiles_in_dir(directory):
@@ -81,6 +92,73 @@ def cmdfiles_in_dir(directory):
                 yield os.path.join(dirpath, filename)
 
 
+def to_cmdfile(path):
+    """Return the path of .cmd file used for the given build artifact
+
+    Args:
+        Path: file path
+
+    Returns:
+        The path to .cmd file
+    """
+    dir, base = os.path.split(path)
+    return os.path.join(dir, '.' + base + '.cmd')
+
+
+def cmdfiles_for_o(obj):
+    """Generate the iterator of .cmd files associated with the object
+
+    Yield the .cmd file used to build the given object
+
+    Args:
+        obj: The object path
+
+    Yields:
+        The path to .cmd file
+    """
+    yield to_cmdfile(obj)
+
+
+def cmdfiles_for_a(archive, ar):
+    """Generate the iterator of .cmd files associated with the archive.
+
+    Parse the given archive, and yield every .cmd file used to build it.
+
+    Args:
+        archive: The archive to parse
+
+    Yields:
+        The path to every .cmd file found
+    """
+    for obj in subprocess.check_output([ar, '-t', archive]).decode().split():
+        yield to_cmdfile(obj)
+
+
+def cmdfiles_for_modorder(modorder):
+    """Generate the iterator of .cmd files associated with the modules.order.
+
+    Parse the given modules.order, and yield every .cmd file used to build the
+    contained modules.
+
+    Args:
+        modorder: The modules.order file to parse
+
+    Yields:
+        The path to every .cmd file found
+    """
+    with open(modorder) as f:
+        for line in f:
+            ko = line.rstrip()
+            base, ext = os.path.splitext(ko)
+            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.
+            with open(mod) as m:
+                for obj in m.readline().split():
+                    yield to_cmdfile(obj)
+
+
 def process_line(root_directory, command_prefix, file_path):
     """Extracts information from a .cmd line and creates an entry from it.
 
@@ -116,7 +194,7 @@ def process_line(root_directory, command_prefix, file_path):
 
 def main():
     """Walks through the directory and finds and parses .cmd files."""
-    log_level, directory, output, paths = parse_arguments()
+    log_level, directory, output, ar, paths = parse_arguments()
 
     level = getattr(logging, log_level)
     logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
@@ -126,7 +204,21 @@ def main():
     compile_commands = []
 
     for path in paths:
-        cmdfiles = cmdfiles_in_dir(path)
+        # If 'path' is a directory, handle all .cmd files under it.
+        # Otherwise, handle .cmd files associated with the file.
+        # Most of built-in objects are linked via archives (built-in.a or lib.a)
+        # but some are linked to vmlinux directly.
+        # Modules are lis
+        if os.path.isdir(path):
+            cmdfiles = cmdfiles_in_dir(path)
+        elif path.endswith('.o'):
+            cmdfiles = cmdfiles_for_o(path)
+        elif path.endswith('.a'):
+            cmdfiles = cmdfiles_for_a(path, ar)
+        elif path.endswith('modules.order'):
+            cmdfiles = cmdfiles_for_modorder(path)
+        else:
+            sys.exit('{}: unknown file type'.format(path))
 
         for cmdfile in cmdfiles:
             with open(cmdfile, 'rt') as f:
-- 
2.25.1

[PATCH v2 8/9] kbuild: wire up the build rule of compile_commands.json to Makefile

[Masahiro Yamada]

Currently, you need to manually run scripts/gen_compile_commands.py
to create compile_commands.json. It parses all the .*.cmd files found
under the specified directory.

If you rebuild the kernel over again without 'make clean',
.*.cmd files from older builds will create stale entries in
compile_commands.json.

This commit wires up the compile_commands.json rule to Makefile, and
makes it parse only the .*.cmd files involved in the current build.

Pass $(KBUILD_VMLINUX_OBJS), $(KBUILD_VMLINUX_LIBS), and modules.order
to the script. The objects or archives linked to vmlinux are listed in
$(KBUILD_VMLINUX_OBJS) or $(KBUILD_VMLINUX_LIBS). All the modules are
listed in modules.order.

You can create compile_commands.json from Make:

  $ make -j$(nproc) CC=clang compile_commands.json

You can also build vmlinux, modules, and compile_commands.json all
together in a single command:

  $ make -j$(nproc) CC=clang all compile_commands.json

It works for M= builds as well. In this case, compile_commands.json
is created in the top directory of the external module.

This is convenient, but it has a drawback; the coverage of the
compile_commands.json is reduced because only the objects linked to
vmlinux or modules are handled. For example, the following C files are
not included in the compile_commands.json:

 - Decompressor source files (arch/*/boot/)
 - VDSO source files
 - C files used to generate intermediates (e.g. kernel/bounds.c)
 - Standalone host programs

I think it is fine for most developers because our main interest is
the kernel-space code.

If you want to cover all the compiled C files, please build the kernel
then run the script manually as before:

  $ make clean    # if you want to delete stale .cmd files [optional]
  $ make -j$(nproc) CC=clang
  $ scripts/gen_compile_commands.json

Here is a note for out-of-tree builds. 'make compile_commands.json'
works with O= option, but please notice compile_commands.json is
created in the object tree instead of the source tree.

Some people may want to have compile_commands.json in the source tree
because Clang Tools searches for it through all parent paths of the
first input source file.

However, you cannot do this for O= builds. Kbuild should never generate
any build artifact in the source tree when O= is given because the
source tree might be read-only. Any write attempt to the source tree
is monitored and the violation may be reported. See the commit log of
8ef14c2c41d9.

So, the only possible way is to create compile_commands.json in the
object tree, then specify '-p <build-path>' when you use clang-check,
clang-tidy, etc.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 Makefile | 29 +++++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 9cac6fde3479..65ed336a6de1 100644
--- a/Makefile
+++ b/Makefile
@@ -635,7 +635,7 @@ endif
 # in addition to whatever we do anyway.
 # Just "make" or "make all" shall build modules as well
 
-ifneq ($(filter all modules nsdeps,$(MAKECMDGOALS)),)
+ifneq ($(filter all modules nsdeps %compile_commands.json,$(MAKECMDGOALS)),)
   KBUILD_MODULES := 1
 endif
 
@@ -1464,7 +1464,8 @@ endif # CONFIG_MODULES
 
 # Directories & files removed with 'make clean'
 CLEAN_FILES += include/ksym vmlinux.symvers \
-	       modules.builtin modules.builtin.modinfo modules.nsdeps
+	       modules.builtin modules.builtin.modinfo modules.nsdeps \
+	       compile_commands.json
 
 # Directories & files removed with 'make mrproper'
 MRPROPER_FILES += include/config include/generated          \
@@ -1698,9 +1699,12 @@ KBUILD_MODULES := 1
 
 build-dirs := $(KBUILD_EXTMOD)
 PHONY += modules
-modules: descend
+modules: $(MODORDER)
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
 
+$(MODORDER): descend
+	@:
+
 PHONY += modules_install
 modules_install: _emodinst_ _emodinst_post
 
@@ -1714,8 +1718,12 @@ PHONY += _emodinst_post
 _emodinst_post: _emodinst_
 	$(call cmd,depmod)
 
+compile_commands.json: $(extmod-prefix)compile_commands.json
+PHONY += compile_commands.json
+
 clean-dirs := $(KBUILD_EXTMOD)
-clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps
+clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps \
+	$(KBUILD_EXTMOD)/compile_commands.json
 
 PHONY += help
 help:
@@ -1828,6 +1836,19 @@ nsdeps: export KBUILD_NSDEPS=1
 nsdeps: modules
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
 
+# Clang Tooling
+# ---------------------------------------------------------------------------
+
+quiet_cmd_gen_compile_commands = GEN     $@
+      cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, $(real-prereqs))
+
+$(extmod-prefix)compile_commands.json: scripts/gen_compile_commands.py \
+	$(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
+	$(if $(CONFIG_MODULES), $(MODORDER)) FORCE
+	$(call if_changed,gen_compile_commands)
+
+targets += $(extmod-prefix)compile_commands.json
+
 # Scripts to check various things for consistency
 # ---------------------------------------------------------------------------
 
-- 
2.25.1

[PATCH v2 9/9] Makefile: Add clang-tidy and static analyzer support to makefile

[Masahiro Yamada]

From: Nathan Huckleberry <nhuck@google.com>

This patch adds clang-tidy and the clang static-analyzer as make
targets. The goal of this patch is to make static analysis tools
usable and extendable by any developer or researcher who is familiar
with basic c++.

The current static analysis tools require intimate knowledge of the
internal workings of the static analysis. Clang-tidy and the clang
static analyzers expose an easy to use api and allow users unfamiliar
with clang to write new checks with relative ease.

===Clang-tidy===

Clang-tidy is an easily extendable 'linter' that runs on the AST.
Clang-tidy checks are easy to write and understand. A check consists of
two parts, a matcher and a checker. The matcher is created using a
domain specific language that acts on the AST
(https://clang.llvm.org/docs/LibASTMatchersReference.html).  When AST
nodes are found by the matcher a callback is made to the checker. The
checker can then execute additional checks and issue warnings.

Here is an example clang-tidy check to report functions that have calls
to local_irq_disable without calls to local_irq_enable and vice-versa.
Functions flagged with __attribute((annotation("ignore_irq_balancing")))
are ignored for analysis. (https://reviews.llvm.org/D65828)

===Clang static analyzer===

The clang static analyzer is a more powerful static analysis tool that
uses symbolic execution to find bugs. Currently there is a check that
looks for potential security bugs from invalid uses of kmalloc and
kfree. There are several more general purpose checks that are useful for
the kernel.

The clang static analyzer is well documented and designed to be
extensible.
(https://clang-analyzer.llvm.org/checker_dev_manual.html)
(https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf)

The main draw of the clang tools is how accessible they are. The clang
documentation is very nice and these tools are built specifically to be
easily extendable by any developer. They provide an accessible method of
bug-finding and research to people who are not overly familiar with the
kernel codebase.

Signed-off-by: Nathan Huckleberry <nhuck@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

(no changes since v1)

 MAINTAINERS                                   |  1 +
 Makefile                                      | 20 ++++-
 .../{ => clang-tools}/gen_compile_commands.py |  0
 scripts/clang-tools/run-clang-tools.py        | 74 +++++++++++++++++++
 4 files changed, 93 insertions(+), 2 deletions(-)
 rename scripts/{ => clang-tools}/gen_compile_commands.py (100%)
 create mode 100755 scripts/clang-tools/run-clang-tools.py

diff --git a/MAINTAINERS b/MAINTAINERS
index deaafb617361..19b916dbc796 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4247,6 +4247,7 @@ W:	https://clangbuiltlinux.github.io/
 B:	https://github.com/ClangBuiltLinux/linux/issues
 C:	irc://chat.freenode.net/clangbuiltlinux
 F:	Documentation/kbuild/llvm.rst
+F:	scripts/clang-tools/
 K:	\b(?i:clang|llvm)\b
 
 CLEANCACHE API
diff --git a/Makefile b/Makefile
index 65ed336a6de1..9ece191d8d51 100644
--- a/Makefile
+++ b/Makefile
@@ -635,7 +635,7 @@ endif
 # in addition to whatever we do anyway.
 # Just "make" or "make all" shall build modules as well
 
-ifneq ($(filter all modules nsdeps %compile_commands.json,$(MAKECMDGOALS)),)
+ifneq ($(filter all modules nsdeps %compile_commands.json clang-%,$(MAKECMDGOALS)),)
   KBUILD_MODULES := 1
 endif
 
@@ -1577,6 +1577,8 @@ help:
 	@echo  '  export_report   - List the usages of all exported symbols'
 	@echo  '  headerdep       - Detect inclusion cycles in headers'
 	@echo  '  coccicheck      - Check with Coccinelle'
+	@echo  '  clang-analyzer  - Check with clang static analyzer'
+	@echo  '  clang-tidy      - Check with clang-tidy'
 	@echo  ''
 	@echo  'Tools:'
 	@echo  '  nsdeps          - Generate missing symbol namespace dependencies'
@@ -1842,13 +1844,27 @@ nsdeps: modules
 quiet_cmd_gen_compile_commands = GEN     $@
       cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, $(real-prereqs))
 
-$(extmod-prefix)compile_commands.json: scripts/gen_compile_commands.py \
+$(extmod-prefix)compile_commands.json: scripts/clang-tools/gen_compile_commands.py \
 	$(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
 	$(if $(CONFIG_MODULES), $(MODORDER)) FORCE
 	$(call if_changed,gen_compile_commands)
 
 targets += $(extmod-prefix)compile_commands.json
 
+PHONY += clang-tidy clang-analyzer
+
+ifdef CONFIG_CC_IS_CLANG
+quiet_cmd_clang_tools = CHECK   $<
+      cmd_clang_tools = $(PYTHON3) $(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<
+
+clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
+	$(call cmd,clang_tools)
+else
+clang-tidy clang-analyzer:
+	@echo "$@ requires CC=clang" >&2
+	@false
+endif
+
 # Scripts to check various things for consistency
 # ---------------------------------------------------------------------------
 
diff --git a/scripts/gen_compile_commands.py b/scripts/clang-tools/gen_compile_commands.py
similarity index 100%
rename from scripts/gen_compile_commands.py
rename to scripts/clang-tools/gen_compile_commands.py
diff --git a/scripts/clang-tools/run-clang-tools.py b/scripts/clang-tools/run-clang-tools.py
new file mode 100755
index 000000000000..fa7655c7cec0
--- /dev/null
+++ b/scripts/clang-tools/run-clang-tools.py
@@ -0,0 +1,74 @@
+#!/usr/bin/env python
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (C) Google LLC, 2020
+#
+# Author: Nathan Huckleberry <nhuck@google.com>
+#
+"""A helper routine run clang-tidy and the clang static-analyzer on
+compile_commands.json.
+"""
+
+import argparse
+import json
+import multiprocessing
+import os
+import subprocess
+import sys
+
+
+def parse_arguments():
+    """Set up and parses command-line arguments.
+    Returns:
+        args: Dict of parsed args
+        Has keys: [path, type]
+    """
+    usage = """Run clang-tidy or the clang static-analyzer on a
+        compilation database."""
+    parser = argparse.ArgumentParser(description=usage)
+
+    type_help = "Type of analysis to be performed"
+    parser.add_argument("type",
+                        choices=["clang-tidy", "clang-analyzer"],
+                        help=type_help)
+    path_help = "Path to the compilation database to parse"
+    parser.add_argument("path", type=str, help=path_help)
+
+    return parser.parse_args()
+
+
+def init(l, a):
+    global lock
+    global args
+    lock = l
+    args = a
+
+
+def run_analysis(entry):
+    # Disable all checks, then re-enable the ones we want
+    checks = "-checks=-*,"
+    if args.type == "clang-tidy":
+        checks += "linuxkernel-*"
+    else:
+        checks += "clang-analyzer-*"
+    p = subprocess.run(["clang-tidy", "-p", args.path, checks, entry["file"]],
+                       stdout=subprocess.PIPE,
+                       stderr=subprocess.STDOUT,
+                       cwd=entry["directory"])
+    with lock:
+        sys.stderr.buffer.write(p.stdout)
+
+
+def main():
+    args = parse_arguments()
+
+    lock = multiprocessing.Lock()
+    pool = multiprocessing.Pool(initializer=init, initargs=(lock, args))
+    # Read JSON data into the datastore variable
+    with open(args.path, "r") as f:
+        datastore = json.load(f)
+        pool.map(run_analysis, datastore)
+
+
+if __name__ == "__main__":
+    main()
-- 
2.25.1

Re: [PATCH v2 2/9] gen_compile_commands: use choices for --log_levels option

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Use 'choices' instead of the own code to check if the given parameter
> is valid.
>
> I also simplified the help message because, with 'choices', --help
> shows the list of valid parameters:
>
>   --log_level {DEBUG,INFO,WARNING,ERROR,CRITICAL}
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/gen_compile_commands.py | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 1bcf33a93cb9..535248cf2d7e 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -45,24 +45,18 @@ def parse_arguments():
>                     'compile_commands.json in the search directory)')
>      parser.add_argument('-o', '--output', type=str, help=output_help)
>
> -    log_level_help = ('The level of log messages to produce (one of ' +
> -                      ', '.join(_VALID_LOG_LEVELS) + '; defaults to ' +
> +    log_level_help = ('the level of log messages to produce (defaults to ' +
>                        _DEFAULT_LOG_LEVEL + ')')
> -    parser.add_argument(
> -        '--log_level', type=str, default=_DEFAULT_LOG_LEVEL,
> -        help=log_level_help)
> +    parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
> +                        default=_DEFAULT_LOG_LEVEL, help=log_level_help)
>
>      args = parser.parse_args()
>
> -    log_level = args.log_level
> -    if log_level not in _VALID_LOG_LEVELS:
> -        raise ValueError('%s is not a valid log level' % log_level)
> -
>      directory = args.directory or os.getcwd()
>      output = args.output or os.path.join(directory, _DEFAULT_OUTPUT)
>      directory = os.path.abspath(directory)
>
> -    return log_level, directory, output
> +    return args.log_level, directory, output
>
>
>  def process_line(root_directory, file_directory, command_prefix, relative_path):
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 3/9] gen_compile_commands: do not support .cmd files under tools/ directory

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The tools/ directory uses a different build system, and the format of
> .cmd files is different because the tools builds run in a different
> work directory.
>
> Supporting two formats compilicates the script.
>
> The only loss by this change is objtool.
>
> Also, rename the confusing variable 'relative_path' because it is
> not necessarily a relative path. When the output directory is not
> the direct child of the source tree (e.g. O=foo/bar), it is an
> absolute path. Rename it to 'file_path'.
>
> os.path.join(root_directory, file_path) works whether the file_path
> is relative or not. If file_path is already absolute, it returns it
> as-is.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/gen_compile_commands.py | 31 +++++++++++--------------------
>  1 file changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 535248cf2d7e..1b9899892d99 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -59,23 +59,21 @@ def parse_arguments():
>      return args.log_level, directory, output
>
>
> -def process_line(root_directory, file_directory, command_prefix, relative_path):
> +def process_line(root_directory, command_prefix, file_path):
>      """Extracts information from a .cmd line and creates an entry from it.
>
>      Args:
>          root_directory: The directory that was searched for .cmd files. Usually
>              used directly in the "directory" entry in compile_commands.json.
> -        file_directory: The path to the directory the .cmd file was found in.
>          command_prefix: The extracted command line, up to the last element.
> -        relative_path: The .c file from the end of the extracted command.
> -            Usually relative to root_directory, but sometimes relative to
> -            file_directory and sometimes neither.
> +        file_path: The .c file from the end of the extracted command.
> +            Usually relative to root_directory, but sometimes absolute.
>
>      Returns:
>          An entry to append to compile_commands.
>
>      Raises:
> -        ValueError: Could not find the extracted file based on relative_path and
> +        ValueError: Could not find the extracted file based on file_path and
>              root_directory or file_directory.
>      """
>      # The .cmd files are intended to be included directly by Make, so they
> @@ -84,20 +82,13 @@ def process_line(root_directory, file_directory, command_prefix, relative_path):
>      # by Make, so this code replaces the escaped version with '#'.
>      prefix = command_prefix.replace('\#', '#').replace('$(pound)', '#')
>
> -    cur_dir = root_directory
> -    expected_path = os.path.join(cur_dir, relative_path)
> -    if not os.path.exists(expected_path):
> -        # Try using file_directory instead. Some of the tools have a different
> -        # style of .cmd file than the kernel.
> -        cur_dir = file_directory
> -        expected_path = os.path.join(cur_dir, relative_path)
> -        if not os.path.exists(expected_path):
> -            raise ValueError('File %s not in %s or %s' %
> -                             (relative_path, root_directory, file_directory))
> +    abs_path = os.path.abspath(os.path.join(root_directory, file_path))
> +    if not os.path.exists(abs_path):
> +        raise ValueError('File %s not found' % abs_path)
>      return {
> -        'directory': cur_dir,
> -        'file': relative_path,
> -        'command': prefix + relative_path,
> +        'directory': root_directory,
> +        'file': abs_path,
> +        'command': prefix + file_path,
>      }
>
>
> @@ -122,7 +113,7 @@ def main():
>                  result = line_matcher.match(f.readline())
>                  if result:
>                      try:
> -                        entry = process_line(directory, dirpath,
> +                        entry = process_line(directory,
>                                               result.group(1), result.group(2))
>                          compile_commands.append(entry)
>                      except ValueError as err:
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 4/9] gen_compile_commands: reword the help message of -d option

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I think the help message of the -d option is somewhat misleading.
>
>   Path to the kernel source directory to search (defaults to the working directory)
>
> The part "kernel source directory" is the source of the confusion.
> Some people misunderstand as if this script did not support separate
> output directories.
>
> Actually, this script also works for out-of-tree builds. You can
> use the -d option to point to the object output directory, not to
> the source directory. It should match to the O= option used in the
> previous kernel build, and then appears in the "directory" field of
> compile_commands.json.
>
> Reword the help message.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/gen_compile_commands.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 1b9899892d99..5f6318da01a2 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -31,13 +31,13 @@ def parse_arguments():
>
>      Returns:
>          log_level: A logging level to filter log output.
> -        directory: The directory to search for .cmd files.
> +        directory: The work directory where the objects were built

Punctuation (add a period `.`).

>          output: Where to write the compile-commands JSON file.
>      """
>      usage = 'Creates a compile_commands.json database from kernel .cmd files'
>      parser = argparse.ArgumentParser(description=usage)
>
> -    directory_help = ('Path to the kernel source directory to search '
> +    directory_help = ('specify the output directory used for the kernel build '

Capitalization (specify -> Specify)

With that:
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>                        '(defaults to the working directory)')
>      parser.add_argument('-d', '--directory', type=str, help=directory_help)
>
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 5/9] gen_compile_commands: make -o option independent of -d option

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Change the -o option independent of the -d option, which is I think
> clearer behavior. Some people may like to use -d to specify a separate
> output directory, but still output the compile_commands.py in the
> source directory (unless the source tree is read-only) because it is
> the default location Clang Tools search for the compilation database.
>
> Also, move the default parameter to the default= argument of the
> .add_argument().
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/gen_compile_commands.py | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 5f6318da01a2..3ed958b64658 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -39,11 +39,13 @@ def parse_arguments():
>
>      directory_help = ('specify the output directory used for the kernel build '
>                        '(defaults to the working directory)')
> -    parser.add_argument('-d', '--directory', type=str, help=directory_help)
> +    parser.add_argument('-d', '--directory', type=str, default='.',
> +                        help=directory_help)
>
> -    output_help = ('The location to write compile_commands.json (defaults to '
> -                   'compile_commands.json in the search directory)')
> -    parser.add_argument('-o', '--output', type=str, help=output_help)
> +    output_help = ('path to the output command database (defaults to ' +
> +                   _DEFAULT_OUTPUT + ')')
> +    parser.add_argument('-o', '--output', type=str, default=_DEFAULT_OUTPUT,
> +                        help=output_help)
>
>      log_level_help = ('the level of log messages to produce (defaults to ' +
>                        _DEFAULT_LOG_LEVEL + ')')
> @@ -52,11 +54,9 @@ def parse_arguments():
>
>      args = parser.parse_args()
>
> -    directory = args.directory or os.getcwd()
> -    output = args.output or os.path.join(directory, _DEFAULT_OUTPUT)
> -    directory = os.path.abspath(directory)
> -
> -    return args.log_level, directory, output
> +    return (args.log_level,
> +            os.path.abspath(args.directory),
> +            args.output)
>
>
>  def process_line(root_directory, command_prefix, file_path):
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 6/9] gen_compile_commands: move directory walk to a generator function

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, this script walks under the specified directory (default to
> the current directory), then parses all .cmd files found.
>
> Split it into a separate helper function because the next commit will
> add more helpers to pick up .cmd files associated with given file(s).
>
> There is no point to build and return a huge list at once. I used a
> generator so it works in the for-loop with less memory.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - New patch
>
>  scripts/gen_compile_commands.py | 44 ++++++++++++++++++++++++---------
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 3ed958b64658..6dec7e2c4098 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -33,6 +33,7 @@ def parse_arguments():
>          log_level: A logging level to filter log output.
>          directory: The work directory where the objects were built
>          output: Where to write the compile-commands JSON file.
> +        paths: The list of directories to handle to find .cmd files

Punctuation: please add a period.

>      """
>      usage = 'Creates a compile_commands.json database from kernel .cmd files'
>      parser = argparse.ArgumentParser(description=usage)
> @@ -56,7 +57,28 @@ def parse_arguments():
>
>      return (args.log_level,
>              os.path.abspath(args.directory),
> -            args.output)
> +            args.output,
> +            [args.directory])
> +
> +
> +def cmdfiles_in_dir(directory):
> +    """Generate the iterator of .cmd files found under the directory.
> +
> +    Walk under the given directory, and yield every .cmd file found.
> +
> +    Args:
> +        directory: The directory to search for .cmd files.
> +
> +    Yields:
> +        The path to a .cmd file.
> +    """
> +
> +    filename_matcher = re.compile(_FILENAME_PATTERN)
> +
> +    for dirpath, _, filenames in os.walk(directory):
> +        for filename in filenames:
> +            if filename_matcher.match(filename):
> +                yield os.path.join(dirpath, filename)
>
>
>  def process_line(root_directory, command_prefix, file_path):
> @@ -94,31 +116,29 @@ def process_line(root_directory, command_prefix, file_path):
>
>  def main():
>      """Walks through the directory and finds and parses .cmd files."""
> -    log_level, directory, output = parse_arguments()
> +    log_level, directory, output, paths = parse_arguments()
>
>      level = getattr(logging, log_level)
>      logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
>
> -    filename_matcher = re.compile(_FILENAME_PATTERN)
>      line_matcher = re.compile(_LINE_PATTERN)
>
>      compile_commands = []
> -    for dirpath, _, filenames in os.walk(directory):
> -        for filename in filenames:
> -            if not filename_matcher.match(filename):
> -                continue
> -            filepath = os.path.join(dirpath, filename)
>
> -            with open(filepath, 'rt') as f:
> +    for path in paths:
> +        cmdfiles = cmdfiles_in_dir(path)
> +
> +        for cmdfile in cmdfiles:

If `cmdfiles` is never referenced again, please make this:

for cmdfile in cmdfiles_in_dir(path):

With those 2 changes feel free to add my
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> +            with open(cmdfile, 'rt') as f:
>                  result = line_matcher.match(f.readline())
>                  if result:
>                      try:
> -                        entry = process_line(directory,
> -                                             result.group(1), result.group(2))
> +                        entry = process_line(directory, result.group(1),
> +                                             result.group(2))
>                          compile_commands.append(entry)
>                      except ValueError as err:
>                          logging.info('Could not add line from %s: %s',
> -                                     filepath, err)
> +                                     cmdfile, err)
>
>      with open(output, 'wt') as f:
>          json.dump(compile_commands, f, indent=2, sort_keys=True)
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 8/9] kbuild: wire up the build rule of compile_commands.json to Makefile

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Currently, you need to manually run scripts/gen_compile_commands.py
> to create compile_commands.json. It parses all the .*.cmd files found
> under the specified directory.
>
> If you rebuild the kernel over again without 'make clean',
> .*.cmd files from older builds will create stale entries in
> compile_commands.json.
>
> This commit wires up the compile_commands.json rule to Makefile, and
> makes it parse only the .*.cmd files involved in the current build.
>
> Pass $(KBUILD_VMLINUX_OBJS), $(KBUILD_VMLINUX_LIBS), and modules.order
> to the script. The objects or archives linked to vmlinux are listed in
> $(KBUILD_VMLINUX_OBJS) or $(KBUILD_VMLINUX_LIBS). All the modules are
> listed in modules.order.
>
> You can create compile_commands.json from Make:
>
>   $ make -j$(nproc) CC=clang compile_commands.json
>
> You can also build vmlinux, modules, and compile_commands.json all
> together in a single command:
>
>   $ make -j$(nproc) CC=clang all compile_commands.json
>
> It works for M= builds as well. In this case, compile_commands.json
> is created in the top directory of the external module.
>
> This is convenient, but it has a drawback; the coverage of the
> compile_commands.json is reduced because only the objects linked to
> vmlinux or modules are handled. For example, the following C files are
> not included in the compile_commands.json:
>
>  - Decompressor source files (arch/*/boot/)
>  - VDSO source files
>  - C files used to generate intermediates (e.g. kernel/bounds.c)
>  - Standalone host programs
>
> I think it is fine for most developers because our main interest is
> the kernel-space code.
>
> If you want to cover all the compiled C files, please build the kernel
> then run the script manually as before:
>
>   $ make clean    # if you want to delete stale .cmd files [optional]
>   $ make -j$(nproc) CC=clang
>   $ scripts/gen_compile_commands.json
>
> Here is a note for out-of-tree builds. 'make compile_commands.json'
> works with O= option, but please notice compile_commands.json is
> created in the object tree instead of the source tree.
>
> Some people may want to have compile_commands.json in the source tree
> because Clang Tools searches for it through all parent paths of the
> first input source file.
>
> However, you cannot do this for O= builds. Kbuild should never generate
> any build artifact in the source tree when O= is given because the
> source tree might be read-only. Any write attempt to the source tree
> is monitored and the violation may be reported. See the commit log of
> 8ef14c2c41d9.
>
> So, the only possible way is to create compile_commands.json in the
> object tree, then specify '-p <build-path>' when you use clang-check,
> clang-tidy, etc.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> (no changes since v1)
>
>  Makefile | 29 +++++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 9cac6fde3479..65ed336a6de1 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -635,7 +635,7 @@ endif
>  # in addition to whatever we do anyway.
>  # Just "make" or "make all" shall build modules as well
>
> -ifneq ($(filter all modules nsdeps,$(MAKECMDGOALS)),)
> +ifneq ($(filter all modules nsdeps %compile_commands.json,$(MAKECMDGOALS)),)
>    KBUILD_MODULES := 1
>  endif
>
> @@ -1464,7 +1464,8 @@ endif # CONFIG_MODULES
>
>  # Directories & files removed with 'make clean'
>  CLEAN_FILES += include/ksym vmlinux.symvers \
> -              modules.builtin modules.builtin.modinfo modules.nsdeps
> +              modules.builtin modules.builtin.modinfo modules.nsdeps \
> +              compile_commands.json
>
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_FILES += include/config include/generated          \
> @@ -1698,9 +1699,12 @@ KBUILD_MODULES := 1
>
>  build-dirs := $(KBUILD_EXTMOD)
>  PHONY += modules
> -modules: descend
> +modules: $(MODORDER)
>         $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>
> +$(MODORDER): descend
> +       @:

^ stuff like this I don't understand.  But I understand the premise of
the patch, the change in behavior, and the tradeoffs, so:
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

> +
>  PHONY += modules_install
>  modules_install: _emodinst_ _emodinst_post
>
> @@ -1714,8 +1718,12 @@ PHONY += _emodinst_post
>  _emodinst_post: _emodinst_
>         $(call cmd,depmod)
>
> +compile_commands.json: $(extmod-prefix)compile_commands.json
> +PHONY += compile_commands.json
> +
>  clean-dirs := $(KBUILD_EXTMOD)
> -clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps
> +clean: rm-files := $(KBUILD_EXTMOD)/Module.symvers $(KBUILD_EXTMOD)/modules.nsdeps \
> +       $(KBUILD_EXTMOD)/compile_commands.json
>
>  PHONY += help
>  help:
> @@ -1828,6 +1836,19 @@ nsdeps: export KBUILD_NSDEPS=1
>  nsdeps: modules
>         $(Q)$(CONFIG_SHELL) $(srctree)/scripts/nsdeps
>
> +# Clang Tooling
> +# ---------------------------------------------------------------------------
> +
> +quiet_cmd_gen_compile_commands = GEN     $@
> +      cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, $(real-prereqs))
> +
> +$(extmod-prefix)compile_commands.json: scripts/gen_compile_commands.py \
> +       $(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
> +       $(if $(CONFIG_MODULES), $(MODORDER)) FORCE
> +       $(call if_changed,gen_compile_commands)
> +
> +targets += $(extmod-prefix)compile_commands.json
> +
>  # Scripts to check various things for consistency
>  # ---------------------------------------------------------------------------
>
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 7/9] gen_compile_commands: support *.o, *.a, modules.order in positional argument

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> This script currently searches the specified directory for .cmd files.
> One drawback is it may contain stale .cmd files after you rebuild the
> kernel several times without 'make clean'.
>
> This commit supports *.o, *.a, and modules.order as positional
> parameters. If such files are given, they are parsed to collect
> associated .cmd files. I added a generator helper for each of them.
>
> This feature is useful to get the list of active .cmd files from the
> last build, and will be used by the next commit to wire up the
> compile_commands.json rule to the Makefile.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
> Changes in v2:
>   - Separate the file parser into generator functions
>   - Use 'obj' instead of 'object' because 'object' is a built-in function
>   - I think using 'file' is OK because it is not a built-in function in Python3
>     (https://docs.python.org/3/library/functions.html)
>     Anyway, the variable 'file' is no longer used in this version
>   - Keep the previous work-flow to allow to search the given directory
>
>  scripts/gen_compile_commands.py | 100 ++++++++++++++++++++++++++++++--
>  1 file changed, 96 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> index 6dec7e2c4098..65859e6044b5 100755
> --- a/scripts/gen_compile_commands.py
> +++ b/scripts/gen_compile_commands.py
> @@ -12,6 +12,7 @@ import json
>  import logging
>  import os
>  import re
> +import subprocess
>
>  _DEFAULT_OUTPUT = 'compile_commands.json'
>  _DEFAULT_LOG_LEVEL = 'WARNING'
> @@ -32,8 +33,9 @@ def parse_arguments():
>      Returns:
>          log_level: A logging level to filter log output.
>          directory: The work directory where the objects were built
> +        ar: Command used for parsing .a archives
>          output: Where to write the compile-commands JSON file.
> -        paths: The list of directories to handle to find .cmd files
> +        paths: The list of files/directories to handle to find .cmd files
>      """
>      usage = 'Creates a compile_commands.json database from kernel .cmd files'
>      parser = argparse.ArgumentParser(description=usage)
> @@ -53,12 +55,21 @@ def parse_arguments():
>      parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
>                          default=_DEFAULT_LOG_LEVEL, help=log_level_help)
>
> +    ar_help = 'command used for parsing .a archives'
> +    parser.add_argument('-a', '--ar', type=str, default='ar', help=ar_help)

If there's a default, doesn't that mean it's no longer required? I
think it should be required.  For a clang specific tool, we'd prefer
the default to be llvm-ar anyways.

> +
> +    paths_help = ('directories to search or files to parse '
> +                  '(files should be *.o, *.a, or modules.order). '
> +                  'If nothing is specified, the current directory is searched')
> +    parser.add_argument('paths', type=str, nargs='*', help=paths_help)
> +
>      args = parser.parse_args()
>
>      return (args.log_level,
>              os.path.abspath(args.directory),
>              args.output,
> -            [args.directory])
> +            args.ar,
> +            args.paths if len(args.paths) > 0 else [args.directory])
>
>
>  def cmdfiles_in_dir(directory):
> @@ -81,6 +92,73 @@ def cmdfiles_in_dir(directory):
>                  yield os.path.join(dirpath, filename)
>
>
> +def to_cmdfile(path):
> +    """Return the path of .cmd file used for the given build artifact
> +
> +    Args:
> +        Path: file path
> +
> +    Returns:
> +        The path to .cmd file
> +    """
> +    dir, base = os.path.split(path)
> +    return os.path.join(dir, '.' + base + '.cmd')
> +
> +
> +def cmdfiles_for_o(obj):
> +    """Generate the iterator of .cmd files associated with the object
> +
> +    Yield the .cmd file used to build the given object
> +
> +    Args:
> +        obj: The object path
> +
> +    Yields:
> +        The path to .cmd file
> +    """
> +    yield to_cmdfile(obj)
> +
> +
> +def cmdfiles_for_a(archive, ar):
> +    """Generate the iterator of .cmd files associated with the archive.
> +
> +    Parse the given archive, and yield every .cmd file used to build it.
> +
> +    Args:
> +        archive: The archive to parse
> +
> +    Yields:
> +        The path to every .cmd file found
> +    """
> +    for obj in subprocess.check_output([ar, '-t', archive]).decode().split():
> +        yield to_cmdfile(obj)
> +
> +
> +def cmdfiles_for_modorder(modorder):
> +    """Generate the iterator of .cmd files associated with the modules.order.
> +
> +    Parse the given modules.order, and yield every .cmd file used to build the
> +    contained modules.
> +
> +    Args:
> +        modorder: The modules.order file to parse
> +
> +    Yields:
> +        The path to every .cmd file found
> +    """
> +    with open(modorder) as f:
> +        for line in f:
> +            ko = line.rstrip()
> +            base, ext = os.path.splitext(ko)

below in main() you check the file extension with endswith().  Would
it be good to be consistent between the two?

> +            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.
> +            with open(mod) as m:
> +                for obj in m.readline().split():
> +                    yield to_cmdfile(obj)
> +
> +
>  def process_line(root_directory, command_prefix, file_path):
>      """Extracts information from a .cmd line and creates an entry from it.
>
> @@ -116,7 +194,7 @@ def process_line(root_directory, command_prefix, file_path):
>
>  def main():
>      """Walks through the directory and finds and parses .cmd files."""
> -    log_level, directory, output, paths = parse_arguments()
> +    log_level, directory, output, ar, paths = parse_arguments()
>
>      level = getattr(logging, log_level)
>      logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
> @@ -126,7 +204,21 @@ def main():
>      compile_commands = []
>
>      for path in paths:
> -        cmdfiles = cmdfiles_in_dir(path)
> +        # If 'path' is a directory, handle all .cmd files under it.
> +        # Otherwise, handle .cmd files associated with the file.
> +        # Most of built-in objects are linked via archives (built-in.a or lib.a)
> +        # but some are linked to vmlinux directly.
> +        # Modules are lis

^ was this comment cut off?


> +        if os.path.isdir(path):
> +            cmdfiles = cmdfiles_in_dir(path)
> +        elif path.endswith('.o'):
> +            cmdfiles = cmdfiles_for_o(path)
> +        elif path.endswith('.a'):
> +            cmdfiles = cmdfiles_for_a(path, ar)
> +        elif path.endswith('modules.order'):
> +            cmdfiles = cmdfiles_for_modorder(path)
> +        else:
> +            sys.exit('{}: unknown file type'.format(path))
>
>          for cmdfile in cmdfiles:
>              with open(cmdfile, 'rt') as f:
> --
> 2.25.1
>


--
Thanks,
~Nick Desaulniers

Re: [PATCH v2 0/9] kbuild: clang-tidy

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> I improved gen_compile_commands.py,
> then rebased Nathan's v7 [1] on top of them.
> To save time, I modified the Makefile part.
> No change for run-clang-tools.py
>
> "make clang-tidy" should work in-tree build,
> out-of-tree build (O=), and external module build (M=).
>
> This version keeps the previous work-flow.
> You can still manually run scripts/gen_compile_commands.json
>
> 'make compile_commands.json' or 'make clang-tidy' is handier
> for most cases. As Nick noted, there is 3 % loss of the coverage.
>
> If you need the full compilation database that covers all the
> compiled C files, please run the script manually.
>
> [1] https://patchwork.kernel.org/patch/11687833/

Thank you for the work that went into this series.  The only reason I
started focusing on compiling the kernel with Clang 3.5 years ago was
that I simply wanted to run scan-build (clang's static analyzer,
enabled by this series) on the kernel to find bugs to start
contributing fixes for.  Turned out compiling the kernel with Clang
was a prerequisite, and I've been distracted with that ever since.
Thank you both for completing this journey.

>
> Masahiro Yamada (8):
>   gen_compile_commands: parse only the first line of .*.cmd files
>   gen_compile_commands: use choices for --log_levels option
>   gen_compile_commands: do not support .cmd files under tools/ directory
>   gen_compile_commands: reword the help message of -d option
>   gen_compile_commands: make -o option independent of -d option
>   gen_compile_commands: move directory walk to a generator function
>   gen_compile_commands: support *.o, *.a, modules.order in positional
>     argument
>   kbuild: wire up the build rule of compile_commands.json to Makefile
>
> Nathan Huckleberry (1):
>   Makefile: Add clang-tidy and static analyzer support to makefile
>
>  MAINTAINERS                                 |   1 +
>  Makefile                                    |  45 +++-
>  scripts/clang-tools/gen_compile_commands.py | 245 ++++++++++++++++++++
>  scripts/clang-tools/run-clang-tools.py      |  74 ++++++
>  scripts/gen_compile_commands.py             | 151 ------------
>  5 files changed, 361 insertions(+), 155 deletions(-)
>  create mode 100755 scripts/clang-tools/gen_compile_commands.py
>  create mode 100755 scripts/clang-tools/run-clang-tools.py
>  delete mode 100755 scripts/gen_compile_commands.py
>
> --
> 2.25.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 0/9] kbuild: clang-tidy

[Sedat Dilek]

On Sat, Aug 22, 2020 at 3:06 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > I improved gen_compile_commands.py,
> > then rebased Nathan's v7 [1] on top of them.
> > To save time, I modified the Makefile part.
> > No change for run-clang-tools.py
> >
> > "make clang-tidy" should work in-tree build,
> > out-of-tree build (O=), and external module build (M=).
> >
> > This version keeps the previous work-flow.
> > You can still manually run scripts/gen_compile_commands.json
> >
> > 'make compile_commands.json' or 'make clang-tidy' is handier
> > for most cases. As Nick noted, there is 3 % loss of the coverage.
> >
> > If you need the full compilation database that covers all the
> > compiled C files, please run the script manually.
> >
> > [1] https://patchwork.kernel.org/patch/11687833/
>
> Thank you for the work that went into this series.  The only reason I
> started focusing on compiling the kernel with Clang 3.5 years ago was
> that I simply wanted to run scan-build (clang's static analyzer,
> enabled by this series) on the kernel to find bugs to start
> contributing fixes for.  Turned out compiling the kernel with Clang
> was a prerequisite, and I've been distracted with that ever since.
> Thank you both for completing this journey.

/me donates Nick a "EoJ" (End Of Journey)

- Sedat -

>
> >
> > Masahiro Yamada (8):
> >   gen_compile_commands: parse only the first line of .*.cmd files
> >   gen_compile_commands: use choices for --log_levels option
> >   gen_compile_commands: do not support .cmd files under tools/ directory
> >   gen_compile_commands: reword the help message of -d option
> >   gen_compile_commands: make -o option independent of -d option
> >   gen_compile_commands: move directory walk to a generator function
> >   gen_compile_commands: support *.o, *.a, modules.order in positional
> >     argument
> >   kbuild: wire up the build rule of compile_commands.json to Makefile
> >
> > Nathan Huckleberry (1):
> >   Makefile: Add clang-tidy and static analyzer support to makefile
> >
> >  MAINTAINERS                                 |   1 +
> >  Makefile                                    |  45 +++-
> >  scripts/clang-tools/gen_compile_commands.py | 245 ++++++++++++++++++++
> >  scripts/clang-tools/run-clang-tools.py      |  74 ++++++
> >  scripts/gen_compile_commands.py             | 151 ------------
> >  5 files changed, 361 insertions(+), 155 deletions(-)
> >  create mode 100755 scripts/clang-tools/gen_compile_commands.py
> >  create mode 100755 scripts/clang-tools/run-clang-tools.py
> >  delete mode 100755 scripts/gen_compile_commands.py
> >
> > --
> > 2.25.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdkUfOnzWH1d7-qAP-PFvkLeahoA8jZdkZEp4-PNFXL_JA%40mail.gmail.com.

Re: [PATCH v2 4/9] gen_compile_commands: reword the help message of -d option

[Masahiro Yamada]

On Sat, Aug 22, 2020 at 9:29 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > I think the help message of the -d option is somewhat misleading.
> >
> >   Path to the kernel source directory to search (defaults to the working directory)
> >
> > The part "kernel source directory" is the source of the confusion.
> > Some people misunderstand as if this script did not support separate
> > output directories.
> >
> > Actually, this script also works for out-of-tree builds. You can
> > use the -d option to point to the object output directory, not to
> > the source directory. It should match to the O= option used in the
> > previous kernel build, and then appears in the "directory" field of
> > compile_commands.json.
> >
> > Reword the help message.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > Changes in v2:
> >   - New patch
> >
> >  scripts/gen_compile_commands.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> > index 1b9899892d99..5f6318da01a2 100755
> > --- a/scripts/gen_compile_commands.py
> > +++ b/scripts/gen_compile_commands.py
> > @@ -31,13 +31,13 @@ def parse_arguments():
> >
> >      Returns:
> >          log_level: A logging level to filter log output.
> > -        directory: The directory to search for .cmd files.
> > +        directory: The work directory where the objects were built
>
> Punctuation (add a period `.`).

Will fix.


> >          output: Where to write the compile-commands JSON file.
> >      """
> >      usage = 'Creates a compile_commands.json database from kernel .cmd files'
> >      parser = argparse.ArgumentParser(description=usage)
> >
> > -    directory_help = ('Path to the kernel source directory to search '
> > +    directory_help = ('specify the output directory used for the kernel build '
>
> Capitalization (specify -> Specify)




The help message of -h starts with a lower case.
The others start with a capital letter.

It would be better if "show this help message and exit"
started with a capital letter. But, it comes from the
library, so I do not know how to change it.

I changed our code to make it consistent, but
starting them with a capital letter is a preferred style,
I can do as you suggest.


Currently, the help looks like follows:

---------------->8-----------------------
masahiro@oscar:~/ref/linux$ ./scripts/gen_compile_commands.py  -h
usage: gen_compile_commands.py [-h] [-d DIRECTORY] [-o OUTPUT]
                               [--log_level LOG_LEVEL]

Creates a compile_commands.json database from kernel .cmd files

optional arguments:
  -h, --help            show this help message and exit
  -d DIRECTORY, --directory DIRECTORY
                        Path to the kernel source directory to search
                        (defaults to the working directory)
  -o OUTPUT, --output OUTPUT
                        The location to write compile_commands.json
                        (defaults to compile_commands.json in the search
                        directory)
  --log_level LOG_LEVEL
                        The level of log messages to produce (one of
                        DEBUG, INFO, WARNING, ERROR, CRITICAL; defaults to
                        WARNING)
---------------->8-----------------------



Thanks.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 4/9] gen_compile_commands: reword the help message of -d option

[Nick Desaulniers]

On Fri, Aug 21, 2020 at 6:56 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sat, Aug 22, 2020 at 9:29 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > I think the help message of the -d option is somewhat misleading.
> > >
> > >   Path to the kernel source directory to search (defaults to the working directory)
> > >
> > > The part "kernel source directory" is the source of the confusion.
> > > Some people misunderstand as if this script did not support separate
> > > output directories.
> > >
> > > Actually, this script also works for out-of-tree builds. You can
> > > use the -d option to point to the object output directory, not to
> > > the source directory. It should match to the O= option used in the
> > > previous kernel build, and then appears in the "directory" field of
> > > compile_commands.json.
> > >
> > > Reword the help message.
> > >
> > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > > ---
> > >
> > > Changes in v2:
> > >   - New patch
> > >
> > >  scripts/gen_compile_commands.py | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> > > index 1b9899892d99..5f6318da01a2 100755
> > > --- a/scripts/gen_compile_commands.py
> > > +++ b/scripts/gen_compile_commands.py
> > > @@ -31,13 +31,13 @@ def parse_arguments():
> > >
> > >      Returns:
> > >          log_level: A logging level to filter log output.
> > > -        directory: The directory to search for .cmd files.
> > > +        directory: The work directory where the objects were built
> >
> > Punctuation (add a period `.`).
>
> Will fix.
>
>
> > >          output: Where to write the compile-commands JSON file.
> > >      """
> > >      usage = 'Creates a compile_commands.json database from kernel .cmd files'
> > >      parser = argparse.ArgumentParser(description=usage)
> > >
> > > -    directory_help = ('Path to the kernel source directory to search '
> > > +    directory_help = ('specify the output directory used for the kernel build '
> >
> > Capitalization (specify -> Specify)
>
>
>
>
> The help message of -h starts with a lower case.
> The others start with a capital letter.
>
> It would be better if "show this help message and exit"
> started with a capital letter. But, it comes from the
> library, so I do not know how to change it.
>
> I changed our code to make it consistent, but
> starting them with a capital letter is a preferred style,
> I can do as you suggest.

Consistency throughout the patch is my priority, not necessarily
whether we're using Capitalization or not.

>
>
> Currently, the help looks like follows:
>
> ---------------->8-----------------------
> masahiro@oscar:~/ref/linux$ ./scripts/gen_compile_commands.py  -h
> usage: gen_compile_commands.py [-h] [-d DIRECTORY] [-o OUTPUT]
>                                [--log_level LOG_LEVEL]
>
> Creates a compile_commands.json database from kernel .cmd files
>
> optional arguments:
>   -h, --help            show this help message and exit
>   -d DIRECTORY, --directory DIRECTORY
>                         Path to the kernel source directory to search
>                         (defaults to the working directory)
>   -o OUTPUT, --output OUTPUT
>                         The location to write compile_commands.json
>                         (defaults to compile_commands.json in the search
>                         directory)
>   --log_level LOG_LEVEL
>                         The level of log messages to produce (one of
>                         DEBUG, INFO, WARNING, ERROR, CRITICAL; defaults to
>                         WARNING)
> ---------------->8-----------------------
>
>
>
> Thanks.
>
>
> --
> Best Regards
> Masahiro Yamada



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 7/9] gen_compile_commands: support *.o, *.a, modules.order in positional argument

[Masahiro Yamada]

On Sat, Aug 22, 2020 at 9:59 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 21, 2020 at 12:02 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > This script currently searches the specified directory for .cmd files.
> > One drawback is it may contain stale .cmd files after you rebuild the
> > kernel several times without 'make clean'.
> >
> > This commit supports *.o, *.a, and modules.order as positional
> > parameters. If such files are given, they are parsed to collect
> > associated .cmd files. I added a generator helper for each of them.
> >
> > This feature is useful to get the list of active .cmd files from the
> > last build, and will be used by the next commit to wire up the
> > compile_commands.json rule to the Makefile.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > Changes in v2:
> >   - Separate the file parser into generator functions
> >   - Use 'obj' instead of 'object' because 'object' is a built-in function
> >   - I think using 'file' is OK because it is not a built-in function in Python3
> >     (https://docs.python.org/3/library/functions.html)
> >     Anyway, the variable 'file' is no longer used in this version
> >   - Keep the previous work-flow to allow to search the given directory
> >
> >  scripts/gen_compile_commands.py | 100 ++++++++++++++++++++++++++++++--
> >  1 file changed, 96 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/gen_compile_commands.py b/scripts/gen_compile_commands.py
> > index 6dec7e2c4098..65859e6044b5 100755
> > --- a/scripts/gen_compile_commands.py
> > +++ b/scripts/gen_compile_commands.py
> > @@ -12,6 +12,7 @@ import json
> >  import logging
> >  import os
> >  import re
> > +import subprocess
> >
> >  _DEFAULT_OUTPUT = 'compile_commands.json'
> >  _DEFAULT_LOG_LEVEL = 'WARNING'
> > @@ -32,8 +33,9 @@ def parse_arguments():
> >      Returns:
> >          log_level: A logging level to filter log output.
> >          directory: The work directory where the objects were built
> > +        ar: Command used for parsing .a archives
> >          output: Where to write the compile-commands JSON file.
> > -        paths: The list of directories to handle to find .cmd files
> > +        paths: The list of files/directories to handle to find .cmd files
> >      """
> >      usage = 'Creates a compile_commands.json database from kernel .cmd files'
> >      parser = argparse.ArgumentParser(description=usage)
> > @@ -53,12 +55,21 @@ def parse_arguments():
> >      parser.add_argument('--log_level', choices=_VALID_LOG_LEVELS,
> >                          default=_DEFAULT_LOG_LEVEL, help=log_level_help)
> >
> > +    ar_help = 'command used for parsing .a archives'
> > +    parser.add_argument('-a', '--ar', type=str, default='ar', help=ar_help)
>
> If there's a default, doesn't that mean it's no longer required? I
> think it should be required.  For a clang specific tool, we'd prefer
> the default to be llvm-ar anyways.

A good point.
I want to set reasonable values as default where possible.
'llvm-ar' is better.

I will change it.



> > +
> > +def cmdfiles_for_modorder(modorder):
> > +    """Generate the iterator of .cmd files associated with the modules.order.
> > +
> > +    Parse the given modules.order, and yield every .cmd file used to build the
> > +    contained modules.
> > +
> > +    Args:
> > +        modorder: The modules.order file to parse
> > +
> > +    Yields:
> > +        The path to every .cmd file found
> > +    """
> > +    with open(modorder) as f:
> > +        for line in f:
> > +            ko = line.rstrip()
> > +            base, ext = os.path.splitext(ko)
>
> below in main() you check the file extension with endswith().  Would
> it be good to be consistent between the two?

I want to re-use 'base' to convert
the *.ko into *.mod

path/to/my/driver.ko
-> path/to/my/driver.mod


I think using os.path.split()
is good for checking the valid suffix,
and replaceing it with '.mod'.






> > +            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.
> > +            with open(mod) as m:
> > +                for obj in m.readline().split():
> > +                    yield to_cmdfile(obj)
> > +
> > +
> >  def process_line(root_directory, command_prefix, file_path):
> >      """Extracts information from a .cmd line and creates an entry from it.
> >
> > @@ -116,7 +194,7 @@ def process_line(root_directory, command_prefix, file_path):
> >
> >  def main():
> >      """Walks through the directory and finds and parses .cmd files."""
> > -    log_level, directory, output, paths = parse_arguments()
> > +    log_level, directory, output, ar, paths = parse_arguments()
> >
> >      level = getattr(logging, log_level)
> >      logging.basicConfig(format='%(levelname)s: %(message)s', level=level)
> > @@ -126,7 +204,21 @@ def main():
> >      compile_commands = []
> >
> >      for path in paths:
> > -        cmdfiles = cmdfiles_in_dir(path)
> > +        # If 'path' is a directory, handle all .cmd files under it.
> > +        # Otherwise, handle .cmd files associated with the file.
> > +        # Most of built-in objects are linked via archives (built-in.a or lib.a)
> > +        # but some are linked to vmlinux directly.
> > +        # Modules are lis
>
> ^ was this comment cut off?

Oops, I will fix it.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 6/9] gen_compile_commands: move directory walk to a generator function

[Masahiro Yamada]

On Sat, Aug 22, 2020 at 9:41 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:

> > +    for path in paths:
> > +        cmdfiles = cmdfiles_in_dir(path)
> > +
> > +        for cmdfile in cmdfiles:
>
> If `cmdfiles` is never referenced again, please make this:
>
> for cmdfile in cmdfiles_in_dir(path):


I intentionally wrote in this way
because in the next commit,
cmdfiles will be selectively set.





> With those 2 changes feel free to add my
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> > +            with open(cmdfile, 'rt') as f:
> >                  result = line_matcher.match(f.readline())
> >                  if result:
> >                      try:
> > -                        entry = process_line(directory,
> > -                                             result.group(1), result.group(2))
> > +                        entry = process_line(directory, result.group(1),
> > +                                             result.group(2))
> >                          compile_commands.append(entry)
> >                      except ValueError as err:
> >                          logging.info('Could not add line from %s: %s',
> > -                                     filepath, err)
> > +                                     cmdfile, err)
> >
> >      with open(output, 'wt') as f:
> >          json.dump(compile_commands, f, indent=2, sort_keys=True)
> > --
> > 2.25.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdn9ZfvC4dzuVnxc_a52JFn_q1ewOWwZZD5b9%3DizeEayKQ%40mail.gmail.com.



--
Best Regards
Masahiro Yamada