From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd2f.google.com (mail-io1-xd2f.google.com [IPv6:2607:f8b0:4864:20::d2f]) by sourceware.org (Postfix) with ESMTPS id 1B1923858D1E for ; Fri, 11 Nov 2022 15:45:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1B1923858D1E Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=google.com Received: by mail-io1-xd2f.google.com with SMTP id y6so3807112iof.9 for ; Fri, 11 Nov 2022 07:45:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=qc1j/nmIi+vFuZTHrlOk2zTsnFjBB7Ln/pRp6YzWD70=; b=jirWi8HNcZivB9yYj2kZZLWRqlbH5+I7fzYy0u94fn8Lb48jALRVIQ5DaFdQkBOCeX jWtGMNQo8uj7b7foyc7AtYNPOYXGPbL4104hq3d5Mf+4go/StMc89r+jz0mWO7c4IKCa 2EdkPW0qy87SS/AB4mI5Kj+a7S4XVI4/3WqQBUR1skfypC7vupZF+PZ/iohKdWBqgQvC B6XnxWhJVuoIuDs1dldGiTjOAT4pMkuEBMB9NMFJ174X1b5LNmhaICSCsXDqIa2tcNOp r2mZdZy80mfew3NxcCmZtf4CmICadUub5ed3v1ePIdRUckl5oNVYkD/vZiXV7sF+voev EhGw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=qc1j/nmIi+vFuZTHrlOk2zTsnFjBB7Ln/pRp6YzWD70=; b=ZgQfrHfI+EG6bww5T2gDi0AuIMPhRDOHrKlaxTEBz+ytQ2H9eGEHlAJWLR2gR18tZQ n46jFTlyPjW1xwzr71Wzk8b/hxdgKBoEQ7APF8r3qtl3RzsmxpKD/Cx6JRtTTo9Sl0vY cdXfs1cXEUgn/GVPcJOaD72Zcxm0Lm59yL9q4MDScTjgVrRasq7Ge4j2YtBa3n6X/rnH CAmmk7ezvZm97Z0Rqin/yCznc7+qz618TxvIWeOLa0sAhHDjchva91cKZgGVtWnmDwko EWnDfAn6yICPmnDFmeAbVGt42NIbQXkHz9aAzyONVDm6isgLKnvV3V+I9oM6WSwMuzxl r8hA== X-Gm-Message-State: ANoB5pmW6Wd0+NvHNm6tyT9/wjATwKdVzW0MX1KniV4KLfX6jx402Etu 9voHdjgnfSlBBUGspPVahzpfvYxYnV98S5s+thhluMLA7rM= X-Google-Smtp-Source: AA0mqf7pfVILgnWGWFwcxMa9keZmIsTnpr3sy13QxMoneIqCcafAAiMv4U5thkz95pe+1Vml7BSeJE6C6YayDT3jLwU= X-Received: by 2002:a05:6602:1309:b0:6d1:8a5f:eae2 with SMTP id h9-20020a056602130900b006d18a5feae2mr1221315iov.80.1668181510766; Fri, 11 Nov 2022 07:45:10 -0800 (PST) MIME-Version: 1.0 References: <20221027140928.1480353-1-gprocida@google.com> <20221111132151.798754-1-gprocida@google.com> In-Reply-To: <20221111132151.798754-1-gprocida@google.com> From: Giuliano Procida Date: Fri, 11 Nov 2022 15:44:34 +0000 Message-ID: Subject: Re: [PATCH v2] Narrow Linux symbol CRCs to 32 bits To: libabigail@sourceware.org Cc: dodji@seketeli.org, kernel-team@android.com, maennich@google.com, sidnayyar@google.com, vvvvvv@google.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-27.1 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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 to optional. > (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 to optional. > (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 to > optional. > * src/abg-symtab-reader.cc (symtab::load_): Change crc_values > value type from uint64_t to uint32_t. > > Signed-off-by: Giuliano Procida > --- > 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& crc = {}, > + const abg_compat::optional& crc = {}, > const abg_compat::optional& ns = {}, > bool is_suppressed = false); > > @@ -966,7 +966,7 @@ public: > const version& ve, > visibility vi, > bool is_in_ksymtab = false, > - const abg_compat::optional& crc = {}, > + const abg_compat::optional& crc = {}, > const abg_compat::optional& ns = {}, > bool is_suppressed = false); > > @@ -1039,11 +1039,11 @@ public: > void > set_is_in_ksymtab(bool is_in_ksymtab); > > - const abg_compat::optional& > + const abg_compat::optional& > get_crc() const; > > void > - set_crc(const abg_compat::optional& crc); > + set_crc(const abg_compat::optional& crc); > > const abg_compat::optional& > 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 crc_; > + abg_compat::optional crc_; > abg_compat::optional 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& crc, > + const abg_compat::optional& crc, > const abg_compat::optional& 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& crc, > + const abg_compat::optional& crc, > const abg_compat::optional& 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& crc, > + const abg_compat::optional& crc, > const abg_compat::optional& 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& > +const abg_compat::optional& > 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& crc) > +elf_symbol::set_crc(const abg_compat::optional& 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& crc1 = symbol1->get_crc(); > - const abg_compat::optional& crc2 = symbol2->get_crc(); > + const abg_compat::optional& crc1 = symbol1->get_crc(); > + const abg_compat::optional& 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 exported_kernel_symbols; > - std::unordered_map crc_values; > + std::unordered_map crc_values; > std::unordered_map namespaces; > > for (size_t i = 0; i < number_syms; ++i) > -- > 2.38.1.493.g58b659f92b-goog >