Skip to content

GitLab

Switch branch/tag
  1. 23 Oct, 2019 1 commit
    • Jessica Yu's avatar
      scripts/nsdeps: use alternative sed delimiter · 09684950
      Jessica Yu authored 
      
When doing an out of tree build with O=, the nsdeps script constructs
the absolute pathname of the module source file so that it can insert
MODULE_IMPORT_NS statements in the right place. However, ${srctree}
contains an unescaped path to the source tree, which, when used in a sed
substitution, makes sed complain:

++ sed 's/[^ ]* *//home/jeyu/jeyu-linux\/&/g'
sed: -e expression #1, char 12: unknown option to `s'

The sed substitution command 's' ends prematurely with the forward
slashes in the pathname, and sed errors out when it encounters the 'h',
which is an invalid sed substitution option. To avoid escaping forward
slashes ${srctree}, we can use '|' as an alternative delimiter for
sed instead to avoid this error.
Reviewed-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: default avatarMatthias Maennich <maennich@google.com>
Tested-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      09684950
  3. 19 Oct, 2019 2 commits
  5. 18 Oct, 2019 3 commits
  7. 17 Oct, 2019 1 commit
  9. 15 Oct, 2019 1 commit
  11. 13 Oct, 2019 1 commit
  13. 07 Oct, 2019 5 commits
    • Masahiro Yamada's avatar
      nsdeps: make generated patches independent of locale · df6f0987
      Masahiro Yamada authored 
      scripts/nsdeps automatically generates a patch to add MODULE_IMPORT_NS
tags, and what is nicer, it sorts the lines alphabetically with the
'sort' command. However, the output from the 'sort' command depends on
locale.

For example, I got this:

$ { echo usbstorage; echo usb_storage; } | LANG=en_US.UTF-8 sort
usbstorage
usb_storage
$ { echo usbstorage; echo usb_storage; } | LANG=C sort
usb_storage
usbstorage

So, this means people might potentially send different patches.

This kind of issue was reported in the past, for example,
commit f55f2328

 ("kbuild: make sorting initramfs contents
independent of locale").

Adding 'LANG=C' is a conventional way of fixing when a deterministic
result is desirable.

I added 'LANG=C' very close to the 'sort' command since changing
locale affects the language of error messages etc. We should respect
users' choice as much as possible.
Reviewed-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      df6f0987
    • Masahiro Yamada's avatar
      nsdeps: fix hashbang of scripts/nsdeps · 40997fb8
      Masahiro Yamada authored 
      
This script does not use bash-extension. I am guessing this hashbang
was copied from scripts/coccicheck, which really uses bash-extension.

/bin/sh is enough for this script.
Reviewed-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      40997fb8
    • Masahiro Yamada's avatar
      modpost: fix broken sym->namespace for external module builds · 389eb3f5
      Masahiro Yamada authored 
      
Currently, external module builds produce tons of false-positives:

  WARNING: module <mod> uses symbol <sym> from namespace <ns>, but does not import it.

Here, the <ns> part shows a random string.

When you build external modules, the symbol info of vmlinux and
in-kernel modules are read from $(objtree)/Module.symvers, but
read_dump() is buggy in multiple ways:

[1] When the modpost is run for vmlinux and in-kernel modules,
sym_extract_namespace() allocates memory for the namespace. On the
other hand, read_dump() does not, then sym->namespace will point to
somewhere in the line buffer of get_next_line(). The data in the
buffer will be replaced soon, and sym->namespace will end up with
pointing to unrelated data. As a result, check_exports() will show
random strings in the warning messages.

[2] When there is no namespace, sym_extract_namespace() returns NULL.
On the other hand, read_dump() sets namespace to an empty string "".
(but, it will be later replaced with unrelated data due to bug [1].)
The check_exports() shows a warning unless exp->namespace is NULL,
so every symbol read from read_dump() emits the warning, which is
mostly false positive.

To address [1], sym_add_exported() calls strdup() for s->namespace.
The namespace from sym_extract_namespace() must be freed to avoid
memory leak.

For [2], I changed the if-conditional in check_exports().

This commit also fixes sym_add_exported() to set s->namespace correctly
when the symbol is preloaded.
Reviewed-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      389eb3f5
    • Masahiro Yamada's avatar
      module: swap the order of symbol.namespace · bf70b050
      Masahiro Yamada authored 
      
Currently, EXPORT_SYMBOL_NS(_GPL) constructs the kernel symbol as
follows:

  __ksymtab_SYMBOL.NAMESPACE

The sym_extract_namespace() in modpost allocates memory for the part
SYMBOL.NAMESPACE when '.' is contained. One problem is that the pointer
returned by strdup() is lost because the symbol name will be copied to
malloc'ed memory by alloc_symbol(). No one will keep track of the
pointer of strdup'ed memory.

sym->namespace still points to the NAMESPACE part. So, you can free it
with complicated code like this:

   free(sym->namespace - strlen(sym->name) - 1);

It complicates memory free.

To fix it elegantly, I swapped the order of the symbol and the
namespace as follows:

  __ksymtab_NAMESPACE.SYMBOL

then, simplified sym_extract_namespace() so that it allocates memory
only for the NAMESPACE part.

I prefer this order because it is intuitive and also matches to major
languages. For example, NAMESPACE::NAME in C++, MODULE.NAME in Python.
Reviewed-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      bf70b050
    • YueHaibing's avatar
      scripts: add_namespace: Fix coccicheck failed · c7c4e29f
      YueHaibing authored 
      Now all scripts in scripts/coccinelle to be automatically called
by coccicheck. However new adding add_namespace.cocci does not
support report mode, which make coccicheck failed.
This add "virtual report" to  make the coccicheck go ahead smoothly.

Fixes: eb8305ae

 ("scripts: Coccinelle script for namespace dependencies.")
Acked-by: default avatarJulia Lawall <julia.lawall@lip6.fr>
Acked-by: default avatarMatthias Maennich <maennich@google.com>
Signed-off-by: default avatarYueHaibing <yuehaibing@huawei.com>
Signed-off-by: default avatarJessica Yu <jeyu@kernel.org>
      c7c4e29f
  15. 05 Oct, 2019 2 commits
    • Masahiro Yamada's avatar
      scripts/setlocalversion: clear local variable to make it work for sh · 7a82e3fa
      Masahiro Yamada authored 
      Geert Uytterhoeven reports a strange side-effect of commit 858805b3
("kbuild: add $(BASH) to run scripts with bash-extension"), which
inserts the contents of a localversion file in the build directory twice.

[Steps to Reproduce]
  $ echo bar > localversion
  $ mkdir build
  $ cd build/
  $ echo foo > localversion
  $ make -s -f ../Makefile defconfig include/config/kernel.release
  $ cat include/config/kernel.release
  5.4.0-rc1foofoobar

This comes down to the behavior change of local variables.

The 'man sh' on my Ubuntu machine, where sh is an alias to dash,
explains as follows:
  When a variable is made local, it inherits the initial value and
  exported and readonly flags from the variable with the same name
  in the surrounding scope, if there is one. Otherwise, the variable
  is initially unset.

[Test Code]

  foo ()
  {
          local res
          echo "res: $res"
  }

  res=1
  foo

[Result]

  $ sh test.sh
  res: 1
  $ b...
      7a82e3fa
    • Jacob Keller's avatar
      namespace: fix namespace.pl script to support relative paths · 82fdd12b
      Jacob Keller authored 
      The namespace.pl script does not work properly if objtree is not set to
an absolute path. The do_nm function is run from within the find
function, which changes directories.

Because of this, appending objtree, $File::Find::dir, and $source, will
return a path which is not valid from the current directory.

This used to work when objtree was set to an absolute path when using
"make namespacecheck". It appears to have not worked when calling
./scripts/namespace.pl directly.

This behavior was changed in 7e1c0477

 ("kbuild: Use relative path
for $(objtree)", 2014-05-14)

Rather than fixing the Makefile to set objtree to an absolute path, just
fix namespace.pl to work when srctree and objtree are relative. Also fix
the script to use an absolute path for these by default.

Use the File::Spec module for this purpose. It's been part of perl
5 since 5.005.

The curdir() function is used to get the current directory when the
objtree and srctree aren't set in the environment.

rel2abs() is used to convert possibly relative objtree and srctree
environment variables to absolute paths.

Finally, the catfile() function is used instead of string appending
paths together, since this is more robust when joining paths together.
Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Acked-by: default avatarRandy Dunlap <rdunlap@infradead.org>
Tested-by: default avatarRandy Dunlap <rdunlap@infradead.org>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      82fdd12b
  17. 01 Oct, 2019 2 commits
    • Masahiro Yamada's avatar
      modpost: fix static EXPORT_SYMBOL warnings for UML build · 47346e96
      Masahiro Yamada authored 
      Johannes Berg reports lots of modpost warnings on ARCH=um builds:

WARNING: "rename" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "lseek" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "ftruncate64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "getuid" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "lseek64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "unlink" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "pwrite64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "close" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "opendir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "pread64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "syscall" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "readdir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "readdir64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "futimes" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__lxstat" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "write" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "closedir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__xstat" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "fsync" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__lxstat64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__fxstat64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "telldir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "printf" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "readlink" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__sprintf_chk" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "link" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "rmdir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "fdatasync" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "truncate" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "statfs" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__errno_location" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__xmknod" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "open64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "truncate64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "open" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "read" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "chown" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "chmod" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "utime" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "fchmod" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "seekdir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "ioctl" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "dup2" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "statfs64" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "utimes" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "mkdir" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "fchown" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__guard" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "symlink" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "access" [vmlinux] is a static EXPORT_SYMBOL
WARNING: "__stack_smash_handler" [vmlinux] is a static EXPORT_SYMBOL

When you run "make", the modpost is run twice; before linking vmlinux,
and before building modules. All the warnings above are from the second
modpost.

The offending symbols are defined not in vmlinux, but in the C library.
The first modpost is run against the relocatable vmlinux.o, and those
warnings are nicely suppressed because the SH_UNDEF entries from the
symbol table clear the ->is_static flag.

The second modpost is run against the executable vmlinux (+ modules),
where those symbols have been resolved, but the definitions do not
exist.

This commit fixes it in a straightforward way; suppress the static
EXPORT_SYMBOL warnings from "vmlinux".

Without this commit, we see valid warnings twice anyway. For example,
ARCH=arm64 defconfig shows the following warning twice:

WARNING: "HYPERVISOR_platform_op" [vmlinux] is a static EXPORT_SYMBOL_GPL

So, it is reasonable to suppress the second one.

Fixes: 15bfc234

 ("modpost: check for static EXPORT_SYMBOL* functions")
Reported-by: default avatarJohannes Berg <johannes@sipsolutions.net>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Tested-by: default avatarJohannes Berg <johannes@sipsolutions.net>
Tested-by: default avatarDenis Efremov <efremov@linux.com>
      47346e96
    • Masahiro Yamada's avatar
      kbuild: remove ar-option and KBUILD_ARFLAGS · 13dc8c02
      Masahiro Yamada authored 
      Commit 40df759e

 ("kbuild: Fix build with binutils <= 2.19")
introduced ar-option and KBUILD_ARFLAGS to deal with old binutils.

According to Documentation/process/changes.rst, the current minimal
supported version of binutils is 2.21 so you can assume the 'D' option
is always supported. Not only GNU ar but also llvm-ar supports it.

With the 'D' option hard-coded, there is no more user of ar-option
or KBUILD_ARFLAGS.
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
Tested-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      13dc8c02
  19. 26 Sep, 2019 10 commits
  21. 14 Sep, 2019 4 commits
    • Masahiro Yamada's avatar
      genksyms: convert to SPDX License Identifier for lex.l and parse.y · 77564a48
      Masahiro Yamada authored 
      
I used the C comment style (/* ... */) for the flex and bison files
as in Kconfig (scripts/kconfig/{lexer.l,parser.y})
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      77564a48
    • Masahiro Yamada's avatar
      modpost: use __section in the output to *.mod.c · a3d0cb04
      Masahiro Yamada authored 
      
Use the __section() shorthand. This avoids escaping double-quotes,
and improves the readability.
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      a3d0cb04
    • Masahiro Yamada's avatar
      modpost: use MODULE_INFO() for __module_depends · 6df7e1ec
      Masahiro Yamada authored 
      
This makes *.mod.c much more readable. I confirmed depmod still
produced the same modules.dep file.
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      6df7e1ec
    • Masahiro Yamada's avatar
      export.h, genksyms: do not make genksyms calculate CRC of trimmed symbols · 69a94abb
      Masahiro Yamada authored 
      Arnd Bergmann reported false-positive modpost warnings detected by his
randconfig testing of linux-next.

Actually, this happens under the combination of CONFIG_MODVERSIONS
and CONFIG_TRIM_UNUSED_KSYMS since commit 15bfc234 ("modpost:
check for static EXPORT_SYMBOL* functions").

For example, arch/arm/config/multi_v7_defconfig + CONFIG_MODVERSIONS
+ CONFIG_TRIM_UNUSED_KSYMS produces the following false-positives:

WARNING: "__lshrdi3" [vmlinux] is a static (unknown)
WARNING: "__ashrdi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lasr" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsr" [vmlinux] is a static (unknown)
WARNING: "ftrace_set_clr_event" [vmlinux] is a static (unknown)
WARNING: "__muldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_ulcmp" [vmlinux] is a static (unknown)
WARNING: "__ucmpdi2" [vmlinux] is a static (unknown)
WARNING: "__aeabi_lmul" [vmlinux] is a static (unknown)
WARNING: "__bswapsi2" [vmlinux] is a static (unknown)
WARNING: "__bswapdi2" [vmlinux] is a static (unknown)
WARNING: "__ashldi3" [vmlinux] is a static (unknown)
WARNING: "__aeabi_llsl" [vmlinux] is a static (unknown)

The root cause of the problem is not in the modpost, but in the
implementation of CONFIG_TRIM_UNUSED_KSYMS.

If there is at least one untrimmed symbol in the file, genksyms is
invoked to calculate CRC of *all* the exported symbols in that file
even if some of them have been trimmed due to no caller existing.

As a result, .tmp_*.ver files contain CRC of trimmed symbols, thus
unneeded, orphan __crc* symbols are added to objects. It had been
harmless until recently.

With commit 15bfc234 ("modpost: check for static EXPORT_SYMBOL*
functions"), it is now harmful because the bogus __crc* symbols make
modpost call sym_update_crc() to add the symbols to the hash table,
but there is no one that clears the ->is_static member.

I gave Fixes to the first commit that uncovered the issue, but the
potential problem has long existed since commit f2355416
("export.h: allow for per-symbol configurable EXPORT_SYMBOL()").

Fixes: 15bfc234

 ("modpost: check for static EXPORT_SYMBOL* functions")
Reported-by: default avatarArnd Bergmann <arnd@arndb.de>
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Tested-by: default avatarArnd Bergmann <arnd@arndb.de>
      69a94abb
  23. 10 Sep, 2019 3 commits
  25. 09 Sep, 2019 1 commit
    • Masahiro Yamada's avatar
      kbuild: allow Clang to find unused static inline functions for W=1 build · 6863f564
      Masahiro Yamada authored 
      GCC and Clang have different policy for -Wunused-function; GCC does not
warn unused static inline functions at all whereas Clang does if they
are defined in source files instead of included headers although it has
been suppressed since commit abb2ea7d ("compiler, clang: suppress
warning for unused static inline functions").

We often miss to delete unused functions where 'static inline' is used
in *.c files since there is no tool to detect them. Unused code remains
until somebody notices. For example, commit 075ddd75 ("regulator:
core: remove unused rdev_get_supply()").

Let's remove __maybe_unused from the inline macro to allow Clang to
start finding unused static inline functions. For now, we do this only
for W=1 build since it is not a good idea to sprinkle warnings for the
normal build (e.g. 35 warnings for arch/x86/configs/x86_64_defconfig).

My initial attempt was to add -Wno-unused-function for no W= build
(https://lore.kernel.org/patchwork/patch/1120594/

)

Nathan Chancellor pointed out that would weaken Clang's checks since
we would no longer get -Wunused-function without W=1. It is true GCC
would catch unused static non-inline functions, but it would weaken
Clang as a standalone compiler, at least.

Hence, here is a counter implementation. The current problem is, W=...
only controls compiler flags, which are globally effective. There is
no way to address only 'static inline' functions.

This commit defines KBUILD_EXTRA_WARN[123] corresponding to W=[123].
When KBUILD_EXTRA_WARN1 is defined, __maybe_unused is omitted from
the 'inline' macro.

The new macro __inline_maybe_unused makes the code a bit uglier, so I
hope we can remove it entirely after fixing most of the warnings.

If you contribute to code clean-up, please run "make CC=clang W=1"
and check -Wunused-function warnings. You will find lots of unused
functions.

Some of them are false-positives because the call-sites are disabled
by #ifdef. I do not like to abuse the inline keyword for suppressing
unused-function warnings because it is intended to be a hint for the
compiler optimization. I prefer #ifdef around the definition, or
__maybe_unused if #ifdef would make the code too ugly.
Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
Reviewed-by: default avatarNathan Chancellor <natechancellor@gmail.com>
Tested-by: default avatarNathan Chancellor <natechancellor@gmail.com>
      6863f564
  27. 06 Sep, 2019 4 commits
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7