Re: [PATCH 5/8] abg-elf-helpers: consolidate the is_linux_kernel* helpers

[Giuliano Procida <gprocida@google.com>]

Looks good.

On Mon, 20 Apr 2020 at 12:09, Matthias Maennich <maennich@google.com> wrote:
>
> This consistently names the is_linux_kernel* helpers and keeps only the
> copy in the elf-helpers. All users are adjusted.
>
>         * src/abg-dwarf-reader.cc
>         (read_context::function_symbol_is_exported): use is_linux_kernel
>         from elf_helpers.
>         (read_context::variable_is_exported): Likewise.
>         (read_context::get_symtab_format): Likewise.
>         (read_context::load_symbol_maps): Likewise.
>         (read_debug_info_into_corpus): Likewise.
>         (read_context::is_linux_kernel_binary): Drop function.
>         (read_context::is_linux_kernel_module): Drop function.
>         * src/abg-elf-helpers.cc (binary_is_linux_kernel): rename to
>         is_linux_kernel
>         (binary_is_linux_kernel_module): rename to is_linux_kernel_module
>         (find_ksymtab_strings_section): Adjust to function renames.
>         * src/abg-elf-helpers.h (binary_is_linux_kernel): rename to
>         is_linux_kernel
>         (binary_is_linux_kernel_module): rename to is_linux_kernel_module

Reviewed-by: Giuliano Procida <gprocida@google.com>

> Signed-off-by: Matthias Maennich <maennich@google.com>
> ---
>  src/abg-dwarf-reader.cc | 37 +++++++------------------------------
>  src/abg-elf-helpers.cc  |  8 ++++----
>  src/abg-elf-helpers.h   |  4 ++--
>  3 files changed, 13 insertions(+), 36 deletions(-)
>
> diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
> index 3adb978b784b..ec1f9f3fe8f3 100644
> --- a/src/abg-dwarf-reader.cc
> +++ b/src/abg-dwarf-reader.cc
> @@ -5939,7 +5939,7 @@ public:
>
>      address_set_sptr set;
>      bool looking_at_linux_kernel_binary =
> -      load_in_linux_kernel_mode() && is_linux_kernel_binary();
> +      load_in_linux_kernel_mode() && is_linux_kernel(elf_handle());
>
>      if (looking_at_linux_kernel_binary)
>        {
> @@ -5979,7 +5979,7 @@ public:
>
>      address_set_sptr set;
>      bool looking_at_linux_kernel_binary =
> -      load_in_linux_kernel_mode() && is_linux_kernel_binary();
> +      load_in_linux_kernel_mode() && is_linux_kernel(elf_handle());
>
>      if (looking_at_linux_kernel_binary)
>        {
> @@ -6920,7 +6920,7 @@ public:
>           {
>             // Since Linux kernel modules are relocatable, we can first try
>             // using a heuristic based on relocations to guess the ksymtab format.
> -           if (is_linux_kernel_module())
> +           if (is_linux_kernel_module(elf_handle()))
>              {
>                ksymtab_format_ = get_ksymtab_format_module();
>                if (ksymtab_format_ != UNDEFINED_KSYMTAB_FORMAT)
> @@ -7467,7 +7467,7 @@ public:
>                                                  load_undefined_fun_map,
>                                                  load_undefined_var_map))
>           {
> -           if (load_in_linux_kernel_mode() && is_linux_kernel_binary())
> +           if (load_in_linux_kernel_mode() && is_linux_kernel(elf_handle()))
>               return load_linux_specific_exported_symbol_maps();
>             return true;
>           }
> @@ -8166,30 +8166,6 @@ public:
>    load_in_linux_kernel_mode(bool f)
>    {options_.load_in_linux_kernel_mode = f;}
>
> -  /// Guess if the current binary is a Linux Kernel or a Linux Kernel module.
> -  ///
> -  /// To guess that, the function looks for the presence of the
> -  /// special "__ksymtab_strings" section in the binary.
> -  ///
> -  bool
> -  is_linux_kernel_binary() const
> -  {
> -    return find_section(elf_handle(), "__ksymtab_strings", SHT_PROGBITS)
> -          || is_linux_kernel_module();
> -  }
> -
> -  /// Guess if the current binary is a Linux Kernel module.
> -  ///
> -  /// To guess that, the function looks for the presence of the special
> -  /// ".modinfo" and ".gnu.linkonce.this_module" sections in the binary.
> -  ///
> -  bool
> -  is_linux_kernel_module() const
> -  {
> -    return find_section(elf_handle(), ".modinfo", SHT_PROGBITS)
> -          && find_section(elf_handle(), ".gnu.linkonce.this_module", SHT_PROGBITS);
> -  }
> -
>    /// Getter of the "show_stats" flag.
>    ///
>    /// This flag tells if we should emit statistics about various
> @@ -16322,7 +16298,7 @@ read_debug_info_into_corpus(read_context& ctxt)
>    // First set some mundane properties of the corpus gathered from
>    // ELF.
>    ctxt.current_corpus()->set_path(ctxt.elf_path());
> -  if (ctxt.is_linux_kernel_binary())
> +  if (is_linux_kernel(ctxt.elf_handle()))
>      ctxt.current_corpus()->set_origin(corpus::LINUX_KERNEL_BINARY_ORIGIN);
>    else
>      ctxt.current_corpus()->set_origin(corpus::DWARF_ORIGIN);
> @@ -16335,7 +16311,8 @@ read_debug_info_into_corpus(read_context& ctxt)
>    // Set symbols information to the corpus.
>    if (!get_ignore_symbol_table(ctxt))
>      {
> -      if (ctxt.load_in_linux_kernel_mode() && ctxt.is_linux_kernel_binary())
> +      if (ctxt.load_in_linux_kernel_mode()
> +         && is_linux_kernel(ctxt.elf_handle()))
>         {
>           string_elf_symbols_map_sptr exported_fn_symbols_map
>             (new string_elf_symbols_map_type);
> diff --git a/src/abg-elf-helpers.cc b/src/abg-elf-helpers.cc
> index ff0941dc6536..ede191014369 100644
> --- a/src/abg-elf-helpers.cc
> +++ b/src/abg-elf-helpers.cc
> @@ -558,7 +558,7 @@ get_symbol_versionning_sections(Elf*                elf_handle,
>  Elf_Scn*
>  find_ksymtab_strings_section(Elf *elf_handle)
>  {
> -  if (binary_is_linux_kernel(elf_handle))
> +  if (is_linux_kernel(elf_handle))
>      return find_section(elf_handle, "__ksymtab_strings", SHT_PROGBITS);
>    return 0;
>  }
> @@ -773,7 +773,7 @@ get_version_for_symbol(Elf*                 elf_handle,
>  /// @return true iff the binary denoted by @p elf_handle is a Linux
>  /// kernel module.
>  bool
> -binary_is_linux_kernel_module(Elf *elf_handle)
> +is_linux_kernel_module(Elf *elf_handle)
>  {
>    return (find_section(elf_handle, ".modinfo", SHT_PROGBITS)
>           && find_section(elf_handle,
> @@ -789,12 +789,12 @@ binary_is_linux_kernel_module(Elf *elf_handle)
>  /// @return true iff the binary denoted by @p elf_handle is a Linux
>  /// kernel binary
>  bool
> -binary_is_linux_kernel(Elf *elf_handle)
> +is_linux_kernel(Elf *elf_handle)
>  {
>    return (find_section(elf_handle,
>                        "__ksymtab_strings",
>                        SHT_PROGBITS)
> -         || binary_is_linux_kernel_module(elf_handle));
> +         || is_linux_kernel_module(elf_handle));
>  }
>
>  } // end namespace elf_helpers
> diff --git a/src/abg-elf-helpers.h b/src/abg-elf-helpers.h
> index 33348126028c..7ddd887de959 100644
> --- a/src/abg-elf-helpers.h
> +++ b/src/abg-elf-helpers.h
> @@ -132,10 +132,10 @@ get_version_for_symbol(Elf*                       elf_handle,
>  //
>
>  bool
> -binary_is_linux_kernel_module(Elf *elf_handle);
> +is_linux_kernel_module(Elf *elf_handle);
>
>  bool
> -binary_is_linux_kernel(Elf *elf_handle);
> +is_linux_kernel(Elf *elf_handle);
>
>
>  } // end namespace elf_helpers
> --
> 2.26.1.301.g55bc3eb7cb9-goog
>