[v2] Narrow Linux symbol CRCs to 32 bits

  MODVERSIONS CRCs are 32-bit hashes of strings representing C type
elements or typed symbols. The hashes are calculated using a 32-bit
CRC, hence the name. The kernel module loading code (implicitly)
truncates any provided CRC value to 32 bits before comparing it with
anything.

When support was added to libabigail, values up to 64 bits wide were
supported. This change narrows libabigail's concept of Linux CRC to 32
bits. No tests are affected.

	* include/abg-ir.h (elf_symbol::elf_symbol): Change CRC type
	from optional<uint64_t> to optional<uint32_t>.
	(elf_symbol::create): Likewise.
	(elf_symbol::get_crc): Likewise.
	(elf_symbol::set_crc): Likewise.
	* src/abg-ir.cc (elf_symbol::priv) Change CRC type from
	optional<uint64_t> to optional<uint32_t>.
	(elf_symbol::priv::priv): Likewise.
	(elf_symbol::elf_symbol): Likewise.
	(elf_symbol::create): Likewise.
	(elf_symbol::get_crc): Likewise.
	(elf_symbol::set_crc): Likewise.
	* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
	Change CRC type from optional<uint64_t> to
	optional<uint32_t>.
	* src/abg-symtab-reader.cc (symtab::load_): Change crc_values
	value type from uint64_t to uint32_t.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 include/abg-ir.h         |  8 ++++----
 src/abg-ir.cc            | 12 ++++++------
 src/abg-reporter-priv.cc |  4 ++--
 src/abg-symtab-reader.cc |  2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)
  

I forgot to mention.

A colleague is working on a fix that will continue from this commit.
Should be ready for review next week.

Giuliano.

On Fri, 11 Nov 2022 at 13:22, Giuliano Procida <gprocida@google.com> wrote:
>
> MODVERSIONS CRCs are 32-bit hashes of strings representing C type
> elements or typed symbols. The hashes are calculated using a 32-bit
> CRC, hence the name. The kernel module loading code (implicitly)
> truncates any provided CRC value to 32 bits before comparing it with
> anything.
>
> When support was added to libabigail, values up to 64 bits wide were
> supported. This change narrows libabigail's concept of Linux CRC to 32
> bits. No tests are affected.
>
>         * include/abg-ir.h (elf_symbol::elf_symbol): Change CRC type
>         from optional<uint64_t> to optional<uint32_t>.
>         (elf_symbol::create): Likewise.
>         (elf_symbol::get_crc): Likewise.
>         (elf_symbol::set_crc): Likewise.
>         * src/abg-ir.cc (elf_symbol::priv) Change CRC type from
>         optional<uint64_t> to optional<uint32_t>.
>         (elf_symbol::priv::priv): Likewise.
>         (elf_symbol::elf_symbol): Likewise.
>         (elf_symbol::create): Likewise.
>         (elf_symbol::get_crc): Likewise.
>         (elf_symbol::set_crc): Likewise.
>         * src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
>         Change CRC type from optional<uint64_t> to
>         optional<uint32_t>.
>         * src/abg-symtab-reader.cc (symtab::load_): Change crc_values
>         value type from uint64_t to uint32_t.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  include/abg-ir.h         |  8 ++++----
>  src/abg-ir.cc            | 12 ++++++------
>  src/abg-reporter-priv.cc |  4 ++--
>  src/abg-symtab-reader.cc |  2 +-
>  4 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/include/abg-ir.h b/include/abg-ir.h
> index 4892f0e2..546603f4 100644
> --- a/include/abg-ir.h
> +++ b/include/abg-ir.h
> @@ -940,7 +940,7 @@ private:
>              const version&     ve,
>              visibility         vi,
>              bool               is_in_ksymtab = false,
> -            const abg_compat::optional<uint64_t>&      crc = {},
> +            const abg_compat::optional<uint32_t>&      crc = {},
>              const abg_compat::optional<std::string>&   ns = {},
>              bool               is_suppressed = false);
>
> @@ -966,7 +966,7 @@ public:
>          const version&     ve,
>          visibility         vi,
>          bool               is_in_ksymtab = false,
> -        const abg_compat::optional<uint64_t>&          crc = {},
> +        const abg_compat::optional<uint32_t>&          crc = {},
>          const abg_compat::optional<std::string>&       ns = {},
>          bool               is_suppressed = false);
>
> @@ -1039,11 +1039,11 @@ public:
>    void
>    set_is_in_ksymtab(bool is_in_ksymtab);
>
> -  const abg_compat::optional<uint64_t>&
> +  const abg_compat::optional<uint32_t>&
>    get_crc() const;
>
>    void
> -  set_crc(const abg_compat::optional<uint64_t>& crc);
> +  set_crc(const abg_compat::optional<uint32_t>& crc);
>
>    const abg_compat::optional<std::string>&
>    get_namespace() const;
> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> index 5193934a..a043a532 100644
> --- a/src/abg-ir.cc
> +++ b/src/abg-ir.cc
> @@ -1788,7 +1788,7 @@ struct elf_symbol::priv
>    //     STT_COMMON definition of that name that has the largest size.
>    bool                 is_common_;
>    bool                 is_in_ksymtab_;
> -  abg_compat::optional<uint64_t>       crc_;
> +  abg_compat::optional<uint32_t>       crc_;
>    abg_compat::optional<std::string>    namespace_;
>    bool                 is_suppressed_;
>    elf_symbol_wptr      main_symbol_;
> @@ -1822,7 +1822,7 @@ struct elf_symbol::priv
>         const elf_symbol::version& ve,
>         elf_symbol::visibility    vi,
>         bool                      is_in_ksymtab,
> -       const abg_compat::optional<uint64_t>&   crc,
> +       const abg_compat::optional<uint32_t>&   crc,
>         const abg_compat::optional<std::string>&        ns,
>         bool                      is_suppressed)
>      : env_(e),
> @@ -1896,7 +1896,7 @@ elf_symbol::elf_symbol(const environment* e,
>                        const version&     ve,
>                        visibility         vi,
>                        bool               is_in_ksymtab,
> -                      const abg_compat::optional<uint64_t>&    crc,
> +                      const abg_compat::optional<uint32_t>&    crc,
>                        const abg_compat::optional<std::string>& ns,
>                        bool               is_suppressed)
>    : priv_(new priv(e,
> @@ -1971,7 +1971,7 @@ elf_symbol::create(const environment* e,
>                    const version&     ve,
>                    visibility         vi,
>                    bool               is_in_ksymtab,
> -                  const abg_compat::optional<uint64_t>&        crc,
> +                  const abg_compat::optional<uint32_t>&        crc,
>                    const abg_compat::optional<std::string>&     ns,
>                    bool               is_suppressed)
>  {
> @@ -2208,7 +2208,7 @@ elf_symbol::set_is_in_ksymtab(bool is_in_ksymtab)
>  /// Getter of the 'crc' property.
>  ///
>  /// @return the CRC (modversions) value for Linux Kernel symbols, if any
> -const abg_compat::optional<uint64_t>&
> +const abg_compat::optional<uint32_t>&
>  elf_symbol::get_crc() const
>  {return priv_->crc_;}
>
> @@ -2216,7 +2216,7 @@ elf_symbol::get_crc() const
>  ///
>  /// @param crc the new CRC (modversions) value for Linux Kernel symbols
>  void
> -elf_symbol::set_crc(const abg_compat::optional<uint64_t>& crc)
> +elf_symbol::set_crc(const abg_compat::optional<uint32_t>& crc)
>  {priv_->crc_ = crc;}
>
>  /// Getter of the 'namespace' property.
> diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
> index 2ecf8016..70839085 100644
> --- a/src/abg-reporter-priv.cc
> +++ b/src/abg-reporter-priv.cc
> @@ -1149,8 +1149,8 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr&       symbol1,
>           << "\n";
>      }
>
> -  const abg_compat::optional<uint64_t>& crc1 = symbol1->get_crc();
> -  const abg_compat::optional<uint64_t>& crc2 = symbol2->get_crc();
> +  const abg_compat::optional<uint32_t>& crc1 = symbol1->get_crc();
> +  const abg_compat::optional<uint32_t>& crc2 = symbol2->get_crc();
>    if (crc1 != crc2)
>      {
>        const std::string none = "(none)";
> diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc
> index 606c96a2..8a566f8d 100644
> --- a/src/abg-symtab-reader.cc
> +++ b/src/abg-symtab-reader.cc
> @@ -265,7 +265,7 @@ symtab::load_(Elf*         elf_handle,
>
>    const bool is_kernel = elf_helpers::is_linux_kernel(elf_handle);
>    std::unordered_set<std::string> exported_kernel_symbols;
> -  std::unordered_map<std::string, uint64_t> crc_values;
> +  std::unordered_map<std::string, uint32_t> crc_values;
>    std::unordered_map<std::string, std::string> namespaces;
>
>    for (size_t i = 0; i < number_syms; ++i)
> --
> 2.38.1.493.g58b659f92b-goog
>
  

Hello Giuliano,

Giuliano Procida <gprocida@google.com> a écrit:

> MODVERSIONS CRCs are 32-bit hashes of strings representing C type
> elements or typed symbols. The hashes are calculated using a 32-bit
> CRC, hence the name. The kernel module loading code (implicitly)
> truncates any provided CRC value to 32 bits before comparing it with
> anything.
>
> When support was added to libabigail, values up to 64 bits wide were
> supported. This change narrows libabigail's concept of Linux CRC to 32
> bits. No tests are affected.
>
> 	* include/abg-ir.h (elf_symbol::elf_symbol): Change CRC type
> 	from optional<uint64_t> to optional<uint32_t>.
> 	(elf_symbol::create): Likewise.
> 	(elf_symbol::get_crc): Likewise.
> 	(elf_symbol::set_crc): Likewise.
> 	* src/abg-ir.cc (elf_symbol::priv) Change CRC type from
> 	optional<uint64_t> to optional<uint32_t>.
> 	(elf_symbol::priv::priv): Likewise.
> 	(elf_symbol::elf_symbol): Likewise.
> 	(elf_symbol::create): Likewise.
> 	(elf_symbol::get_crc): Likewise.
> 	(elf_symbol::set_crc): Likewise.
> 	* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
> 	Change CRC type from optional<uint64_t> to
> 	optional<uint32_t>.
> 	* src/abg-symtab-reader.cc (symtab::load_): Change crc_values
> 	value type from uint64_t to uint32_t.

Applied to master, thanks!

[...]

Cheers,