1. 06 Jun, 2020 26 commits
    • Masahiro Yamada's avatar
      modpost: remove is_vmlinux() helper · 4de7b629
      Masahiro Yamada authored
      Now that is_vmlinux() is called only in new_module(), we can inline
      the function call.
      
      modname is the basename with '.o' is stripped. No need to compare it
      with 'vmlinux.o'.
      
      vmlinux is always located at the current working directory. No need
      to strip the directory path.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      4de7b629
    • Masahiro Yamada's avatar
      modpost: strip .o from modname before calling new_module() · a82f794c
      Masahiro Yamada authored
      new_module() conditionally strips the .o because the modname has .o
      suffix when it is called from read_symbols(), but no .o when it is
      called from read_dump().
      
      It is clearer to strip .o in read_symbols().
      
      I also used flexible-array for mod->name.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      a82f794c
    • Masahiro Yamada's avatar
      modpost: set have_vmlinux in new_module() · 858b937d
      Masahiro Yamada authored
      Set have_vmlinux flag in a single place.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      858b937d
    • Masahiro Yamada's avatar
      modpost: remove mod->skip struct member · 0b19d54c
      Masahiro Yamada authored
      The meaning of 'skip' is obscure since it does not explain
      "what to skip".
      
      mod->skip is set when it is vmlinux or the module info came from
      a dump file.
      
      So, mod->skip is equivalent to (mod->is_vmlinux || mod->from_dump).
      
      For the check in write_namespace_deps_files(), mod->is_vmlinux is
      unneeded because the -d option is not passed in the first pass of
      modpost.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      0b19d54c
    • Masahiro Yamada's avatar
      modpost: add mod->is_vmlinux struct member · 5a438af9
      Masahiro Yamada authored
      is_vmlinux() is called in several places to check whether the current
      module is vmlinux or not.
      
      It is faster and clearer to check mod->is_vmlinux flag.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      5a438af9
    • Masahiro Yamada's avatar
      modpost: remove is_vmlinux() call in check_for_{gpl_usage,unused}() · 1be5fa6c
      Masahiro Yamada authored
      check_exports() is never called for vmlinux because mod->skip is set
      for vmlinux.
      
      Hence, check_for_gpl_usage() and check_for_unused() are not called
      for vmlinux, either. is_vmlinux() is always false here.
      
      Remove the is_vmlinux() calls, and hard-code the ".ko" suffix.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      1be5fa6c
    • Masahiro Yamada's avatar
      modpost: remove mod->is_dot_o struct member · 3379576d
      Masahiro Yamada authored
      Previously, there were two cases where mod->is_dot_o is unset:
      
      [1] the executable 'vmlinux' in the second pass of modpost
      [2] modules loaded by read_dump()
      
      I think [1] was intended usage to distinguish 'vmlinux.o' and 'vmlinux'.
      Now that modpost does not parse the executable 'vmlinux', this case
      does not happen.
      
      [2] is obscure, maybe a bug. Module.symver stores module paths without
      extension. So, none of modules loaded by read_dump() has the .o suffix,
      and new_module() unsets ->is_dot_o. Anyway, it is not a big deal because
      handle_symbol() is not called for the case.
      
      To sum up, all the parsed ELF files are .o files.
      
      mod->is_dot_o is unneeded.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      3379576d
    • Masahiro Yamada's avatar
      modpost: move -d option in scripts/Makefile.modpost · 859c926a
      Masahiro Yamada authored
      Collect options for modules into a single place.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      859c926a
    • Masahiro Yamada's avatar
      modpost: remove -s option · 467b82d7
      Masahiro Yamada authored
      The -s option was added by commit 8d8d8289 ("kbuild: do not do
      section mismatch checks on vmlinux in 2nd pass").
      
      Now that the second pass does not parse vmlinux, this option is
      unneeded.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      467b82d7
    • Masahiro Yamada's avatar
      modpost: remove get_next_text() and make {grab,release_}file static · 75893572
      Masahiro Yamada authored
      get_next_line() is no longer used. Remove.
      
      grab_file() and release_file() are only used in modpost.c. Make them
      static.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      75893572
    • Masahiro Yamada's avatar
      modpost: use read_text_file() and get_line() for reading text files · 70f30cfe
      Masahiro Yamada authored
      grab_file() mmaps a file, but it is not so efficient here because
      get_next_line() copies every line to the temporary buffer anyway.
      
      read_text_file() and get_line() are simpler. get_line() exploits the
      library function strchr().
      
      Going forward, the missing *.symvers or *.cmd is a fatal error.
      This should not happen because scripts/Makefile.modpost guards the
      -i option files with $(wildcard $(input-symdump)).
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      70f30cfe
    • Masahiro Yamada's avatar
      modpost: avoid false-positive file open error · 7c8f5662
      Masahiro Yamada authored
      One problem of grab_file() is that it cannot distinguish the following
      two cases:
      
       - It cannot read the file (the file does not exist, or read permission
         is not set)
      
       - It can read the file, but the file size is zero
      
      This is because grab_file() calls mmap(), which requires the mapped
      length is greater than 0. Hence, grab_file() fails for both cases.
      
      If an empty header file were included for checksum calculation, the
      following warning would be printed:
      
        WARNING: modpost: could not open ...: Invalid argument
      
      An empty file is a valid source file, so it should not fail.
      
      Use read_text_file() instead. It can read a zero-length file.
      Then, parse_file() will succeed with doing nothing.
      
      Going forward, the first case (it cannot read the file) is a fatal
      error. If the source file from which an object was compiled is missing,
      something went wrong.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      7c8f5662
    • Masahiro Yamada's avatar
      modpost: fix potential mmap'ed file overrun in get_src_version() · f531c1b5
      Masahiro Yamada authored
      I do not know how reliably this function works, but it looks dangerous
      to me.
      
          strchr(sources, '\n');
      
      ... continues searching until it finds '\n' or it reaches the '\0'
      terminator. In other words, 'sources' should be a null-terminated
      string.
      
      However, grab_file() just mmaps a file, so 'sources' is not terminated
      with null byte. If the file does not contain '\n' at all, strchr() will
      go beyond the mmap'ed memory.
      
      Use read_text_file(), which loads the file content into a malloc'ed
      buffer, appending null byte.
      
      Here we are interested only in the first line of *.mod files. Use
      get_line() helper to get the first line.
      
      This also makes missing *.mod file a fatal error.
      
      Commit 4be40e22 ("kbuild: do not emit src version warning for
      non-modules") ignored missing *.mod files.
      
      I do not fully understand what that commit addressed, but commit
      91341d4b ("kbuild: introduce new option to enhance section mismatch
      analysis") introduced partial section checks by using modpost. built-in.o
      was parsed by modpost. Even modules had a problem because *.mod files
      were created after the modpost check.
      
      Commit b7dca6dd ("kbuild: create *.mod with full directory path and
      remove MODVERDIR") stopped doing that. Now that modpost is only invoked
      after the directory descend, *.mod files should always exist at the
      modpost stage.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      f531c1b5
    • Masahiro Yamada's avatar
      modpost: add read_text_file() and get_line() helpers · ac5100f5
      Masahiro Yamada authored
      modpost uses grab_file() to open a file, but it is not suitable for
      a text file because the mmap'ed file is not terminated by null byte.
      Actually, I see some issues for the use of grab_file().
      
      The new helper, read_text_file() loads the whole file content into a
      malloc'ed buffer, and appends a null byte. Then, get_line() reads
      each line.
      
      To handle text files, I intend to replace as follows:
      
        grab_file()    -> read_text_file()
        get_new_line() -> get_line()
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      ac5100f5
    • Masahiro Yamada's avatar
      modpost: do not call get_modinfo() for vmlinux(.o) · 4ddea2f8
      Masahiro Yamada authored
      The three calls of get_modinfo() ("license", "import_ns", "version")
      always return NULL for vmlinux(.o) because the built-in module info is
      prefixed with __MODULE_INFO_PREFIX.
      
      It is harmless to call get_modinfo(), but there is no point to search
      for what apparently does not exist.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      4ddea2f8
    • Masahiro Yamada's avatar
      modpost: drop RCS/CVS $Revision handling in MODULE_VERSION() · f6931535
      Masahiro Yamada authored
      As far as I understood, this code gets rid of '$Revision$' or '$Revision:'
      of CVS, RCS or whatever in MODULE_VERSION() tags.
      
      Remove the primeval code.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      f6931535
    • Masahiro Yamada's avatar
      modpost: show warning if any of symbol dump files is missing · 48a0f727
      Masahiro Yamada authored
      If modpost fails to load a symbol dump file, it cannot check unresolved
      symbols, hence module dependency will not be added. Nor CRCs can be added.
      
      Currently, external module builds check only $(objtree)/Module.symvers,
      but it should check files specified by KBUILD_EXTRA_SYMBOLS as well.
      
      Move the warning message from the top Makefile to scripts/Makefile.modpost
      and print the warning if any dump file is missing.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      48a0f727
    • Masahiro Yamada's avatar
      modpost: show warning if vmlinux is not found when processing modules · 7e8a3235
      Masahiro Yamada authored
      check_exports() does not print warnings about unresolved symbols if
      vmlinux is missing because there would be too many.
      
      This situation happens when you do 'make modules' from the clean
      tree, or compile external modules against a kernel tree that has
      not been completely built.
      
      It is dangerous to not check unresolved symbols because you might be
      building useless modules. At least it should be warned.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      7e8a3235
    • Masahiro Yamada's avatar
      modpost: invoke modpost only when input files are updated · 436b2ac6
      Masahiro Yamada authored
      Currently, the second pass of modpost is always invoked when you run
      'make' or 'make modules' even if none of modules is changed.
      
      Use if_changed to invoke it only when it is necessary.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      436b2ac6
    • Masahiro Yamada's avatar
      modpost: generate vmlinux.symvers and reuse it for the second modpost · 269a535c
      Masahiro Yamada authored
      The full build runs modpost twice, first for vmlinux.o and second for
      modules.
      
      The first pass dumps all the vmlinux symbols into Module.symvers, but
      the second pass parses vmlinux again instead of reusing the dump file,
      presumably because it needs to avoid accumulating stale symbols.
      
      Loading symbol info from a dump file is faster than parsing an ELF object.
      Besides, modpost deals with various issues to parse vmlinux in the second
      pass.
      
      A solution is to make the first pass dumps symbols into a separate file,
      vmlinux.symvers. The second pass reads it, and parses module .o files.
      The merged symbol information is dumped into Module.symvers in the same
      way as before.
      
      This makes further modpost cleanups possible.
      
      Also, it fixes the problem of 'make vmlinux', which previously overwrote
      Module.symvers, throwing away module symbols.
      
      I slightly touched scripts/link-vmlinux.sh so that vmlinux is re-linked
      when you cross this commit. Otherwise, vmlinux.symvers would not be
      generated.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      269a535c
    • Masahiro Yamada's avatar
      modpost: refactor -i option calculation · f1005b30
      Masahiro Yamada authored
      Prepare to use -i for in-tree modpost as well.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      f1005b30
    • Masahiro Yamada's avatar
      modpost: print symbol dump file as the build target in short log · bcfedae7
      Masahiro Yamada authored
      The symbol dump *.symvers is the output of modpost. Print it in
      the short log.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      bcfedae7
    • Masahiro Yamada's avatar
      modpost: re-add -e to set external_module flag · e3fb4df7
      Masahiro Yamada authored
      Previously, the -i option had two functions; load a symbol dump file,
      and set the external_module flag.
      
      I want to assign a dedicate option for each of them.
      
      Going forward, the -i is used to load a symbol dump file, and the -e
      to set the external_module flag.
      
      With this, we will be able to use -i for loading in-kernel symbols.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      e3fb4df7
    • Masahiro Yamada's avatar
      modpost: rename ext_sym_list to dump_list · 7924799e
      Masahiro Yamada authored
      The -i option is used to include Modules.symver as well as files from
      $(KBUILD_EXTRA_SYMBOLS).
      
      Make the struct and variable names more generic.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      7924799e
    • Masahiro Yamada's avatar
      modpost: allow to pass -i option multiple times to remove -e option · ce2ddd6d
      Masahiro Yamada authored
      Now that there is no difference between -i and -e, they can be unified.
      
      Make modpost accept the -i option multiple times, then remove -e.
      
      I will reuse -e for a different purpose.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      ce2ddd6d
    • Masahiro Yamada's avatar
      modpost: track if the symbol origin is a dump file or ELF object · 52c3416d
      Masahiro Yamada authored
      The meaning of sym->kernel is obscure; it is set for in-kernel symbols
      loaded from Modules.symvers. This happens only when we are building
      external modules, and it is used to determine whether to dump symbols
      to $(KBUILD_EXTMOD)/Modules.symvers
      
      It is clearer to remember whether the symbol or module came from a dump
      file or ELF object.
      
      This changes the KBUILD_EXTRA_SYMBOLS behavior. Previously, symbols
      loaded from KBUILD_EXTRA_SYMBOLS are accumulated into the current
      $(KBUILD_EXTMOD)/Modules.symvers
      
      Going forward, they will be only used to check symbol references, but
      not dumped into the current $(KBUILD_EXTMOD)/Modules.symvers. I believe
      this makes more sense.
      
      sym->vmlinux will have no user. Remove it too.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      52c3416d
  2. 03 Jun, 2020 6 commits
    • Masahiro Yamada's avatar
      modpost: load KBUILD_EXTRA_SYMBOLS files in order · 2beee868
      Masahiro Yamada authored
      Currently, modpost reads extra symbol dump files in the reverse order.
      If '-e foo -e bar' is given, modpost reads bar, foo, in this order.
      
      This is probably not a big deal, but there is no good reason to reverse
      the order. Read files in the given order.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      2beee868
    • Masahiro Yamada's avatar
      modpost: pass -N option only for modules modpost · 4e5ab74c
      Masahiro Yamada authored
      The built-in only code is not required to have MODULE_IMPORT_NS() to
      use symbols. So, the namespace is not checked for vmlinux(.o).
      
      Do not pass the meaningless -N option to the first pass of modpost.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      4e5ab74c
    • Masahiro Yamada's avatar
      modpost: move -T option close to the modpost command · 89d61176
      Masahiro Yamada authored
      The '-T -' option reads the file list from stdin.
      
      It is clearer to put it close to the piped command.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      89d61176
    • Masahiro Yamada's avatar
      modpost: fix -i (--ignore-errors) MAKEFLAGS detection · 91e6ee58
      Masahiro Yamada authored
      $(filter -i,$(MAKEFLAGS)) works only in limited use-cases.
      
      The representation of $(MAKEFLAGS) depends on various factors:
        - GNU Make version (version 3.8x or version 4.x)
        - The presence of other flags like -j
      
      In my experiments, $(MAKEFLAGS) is expanded as follows:
      
        * GNU Make 3.8x:
      
          * without -j option:
            --no-print-directory -Rri
      
          * with -j option:
            --no-print-directory -Rr --jobserver-fds=3,4 -j -i
      
        * GNU Make 4.x:
      
          * without -j option:
            irR --no-print-directory
      
          * with -j option:
            irR -j --jobserver-fds=3,4 --no-print-directory
      
      For GNU Make 4.x, the flags are grouped as 'irR', which does not work.
      
      For the single thread build with GNU Make 3.8x, the flags are grouped
      as '-Rri', which does not work either.
      
      To make it work for all cases, do likewise as commit 6f0fa58e
      ("kbuild: simplify silent build (-s) detection").
      
      BTW, since commit ff9b45c5 ("kbuild: modpost: read modules.order
      instead of $(MODVERDIR)/*.mod"), you also need to pass -k option to
      build final *.ko files. 'make -i -k' ignores compile errors in modules,
      and build as many remaining *.ko as possible.
      
      Please note this feature is kind of dangerous if other modules depend
      on the broken module because the generated modules will lack the correct
      module dependency or CRC. Honestly, I am not a big fan of it, but I am
      keeping this feature.
      
      Fixes: eed380f3 ("modpost: Optionally ignore secondary errors seen if a single module build fails")
      Cc: Guenter Roeck <linux@roeck-us.net>
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      91e6ee58
    • Masahiro Yamada's avatar
      kbuild: update modules.order only when contained modules are updated · b2c88554
      Masahiro Yamada authored
      Make modules.order depend on $(obj-m), and use if_changed to build it.
      This will avoid unneeded update of modules.order, which will be useful
      to optimize the modpost stage.
      
      Currently, the second pass of modpost is always invoked. By checking the
      timestamp of modules.order, we can avoid the unneeded modpost.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      b2c88554
    • Masahiro Yamada's avatar
      kbuild: refactor KBUILD_VMLINUX_{OBJS,LIBS} calculation · f0d50ca0
      Masahiro Yamada authored
      Do not overwrite core-y or drivers-y. Remove libs-y1 and libs-y2.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      f0d50ca0
  3. 01 Jun, 2020 6 commits
  4. 28 May, 2020 2 commits
    • Nick Desaulniers's avatar
      Makefile: support compressed debug info · 10e68b02
      Nick Desaulniers authored
      As debug information gets larger and larger, it helps significantly save
      the size of vmlinux images to compress the information in the debug
      information sections. Note: this debug info is typically split off from
      the final compressed kernel image, which is why vmlinux is what's used
      in conjunction with GDB. Minimizing the debug info size should have no
      impact on boot times, or final compressed kernel image size.
      
      All of the debug sections will have a `C` flag set.
      $ readelf -S <object file>
      
      $ bloaty vmlinux.gcc75.compressed.dwarf4 -- \
          vmlinux.gcc75.uncompressed.dwarf4
      
          FILE SIZE        VM SIZE
       --------------  --------------
        +0.0%     +18  [ = ]       0    [Unmapped]
       -73.3%  -114Ki  [ = ]       0    .debug_aranges
       -76.2% -2.01Mi  [ = ]       0    .debug_frame
       -73.6% -2.89Mi  [ = ]       0    .debug_str
       -80.7% -4.66Mi  [ = ]       0    .debug_abbrev
       -82.9% -4.88Mi  [ = ]       0    .debug_ranges
       -70.5% -9.04Mi  [ = ]       0    .debug_line
       -79.3% -10.9Mi  [ = ]       0    .debug_loc
       -39.5% -88.6Mi  [ = ]       0    .debug_info
       -18.2%  -123Mi  [ = ]       0    TOTAL
      
      $ bloaty vmlinux.clang11.compressed.dwarf4 -- \
          vmlinux.clang11.uncompressed.dwarf4
      
          FILE SIZE        VM SIZE
       --------------  --------------
        +0.0%     +23  [ = ]       0    [Unmapped]
       -65.6%    -871  [ = ]       0    .debug_aranges
       -77.4% -1.84Mi  [ = ]       0    .debug_frame
       -82.9% -2.33Mi  [ = ]       0    .debug_abbrev
       -73.1% -2.43Mi  [ = ]       0    .debug_str
       -84.8% -3.07Mi  [ = ]       0    .debug_ranges
       -65.9% -8.62Mi  [ = ]       0    .debug_line
       -86.2% -40.0Mi  [ = ]       0    .debug_loc
       -42.0% -64.1Mi  [ = ]       0    .debug_info
       -22.1%  -122Mi  [ = ]       0    TOTAL
      
      For x86_64 defconfig + LLVM=1 (before):
      Elapsed (wall clock) time (h:mm:ss or m:ss): 3:22.03
      Maximum resident set size (kbytes): 43856
      
      For x86_64 defconfig + LLVM=1 (after):
      Elapsed (wall clock) time (h:mm:ss or m:ss): 3:32.52
      Maximum resident set size (kbytes): 1566776
      
      Thanks to:
      Nick Clifton helped us to provide the minimal binutils version.
      Sedat Dilek found an increase in size of debug .deb package.
      
      Cc: Nick Clifton <nickc@redhat.com>
      Suggested-by: default avatarDavid Blaikie <blaikie@google.com>
      Reviewed-by: default avatarFangrui Song <maskray@google.com>
      Tested-by: default avatarSedat Dilek <sedat.dilek@gmail.com>
      Signed-off-by: default avatarNick Desaulniers <ndesaulniers@google.com>
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      10e68b02
    • Masahiro Yamada's avatar
      modpost: refactor sech_name() · 565587d8
      Masahiro Yamada authored
      Use sym_get_data_by_offset() helper to get access to the .shstrtab
      section data. No functional change is intended because
      elf->sechdrs[elf->secindex_strings].sh_addr is 0 for both ET_REL
      and ET_EXEC object types.
      Signed-off-by: default avatarMasahiro Yamada <masahiroy@kernel.org>
      565587d8