1. 14 Sep, 2019 1 commit
    • 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
  2. 10 Sep, 2019 1 commit
  3. 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
  4. 06 Sep, 2019 2 commits
    • Masahiro Yamada's avatar
      kbuild: rename KBUILD_ENABLE_EXTRA_GCC_CHECKS to KBUILD_EXTRA_WARN · e27128db
      Masahiro Yamada authored
      KBUILD_ENABLE_EXTRA_GCC_CHECKS started as a switch to add extra warning
      options for GCC, but now it is a historical misnomer since we use it
      also for Clang, DTC, and even kernel-doc.
      
      Rename it to more sensible, shorter KBUILD_EXTRA_WARN.
      
      For the backward compatibility, KBUILD_ENABLE_EXTRA_GCC_CHECKS is still
      supported (but not advertised in the documentation).
      
      I also fixed up 'make help', and updated the documentation.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Reviewed-by: default avatarNathan Chancellor <natechancellor@gmail.com>
      Reviewed-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Reviewed-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      e27128db
    • Masahiro Yamada's avatar
      kbuild: refactor scripts/Makefile.extrawarn · 64a91907
      Masahiro Yamada authored
      Instead of the warning-[123] magic, let's accumulate compiler options
      to KBUILD_CFLAGS directly as the top Makefile does. I think this makes
      it easier to understand what is going on in this file.
      
      This commit slightly changes the behavior, I think all of which are OK.
      
      [1] Currently, cc-option calls are needlessly evaluated. For example,
            warning-3 += $(call cc-option, -Wpacked-bitfield-compat)
          needs evaluating only when W=3, but it is actually evaluated for
          W=1, W=2 as well. With this commit, only relevant cc-option calls
          will be evaluated. This is a slight optimization.
      
      [2] Currently, unsupported level like W=4 is checked by:
            $(error W=$(KBUILD_ENABLE_EXTRA_GCC_CHECKS) is unknown)
          This will no longer be checked, but I do not think it is a big
          deal.
      
      [3] Currently, 4 Clang warnings (Winitializer-overrides, Wformat,
          Wsign-compare, Wformat-zero-length) are shown by any of W=1, W=2,
          and W=3. With this commit, they will be warned only by W=1. I
          think this is a more correct behavior since each warning belongs
          to only one group.
      
      For understanding this commit correctly:
      
      We have 3 warning groups, W=1, W=2, and W=3. You may think W=3 has a
      higher level than W=1, but they are actually independent. If you like,
      you can combine them like W=13. To enable all the warnings, you can
      pass W=123. It is shown by 'make help', but not noticed much. Since we
      support W= combination, there should not exist intersection among the
      three groups. If we enable Winitializer-overrides for W=1, we do not
      need to for W=2 or W=3. This is the reason why I think the change [3]
      makes sense.
      
      The documentation says -Winitializer-overrides is enabled by default.
      (https://clang.llvm.org/docs/DiagnosticsReference.html#winitializer-overrides)
      We negate it by passing -Wno-initializer-overrides for the normal
      build, but we do not do that for W=1. This means, W=1 effectively
      enables -Winitializer-overrides by the clang's default. The same for
      the other three.
      
      Add comments in case people are confused with the code.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Reviewed-by: default avatarNathan Chancellor <natechancellor@gmail.com>
      Tested-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      Acked-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Acked-by: default avatarMiguel Ojeda <miguel.ojeda.sandonis@gmail.com>
      64a91907
  5. 04 Sep, 2019 7 commits
    • Guillaume Tucker's avatar
      merge_config.sh: ignore unwanted grep errors · 60bef52c
      Guillaume Tucker authored
      The merge_config.sh script verifies that all the config options have
      their expected value in the resulting file and prints any issues as
      warnings.  These checks aren't intended to be treated as errors given
      the current implementation.  However, since "set -e" was added, if the
      grep command to look for a config option does not find it the script
      will then abort prematurely.
      
      Handle the case where the grep exit status is non-zero by setting
      ACTUAL_VAL to an empty string to restore previous functionality.
      
      Fixes: cdfca821 ("merge_config.sh: Check error codes from make")
      Signed-off-by: default avatarGuillaume Tucker <guillaume.tucker@collabora.com>
      Acked-by: default avatarJon Hunter <jonathanh@nvidia.com>
      Tested-by: default avatarJon Hunter <jonathanh@nvidia.com>
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      60bef52c
    • Masahiro Yamada's avatar
      kbuild: change *FLAGS_<basetarget>.o to take the path relative to $(obj) · 54b8ae66
      Masahiro Yamada authored
      Kbuild provides per-file compiler flag addition/removal:
      
        CFLAGS_<basetarget>.o
        CFLAGS_REMOVE_<basetarget>.o
        AFLAGS_<basetarget>.o
        AFLAGS_REMOVE_<basetarget>.o
        CPPFLAGS_<basetarget>.lds
        HOSTCFLAGS_<basetarget>.o
        HOSTCXXFLAGS_<basetarget>.o
      
      The <basetarget> is the filename of the target with its directory and
      suffix stripped.
      
      This syntax comes into a trouble when two files with the same basename
      appear in one Makefile, for example:
      
        obj-y += foo.o
        obj-y += dir/foo.o
        CFLAGS_foo.o := <some-flags>
      
      Here, the <some-flags> applies to both foo.o and dir/foo.o
      
      The real world problem is:
      
        scripts/kconfig/util.c
        scripts/kconfig/lxdialog/util.c
      
      Both files are compiled into scripts/kconfig/mconf, but only the
      latter should be given with the ncurses flags.
      
      It is more sensible to use the relative path to the Makefile, like this:
      
        obj-y += foo.o
        CFLAGS_foo.o := <some-flags>
        obj-y += dir/foo.o
        CFLAGS_dir/foo.o := <other-flags>
      
      At first, I attempted to replace $(basetarget) with $*. The $* variable
      is replaced with the stem ('%') part in a pattern rule. This works with
      most of cases, but does not for explicit rules.
      
      For example, arch/ia64/lib/Makefile reuses rule_as_o_S in its own
      explicit rules, so $* will be empty, resulting in ignoring the per-file
      AFLAGS.
      
      I introduced a new variable, target-stem, which can be used also from
      explicit rules.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Acked-by: default avatarMarc Zyngier <maz@kernel.org>
      54b8ae66
    • Denis Efremov's avatar
      modpost: add NOFAIL to strndup · 6f02bdfc
      Denis Efremov authored
      Add NOFAIL check for the strndup call, because the function
      allocates memory and can return NULL. All calls to strdup in
      modpost are checked with NOFAIL.
      Signed-off-by: default avatarDenis Efremov <efremov@linux.com>
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      6f02bdfc
    • Heikki Krogerus's avatar
      modpost: add guid_t type definition · 389c9af7
      Heikki Krogerus authored
      Since guid_t is the recommended data type for UUIDs in
      kernel (and I guess uuid_le is meant to be ultimately
      replaced with it), it should be made available here as
      well.
      Signed-off-by: default avatarHeikki Krogerus <heikki.krogerus@linux.intel.com>
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      389c9af7
    • Masahiro Yamada's avatar
      kbuild: add $(BASH) to run scripts with bash-extension · 858805b3
      Masahiro Yamada authored
      CONFIG_SHELL falls back to sh when bash is not installed on the system,
      but nobody is testing such a case since bash is usually installed.
      So, shell scripts invoked by CONFIG_SHELL are only tested with bash.
      
      It makes it difficult to test whether the hashbang #!/bin/sh is real.
      For example, #!/bin/sh in arch/powerpc/kernel/prom_init_check.sh is
      false. (I fixed it up)
      
      Besides, some shell scripts invoked by CONFIG_SHELL use bash-extension
      and #!/bin/bash is specified as the hashbang, while CONFIG_SHELL may
      not always be set to bash.
      
      Probably, the right thing to do is to introduce BASH, which is bash by
      default, and always set CONFIG_SHELL to sh. Replace $(CONFIG_SHELL)
      with $(BASH) for bash scripts.
      
      If somebody tries to add bash-extension to a #!/bin/sh script, it will
      be caught in testing because /bin/sh is a symlink to dash on some major
      distributions.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      858805b3
    • Masahiro Yamada's avatar
      kbuild: remove ARCH_{CPP,A,C}FLAGS · 8cc7af75
      Masahiro Yamada authored
      These flags were added by commit 61754c18 ("kbuild: Allow arch
      Makefiles to override {cpp,ld,c}flags") to allow ARC to override -O2.
      
      We did not see any other usage after all. Now that ARC switched to
      CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3, there is no more user of
      these variables.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      8cc7af75
    • Masahiro Yamada's avatar
      kbuild,arc: add CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3 for ARC · 15f5db60
      Masahiro Yamada authored
      arch/arc/Makefile overrides -O2 with -O3. This is the only user of
      ARCH_CFLAGS. There is no user of ARCH_CPPFLAGS or ARCH_AFLAGS.
      My plan is to remove ARCH_{CPP,A,C}FLAGS after refactoring the ARC
      Makefile.
      
      Currently, ARC has no way to enable -Wmaybe-uninitialized because both
      -O3 and -Os disable it. Enabling it will be useful for compile-testing.
      This commit allows allmodconfig (, which defaults to -O2) to enable it.
      
      Add CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3=y to all the defconfig files
      in arch/arc/configs/ in order to keep the current config settings.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      Acked-by: default avatarVineet Gupta <vgupta@synopsys.com>
      15f5db60
  6. 29 Aug, 2019 13 commits
  7. 28 Aug, 2019 3 commits
    • Masahiro Yamada's avatar
      init/Kconfig: rework help of CONFIG_CC_OPTIMIZE_FOR_SIZE · ce3b487f
      Masahiro Yamada authored
      CONFIG_CC_OPTIMIZE_FOR_SIZE was originally an independent boolean
      option, but commit 877417e6 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
      definition") turned it into a choice between _PERFORMANCE and _SIZE.
      
      The phrase "If unsure, say N." sounds like an independent option.
      Reword the help text to make it appropriate for the choice menu.
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      ce3b487f
    • Masahiro Yamada's avatar
      docs: kbuild: remove cc-ldoption from document again · d20558d1
      Masahiro Yamada authored
      Commit 055efab3 ("kbuild: drop support for cc-ldoption") correctly
      removed the cc-ldoption from Documentation/kbuild/makefiles.txt, but
      commit cd238eff ("docs: kbuild: convert docs to ReST and rename
      to *.rst") revived it. I guess it was a rebase mistake.
      
      Remove it again.
      
      Fixes: cd238eff ("docs: kbuild: convert docs to ReST and rename to *.rst")
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      d20558d1
    • Masahiro Yamada's avatar
      docs: kbuild: fix invalid ReST syntax · 4fef9dec
      Masahiro Yamada authored
      I see the following warnings when I open this document with a ReST
      viewer, retext:
      
      /home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1142: (WARNING/2) Inline emphasis start-string without end-string.
      /home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1152: (WARNING/2) Inline emphasis start-string without end-string.
      /home/masahiro/ref/linux/Documentation/kbuild/makefiles.rst:1154: (WARNING/2) Inline emphasis start-string without end-string.
      
      These hunks were added by commit e846f0dc ("kbuild: add support
      for ensuring headers are self-contained") and commit 1e21cbfa
      ("kbuild: support header-test-pattern-y"), respectively. They were
      written not for ReST but for the plain text, and merged via the
      kbuild tree.
      
      In the same development cycle, this document was converted to ReST
      by commit cd238eff ("docs: kbuild: convert docs to ReST and rename
      to *.rst"), and merged via the doc sub-system.
      
      Merging them together into Linus' tree resulted in the current situation.
      
      To fix the syntax, surround the asterisks with back-quotes, and
      use :: for the code sample.
      
      Fixes: 39ceda5c ("Merge tag 'kbuild-v5.3' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild")
      Signed-off-by: default avatarMasahiro Yamada <yamada.masahiro@socionext.com>
      4fef9dec
  8. 25 Aug, 2019 1 commit
  9. 24 Aug, 2019 3 commits
  10. 21 Aug, 2019 8 commits