From patchwork Mon Mar 21 16:02:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 52179 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 5C6573860825 for ; Mon, 21 Mar 2022 16:02:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5C6573860825 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1647878554; bh=97sbJG6epk6xgM6PR2EhU60ZnF/WkzV5P0dALsI2fbs=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=hp+H/mcMSvH3vq3UdgtgAQfU2QDPA2Pp8p7EpVTTBleJuJTJ72lI7O4PKVgUFKe7W 5rgc2if+asQKluQlCf8XWTlfNHduGSvP4LvbyiBAzOk0ou5Mg0MmxXSBw+jWTsU3HN RbIjo1gjGscBCiZC5viI8SNftTQa1RADwOM+iNNg= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-ej1-x64a.google.com (mail-ej1-x64a.google.com [IPv6:2a00:1450:4864:20::64a]) by sourceware.org (Postfix) with ESMTPS id 448423857419 for ; Mon, 21 Mar 2022 16:02:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 448423857419 Received: by mail-ej1-x64a.google.com with SMTP id hy26-20020a1709068a7a00b006dfa034862cso5064157ejc.23 for ; Mon, 21 Mar 2022 09:02:28 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=97sbJG6epk6xgM6PR2EhU60ZnF/WkzV5P0dALsI2fbs=; b=mKOv3MQCiOot4zYxyDmAkLXmfU1/IYJg8YVHrWw0m/x18QEgmeEQvVBupeSazh/w61 DZw77lHDCCYm4ZDE9H1wdViHzE5h3DJSXS4L42fTo9S1B5X/RZZ68Os0oxYacrFrUpcs ZJXFOrF35nvnLOHYynnPc0TQF6chU4GkJiLLi+fnrsjWETJA1UjFhF9GVAYGq0GcznKR t+fTCVd/AhVE5fiROhUvhmcSoIKr5E+GblDyXLyyvE0hHyKftfGWHVwS8mIogC76IA+z 116weVLK+hNOrdmMfzNzuW8tZlWLq6/M1/KjopNbWpZT/Lthhrj1BdkRRZ3+gqyJIzI6 6xZQ== X-Gm-Message-State: AOAM530VbxSZfPcomaZgyRgN9s0LUAuFi2S1WXIhHM0RNpXCXUD8GAdd XJgTxCG9voy4Xg87p+5jmdIF/IY51Xfix2+C9fR3C9fRw3mu5pNFC0h56oXOhPL8K37icWcjHYg LqQIY4RntyIUIZrw2hw1K01HkbMXHz8zJh+7c5WVfYl2Uow81mNR39KJKNR132GveWL62r20= X-Google-Smtp-Source: ABdhPJxhZneo4b77upzwXGOZiD4Ht3u2dvMRqvYS0VOkPS5XuMi/Ca2HkKXJH74Wm04plhC4pXYiWHqSN13z5g== X-Received: from tef.lon.corp.google.com ([2a00:79e0:d:210:45b0:e68c:9bc8:4749]) (user=gprocida job=sendgmr) by 2002:a17:906:1f11:b0:685:d50e:3bf9 with SMTP id w17-20020a1709061f1100b00685d50e3bf9mr22177860ejj.275.1647878546970; Mon, 21 Mar 2022 09:02:26 -0700 (PDT) Date: Mon, 21 Mar 2022 16:02:18 +0000 In-Reply-To: <20220321160221.1372398-1-gprocida@google.com> Message-Id: <20220321160221.1372398-2-gprocida@google.com> Mime-Version: 1.0 References: <20220317163858.353762-1-gprocida@google.com> <20220321160221.1372398-1-gprocida@google.com> X-Mailer: git-send-email 2.35.1.894.gb6a874cedc-goog Subject: [PATCH v4 1/4] crc_changed: eliminate copying of shared_ptr values To: libabigail@sourceware.org X-Spam-Status: No, score=-21.3 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" As pointed out in a review of similar code, it is possible to avoid copying a couple of shared pointers in this function, by taking references instead. This commit also splits declarations to one per line and removes the unnecessary parentheses around the return expression. * src/abg-comp-filter.cc (crc_changed): Take references to avoid std::shared_ptr copying. Split declarations into one per line. Remove unnecessary return expression parentheses. Reviewed-by: Matthias Maennich Signed-off-by: Giuliano Procida --- src/abg-comp-filter.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 56251274..f90fdc78 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -230,11 +230,13 @@ static bool crc_changed(const function_or_var_decl_sptr& f, const function_or_var_decl_sptr& s) { - const auto symbol_f = f->get_symbol(), symbol_s = s->get_symbol(); + const auto& symbol_f = f->get_symbol(); + const auto& symbol_s = s->get_symbol(); if (!symbol_f || !symbol_s) return false; - const auto crc_f = symbol_f->get_crc(), crc_s = symbol_s->get_crc(); - return (crc_f != 0 && crc_s != 0 && crc_f != crc_s); + const auto crc_f = symbol_f->get_crc(); + const auto crc_s = symbol_s->get_crc(); + return crc_f != 0 && crc_s != 0 && crc_f != crc_s; } /// Test if the current diff tree node carries a CRC change in either a From patchwork Mon Mar 21 16:02:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 52180 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 62DD13860004 for ; Mon, 21 Mar 2022 16:02:37 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 62DD13860004 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1647878557; bh=NKbddToFD1QcmBdPRTG0aRcudo0hmqfb5JZbFLp04N4=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=bjW82eU+tZSiqBH2dw4DKYNWu/fVHFPY5tpcAUQ5wHrHzdzeaPqqhGswh0CMSevDI 95HZaM3hHqjTHztgc1+2W0nM2Yaisvlq3a6kV2bJiepYXwwD/UbTGPUtdNgjMsVLz0 RX/h7FQ52PR+fVAkBLC8wIQLuGqktsDgGchWXE2w= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-ed1-x54a.google.com (mail-ed1-x54a.google.com [IPv6:2a00:1450:4864:20::54a]) by sourceware.org (Postfix) with ESMTPS id E5F62385ED4C for ; Mon, 21 Mar 2022 16:02:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E5F62385ED4C Received: by mail-ed1-x54a.google.com with SMTP id b71-20020a509f4d000000b00418d658e9d1so8801869edf.19 for ; Mon, 21 Mar 2022 09:02:30 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=NKbddToFD1QcmBdPRTG0aRcudo0hmqfb5JZbFLp04N4=; b=GzmmKw6Gw0XdnTqR8qIXLfeUbEbc0rSuwMTCh6pisRGd4ml3wcXohLqlwZq79cBqQS BD69TaF0oY6nLEcN+1ocpifcGrKlWIq2QjSaIyDgUObXICGCtMUJDS3IqxHubTnuokVE DkZbnj/vdtcL9foc+pibB//umN8+TdV5uy+tqzQ5y49dQw9L0Q6DNhoMD4gAD/BwU5nI dLHSt3pCg2c8oS4S9wzLhxB2Mjw2aZTwBpO+7Wg25NkXbWmSET7JDJklLhhe+X4F9Idk Cfpts/HZu042paPq0opD/yiGl23lFny5gh81V/w9knP5dUHJRNdcg1ULDNViTBkVjR5h PPDg== X-Gm-Message-State: AOAM530vzo783VvMBoZVRB8la3vQLfuEDOZYV7JZxu2Li+lmWBf6ZAik tgy/4kmigYGeDSJ+LJU6DmX/mpcjBA7NXRK+nbzf4hyVFSB/U2ZA2bVAlJilyTk8f32YYWrEIel 7Sr3GCLDq387wKWq8hIN2h0z7XzgqUWxUqKuBUvzSLIzNUj6diDOe0hBFVtSaxvFgzAy7wcc= X-Google-Smtp-Source: ABdhPJyN4zSVjqk0Kc2OhOiIrT3lW065kkAQDIIp1drPLGxJclWDcXJgw1h4/3RGaR8PeuYvFzR55IrHPFmYag== X-Received: from tef.lon.corp.google.com ([2a00:79e0:d:210:45b0:e68c:9bc8:4749]) (user=gprocida job=sendgmr) by 2002:a17:907:3e82:b0:6da:6f15:ff38 with SMTP id hs2-20020a1709073e8200b006da6f15ff38mr21472229ejc.324.1647878549317; Mon, 21 Mar 2022 09:02:29 -0700 (PDT) Date: Mon, 21 Mar 2022 16:02:19 +0000 In-Reply-To: <20220321160221.1372398-1-gprocida@google.com> Message-Id: <20220321160221.1372398-3-gprocida@google.com> Mime-Version: 1.0 References: <20220317163858.353762-1-gprocida@google.com> <20220321160221.1372398-1-gprocida@google.com> X-Mailer: git-send-email 2.35.1.894.gb6a874cedc-goog Subject: [PATCH v4 2/4] optional: minor improvements To: libabigail@sourceware.org X-Spam-Status: No, score=-21.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" This change makes minor improvements to the optional class used with pre-C++17 compilers. - adds operator== and operator!= - adds various missing noexcept (but not constexpr) decorations - defines operator bool in terms of has_value Note that some constexpr decorations would require C++17 anyway. * include/abg-cxx-compat.h (optional): Add operator== and operator!=. Add noexcept decorations. Tweak operator bool. Reviewed-by: Matthias Maennich Signed-off-by: Giuliano Procida --- include/abg-cxx-compat.h | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/include/abg-cxx-compat.h b/include/abg-cxx-compat.h index 443905c7..5c5943d0 100644 --- a/include/abg-cxx-compat.h +++ b/include/abg-cxx-compat.h @@ -45,7 +45,7 @@ public: optional(const T& value) : has_value_(true), value_(value) {} bool - has_value() const + has_value() const noexcept { return has_value_; } @@ -67,19 +67,19 @@ public: } const T& - operator*() const + operator*() const& noexcept { return value_; } T& - operator*() + operator*() & noexcept { return value_; } const T* - operator->() const + operator->() const noexcept { return &value_; } T* - operator->() + operator->() noexcept { return &value_; } optional& @@ -90,9 +90,27 @@ public: return *this; } - explicit operator bool() const { return has_value_; } + explicit operator bool() const noexcept { return has_value(); } }; +template +bool +operator==(const optional& lhs, const optional& rhs) +{ + if (!lhs.has_value() && !rhs.has_value()) + return true; + if (!lhs.has_value() || !rhs.has_value()) + return false; + return lhs.value() == rhs.value(); +} + +template +bool +operator!=(const optional& lhs, const optional& rhs) +{ + return !(lhs == rhs); +} + #endif } From patchwork Mon Mar 21 16:02:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 52181 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7951A3860004 for ; Mon, 21 Mar 2022 16:02:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7951A3860004 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1647878560; bh=16RhhnqlOfLbhr9pxJY0yR6ZCSO6VMMVVql4boifX1Q=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=dBzfTCSpkG7OT3+HaCy0nH/SdykNBz+IfFeX2nSPlsH7GEbejqD0aToiSoBwEmRXr sK5xqpJe51zkr1/8A4R/LaAsNRbP+X4QWEacNE5nqvK+kF8r+OK/cPEmgF4bnWPL16 o9wZ/0/oPyVmhpgL6vZzdGcIEoAsHIlz+9NURQ0w= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-ej1-x649.google.com (mail-ej1-x649.google.com [IPv6:2a00:1450:4864:20::649]) by sourceware.org (Postfix) with ESMTPS id 437FD385EC4E for ; Mon, 21 Mar 2022 16:02:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 437FD385EC4E Received: by mail-ej1-x649.google.com with SMTP id hg13-20020a1709072ccd00b006dfa484ec23so4845719ejc.8 for ; Mon, 21 Mar 2022 09:02:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=16RhhnqlOfLbhr9pxJY0yR6ZCSO6VMMVVql4boifX1Q=; b=sdY4LHXSw5Qqm/E3XX+ZJZqZh1QT32cell1TV8ti8gES1WxBSiZ6GJgUpiK7pTHDr9 7uzac4POk5Use1K7Nn4NtxnjurK3aIKEG+x4lR6rFX/kQ0xXKIgAoEljFgUKW2gMekan SfkYTUmHnvyERCNiNXiSFmpkSAWVCKKC9ZOfADXku6o84VQpsMGo+9OWZ9YIy686YGFg w4hxnBgl7xmDYrqtWGcXPhg7rJWFD6zJvxsD1IkH83zWj5lWwuBWychDZKJITr4iMiCC bs3NPgSO0oEnvcN0CHzxN9lDpRvElvwcbxSVpI4BNUTtFHAG4HQVuM1bqURlPQ3zIe9g pr+w== X-Gm-Message-State: AOAM5321Mq+QVbWDZY17V2rs3xva4/hu6rSZWZfm7kyxDb8oW57lv3JE dvY5jKN9unXdLw2Ki3tSHBQrVltVE0fvl+vwiIdN2kcWp2oC/oE+AQZkowrq5iPPGomZchbmFxS YthAITo4qj43t2Y4Xs6j315Wuc/iaWJY3f3gksIdGPXSM///6O0d7nnfIWqRyQ7Bynp/vBXs= X-Google-Smtp-Source: ABdhPJyl3JerHUgGWtIknLJAhY9r9ZZiBpfDVhcfcnyU5Gs7O5qc7/OYUEjV97Xxw112pO4yGqTYy9qo06fZCw== X-Received: from tef.lon.corp.google.com ([2a00:79e0:d:210:45b0:e68c:9bc8:4749]) (user=gprocida job=sendgmr) by 2002:aa7:de10:0:b0:419:430c:9b3d with SMTP id h16-20020aa7de10000000b00419430c9b3dmr4962070edv.366.1647878551951; Mon, 21 Mar 2022 09:02:31 -0700 (PDT) Date: Mon, 21 Mar 2022 16:02:20 +0000 In-Reply-To: <20220321160221.1372398-1-gprocida@google.com> Message-Id: <20220321160221.1372398-4-gprocida@google.com> Mime-Version: 1.0 References: <20220317163858.353762-1-gprocida@google.com> <20220321160221.1372398-1-gprocida@google.com> X-Mailer: git-send-email 2.35.1.894.gb6a874cedc-goog Subject: [PATCH v4 3/4] Linux symbol CRCs: support 0 and report presence changes To: libabigail@sourceware.org X-Spam-Status: No, score=-21.5 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" The CRC with value zero was used to mean "absent". This can be better modelled using optional. This commit makes this change and also tweaks reporting so that disappearing / appearing CRCs are noted. This should be essentially impossible unless CRCs are enabled / disabled altogether but would be very noteworthy otherwise. * include/abg-ir.h (elf_symbol::elf_symbol): Argument crc is now an optional defaulted to absent. (elf_symbol::create): Likewise. (elf_symbol::get_crc): Now returns an optional uint64_t. (elf_symbol::set_src): Now takes an optional uint64_t. * src/abg-comp-filter.cc (crc_changed): Simplify comparison. * src/abg-ir.cc (elf_symbol::priv): Member crc_ is now an optional uint64_t. (elf_symbol::priv::priv): Argument crc is now an optional uint64_t. (elf_symbol::elf_symbol): Likewise. (elf_symbol::create): Argument crc is now an optional uint64_t and defaults to absent. (textually_equals): Simplify comparison. (elf_symbol::get_crc): Now returns an optional uint64_t. (elf_symbol::set_crc): Now takes an optional uint64_t. * src/abg-reader.cc (build_elf_symbol): Treat CRC 0 the same as other CRC values. * src/abg-reporter-priv.cc (maybe_report_diff_for_symbol): Treat CRC 0 the same as other CRC values and also report changes to CRC presence. * src/abg-writer.cc (write_elf_symbol): Treat CRC 0 the same as other CRC values. * tests/data/Makefile: Remove test-abidiff/test-crc-report.txt and add test-abidiff/test-crc-report-{0-1,1-0,1-2}.txt. * tests/data/test-abidiff/test-crc-report-0-1.txt: Report showing additional of CRCs. * tests/data/test-abidiff/test-crc-report-1-0.txt: Report showing removal of CRCs. * tests/data/test-abidiff/test-crc-report-1-2.txt: Renamed from tests/data/test-abidiff/test-crc-report.txt. * tests/test-abidiff.cc: Update test cases that no longer generate empty reports. * tests/test-symtab.cc: Update KernelSymtabsWithCRC test. Reviewed-by: Matthias Maennich Signed-off-by: Giuliano Procida --- include/abg-ir.h | 9 +++++---- src/abg-comp-filter.cc | 4 +--- src/abg-ir.cc | 19 +++++++++--------- src/abg-reader.cc | 8 ++------ src/abg-reporter-priv.cc | 20 ++++++++++++++----- src/abg-writer.cc | 6 +++--- tests/data/Makefile.am | 4 +++- .../data/test-abidiff/test-crc-report-0-1.txt | 16 +++++++++++++++ .../data/test-abidiff/test-crc-report-1-0.txt | 16 +++++++++++++++ ...crc-report.txt => test-crc-report-1-2.txt} | 0 tests/test-abidiff.cc | 6 +++--- tests/test-symtab.cc | 8 ++++---- 12 files changed, 77 insertions(+), 39 deletions(-) create mode 100644 tests/data/test-abidiff/test-crc-report-0-1.txt create mode 100644 tests/data/test-abidiff/test-crc-report-1-0.txt rename tests/data/test-abidiff/{test-crc-report.txt => test-crc-report-1-2.txt} (100%) diff --git a/include/abg-ir.h b/include/abg-ir.h index a2f4e1a7..b05a8c6f 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -21,6 +21,7 @@ #include #include #include +#include "abg-cxx-compat.h" #include "abg-fwd.h" #include "abg-hash.h" #include "abg-traverse.h" @@ -920,7 +921,7 @@ private: const version& ve, visibility vi, bool is_in_ksymtab = false, - uint64_t crc = 0, + const abg_compat::optional& crc = {}, bool is_suppressed = false); elf_symbol(const elf_symbol&); @@ -945,7 +946,7 @@ public: const version& ve, visibility vi, bool is_in_ksymtab = false, - uint64_t crc = 0, + const abg_compat::optional& crc = {}, bool is_suppressed = false); const environment* @@ -1017,11 +1018,11 @@ public: void set_is_in_ksymtab(bool is_in_ksymtab); - uint64_t + const abg_compat::optional& get_crc() const; void - set_crc(uint64_t crc); + set_crc(const abg_compat::optional& crc); bool is_suppressed() const; diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index f90fdc78..31590284 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -234,9 +234,7 @@ crc_changed(const function_or_var_decl_sptr& f, const auto& symbol_s = s->get_symbol(); if (!symbol_f || !symbol_s) return false; - const auto crc_f = symbol_f->get_crc(); - const auto crc_s = symbol_s->get_crc(); - return crc_f != 0 && crc_s != 0 && crc_f != crc_s; + return symbol_f->get_crc() != symbol_s->get_crc(); } /// Test if the current diff tree node carries a CRC change in either a diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 0ef5e8b2..853b01ee 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -1727,7 +1727,7 @@ struct elf_symbol::priv // STT_COMMON definition of that name that has the largest size. bool is_common_; bool is_in_ksymtab_; - uint64_t crc_; + abg_compat::optional crc_; bool is_suppressed_; elf_symbol_wptr main_symbol_; elf_symbol_wptr next_alias_; @@ -1744,7 +1744,7 @@ struct elf_symbol::priv is_defined_(false), is_common_(false), is_in_ksymtab_(false), - crc_(0), + crc_(), is_suppressed_(false) {} @@ -1759,7 +1759,7 @@ struct elf_symbol::priv const elf_symbol::version& ve, elf_symbol::visibility vi, bool is_in_ksymtab, - uint64_t crc, + const abg_compat::optional& crc, bool is_suppressed) : env_(e), index_(i), @@ -1829,7 +1829,7 @@ elf_symbol::elf_symbol(const environment* e, const version& ve, visibility vi, bool is_in_ksymtab, - uint64_t crc, + const abg_compat::optional& crc, bool is_suppressed) : priv_(new priv(e, i, @@ -1900,7 +1900,7 @@ elf_symbol::create(const environment* e, const version& ve, visibility vi, bool is_in_ksymtab, - uint64_t crc, + const abg_compat::optional& crc, bool is_suppressed) { elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi, @@ -1926,8 +1926,7 @@ textually_equals(const elf_symbol&l, && l.is_defined() == r.is_defined() && l.is_common_symbol() == r.is_common_symbol() && l.get_version() == r.get_version() - && (l.get_crc() == 0 || r.get_crc() == 0 - || l.get_crc() == r.get_crc())); + && l.get_crc() == r.get_crc()); if (equals && l.is_variable()) // These are variable symbols. Let's compare their symbol size. @@ -2135,8 +2134,8 @@ 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 present) -uint64_t +/// @return the CRC (modversions) value for Linux Kernel symbols, if any +const abg_compat::optional& elf_symbol::get_crc() const {return priv_->crc_;} @@ -2144,7 +2143,7 @@ elf_symbol::get_crc() const /// /// @param crc the new CRC (modversions) value for Linux Kernel symbols void -elf_symbol::set_crc(uint64_t crc) +elf_symbol::set_crc(const abg_compat::optional& crc) {priv_->crc_ = crc;} /// Getter for the 'is-suppressed' property. diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 31885692..7a9ad1f9 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -3239,10 +3239,6 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node, is_default_version = true; } - uint64_t crc = 0; - if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "crc")) - crc = strtoull(CHAR_STR(s), NULL, 0); - elf_symbol::type type = elf_symbol::NOTYPE_TYPE; read_elf_symbol_type(node, type); @@ -3266,8 +3262,8 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node, e->set_is_suppressed(is_suppressed); - if (crc != 0) - e->set_crc(crc); + if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "crc")) + e->set_crc(strtoull(CHAR_STR(s), NULL, 0)); return e; } diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc index 7012f5dc..f9dd928b 100644 --- a/src/abg-reporter-priv.cc +++ b/src/abg-reporter-priv.cc @@ -1149,13 +1149,23 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1, << "\n"; } - if (symbol1->get_crc() != 0 && symbol2->get_crc() != 0 - && symbol1->get_crc() != 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)"; out << indent << "CRC (modversions) changed from " - << std::showbase << std::hex - << symbol1->get_crc() << " to " << symbol2->get_crc() - << std::noshowbase << std::dec + << std::showbase << std::hex; + if (crc1.has_value()) + out << crc1.value(); + else + out << none; + out << " to "; + if (crc2.has_value()) + out << crc2.value(); + else + out << none; + out << std::noshowbase << std::dec << "\n"; } } diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 7802128d..0a1e7b66 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -3131,10 +3131,10 @@ write_elf_symbol(const elf_symbol_sptr& sym, if (sym->is_common_symbol()) o << " is-common='yes'"; - if (sym->get_crc() != 0) + if (sym->get_crc().has_value()) o << " crc='" - << std::hex << std::showbase << sym->get_crc() << "'" - << std::dec << std::noshowbase; + << std::hex << std::showbase << sym->get_crc().value() + << std::dec << std::noshowbase << "'"; o << "/>\n"; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index a7eb7ff0..90bd1934 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -90,7 +90,9 @@ test-abidiff/test-empty-corpus-2.xml \ test-abidiff/test-crc-0.xml \ test-abidiff/test-crc-1.xml \ test-abidiff/test-crc-2.xml \ -test-abidiff/test-crc-report.txt \ +test-abidiff/test-crc-report-0-1.txt \ +test-abidiff/test-crc-report-1-0.txt \ +test-abidiff/test-crc-report-1-2.txt \ test-abidiff/test-PR27985-report.txt \ test-abidiff/test-PR27985-v0.c \ test-abidiff/test-PR27985-v0.o \ diff --git a/tests/data/test-abidiff/test-crc-report-0-1.txt b/tests/data/test-abidiff/test-crc-report-0-1.txt new file mode 100644 index 00000000..0db42f68 --- /dev/null +++ b/tests/data/test-abidiff/test-crc-report-0-1.txt @@ -0,0 +1,16 @@ +Functions changes summary: 0 Removed, 1 Changed, 0 Added function +Variables changes summary: 0 Removed, 2 Changed, 0 Added variables + +1 function with some indirect sub-type change: + + [C] 'function void exported_function()' has some indirect sub-type changes: + CRC (modversions) changed from (none) to 0xe52d5bcf + +2 Changed variables: + + [C] 'int exported_variable' was changed: + CRC (modversions) changed from (none) to 0xee94d699 + + [C] 'int exported_variable_gpl' was changed: + CRC (modversions) changed from (none) to 0x41336c46 + diff --git a/tests/data/test-abidiff/test-crc-report-1-0.txt b/tests/data/test-abidiff/test-crc-report-1-0.txt new file mode 100644 index 00000000..e11f29c1 --- /dev/null +++ b/tests/data/test-abidiff/test-crc-report-1-0.txt @@ -0,0 +1,16 @@ +Functions changes summary: 0 Removed, 1 Changed, 0 Added function +Variables changes summary: 0 Removed, 2 Changed, 0 Added variables + +1 function with some indirect sub-type change: + + [C] 'function void exported_function()' has some indirect sub-type changes: + CRC (modversions) changed from 0xe52d5bcf to (none) + +2 Changed variables: + + [C] 'int exported_variable' was changed: + CRC (modversions) changed from 0xee94d699 to (none) + + [C] 'int exported_variable_gpl' was changed: + CRC (modversions) changed from 0x41336c46 to (none) + diff --git a/tests/data/test-abidiff/test-crc-report.txt b/tests/data/test-abidiff/test-crc-report-1-2.txt similarity index 100% rename from tests/data/test-abidiff/test-crc-report.txt rename to tests/data/test-abidiff/test-crc-report-1-2.txt diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc index 32858ece..a02bbe3d 100644 --- a/tests/test-abidiff.cc +++ b/tests/test-abidiff.cc @@ -113,19 +113,19 @@ static InOutSpec specs[] = { "data/test-abidiff/test-crc-0.xml", "data/test-abidiff/test-crc-1.xml", - "data/test-abidiff/empty-report.txt", + "data/test-abidiff/test-crc-report-0-1.txt", "output/test-abidiff/test-crc-report-0-1.txt" }, { "data/test-abidiff/test-crc-1.xml", "data/test-abidiff/test-crc-0.xml", - "data/test-abidiff/empty-report.txt", + "data/test-abidiff/test-crc-report-1-0.txt", "output/test-abidiff/test-crc-report-1-0.txt" }, { "data/test-abidiff/test-crc-1.xml", "data/test-abidiff/test-crc-2.xml", - "data/test-abidiff/test-crc-report.txt", + "data/test-abidiff/test-crc-report-1-2.txt", "output/test-abidiff/test-crc-report-1-2.txt" }, { diff --git a/tests/test-symtab.cc b/tests/test-symtab.cc index 7d7a2df0..85303563 100644 --- a/tests/test-symtab.cc +++ b/tests/test-symtab.cc @@ -420,9 +420,9 @@ TEST_CASE("Symtab::KernelSymtabsWithCRC", "[symtab, crc, kernel, ksymtab]") { const std::string binary = base_path + "one_of_each.ko"; const corpus_sptr& corpus = assert_symbol_count(binary, 2, 2); - CHECK(corpus->lookup_function_symbol("exported_function")->get_crc() != 0); - CHECK(corpus->lookup_function_symbol("exported_function_gpl")->get_crc() != 0); - CHECK(corpus->lookup_variable_symbol("exported_variable")->get_crc() != 0); - CHECK(corpus->lookup_variable_symbol("exported_variable_gpl")->get_crc() != 0); + CHECK(corpus->lookup_function_symbol("exported_function")->get_crc()); + CHECK(corpus->lookup_function_symbol("exported_function_gpl")->get_crc()); + CHECK(corpus->lookup_variable_symbol("exported_variable")->get_crc()); + CHECK(corpus->lookup_variable_symbol("exported_variable_gpl")->get_crc()); } } From patchwork Mon Mar 21 16:02:21 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 52182 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8789B385ED4C for ; Mon, 21 Mar 2022 16:02:43 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8789B385ED4C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1647878563; bh=Me7Dsn65gAc8YGnhdDU+uJa+DYZ3/dTSqZMzdukQDI4=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=N7myKjgtCKZo2XubC9JLBl07iKpk3/bZ8+YF07mTkPCGvIKKzi4CsfYybYkXiOeO9 wXCMfwRNKwav1k3x3G13AGU7KclX6oxbZgjJ/tKJRFZUZsSFrItN0rGIVUD0gR9+bp vsICXqp5e40lcPTuKWLpfPc0RCznSxkSjsqoOghc= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-ej1-x64a.google.com (mail-ej1-x64a.google.com [IPv6:2a00:1450:4864:20::64a]) by sourceware.org (Postfix) with ESMTPS id D67C9385E004 for ; Mon, 21 Mar 2022 16:02:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D67C9385E004 Received: by mail-ej1-x64a.google.com with SMTP id m12-20020a1709062acc00b006cfc98179e2so7305633eje.6 for ; Mon, 21 Mar 2022 09:02:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=Me7Dsn65gAc8YGnhdDU+uJa+DYZ3/dTSqZMzdukQDI4=; b=BqGlCyekNlYwRYmdXIg9a4ZpS204LSlsruhot7xkk6pNNiIivGzqe9vUHPfpcj4Qxu k8YPmGkHC5TvNgdYH6ogzjA5pHkC1swxgNkLG3wM6+Q5cI2Ehc6QWoMm0sNIRhFTp2C4 pB07aYWQYs4iuawyEtDJRI/appHFTzc+nnGDDMs1xg2MPc3Vo66F9ph1EYEyeyWMDFkx P9Nj0BpPq9tXdIyCyWRgqlw0X8iSCMsvqkZEzkFjzD5LZdJjFO9eN5mmBgcsk/uqiYFJ ZEIrO+rj8aC0/IR7UOdOPvMpDDUXA0H8+rz2hM00RWa1Be/wjfaLXAhkokuiPiJY7+bw ujsg== X-Gm-Message-State: AOAM531Mdg/8NrXviSi0Y87csrL76uTY/jmwDtdqw+fG9+0JxtDmpIJj anlCmGLDltlHHlTUfRxD+MHT5RP4UOtJmNzzYslPLp7w9N5SP3hR2gwSAY59PUVhafSNcasn9fI MiYBOXiEw2GHkP4flm5FPJUITmn39Fix8AbUhz/PqO2u7L0WzcOCmE2PHcVM+xm7GoWROcwE= X-Google-Smtp-Source: ABdhPJzZcyh5r+Gv1lgCmA3u0FVHznFGmi3202ZB90APa/EVM8whSpQjUXQfpitFY5vBU69V5vcFrx/qObmt5g== X-Received: from tef.lon.corp.google.com ([2a00:79e0:d:210:45b0:e68c:9bc8:4749]) (user=gprocida job=sendgmr) by 2002:a50:9fad:0:b0:419:f22:46db with SMTP id c42-20020a509fad000000b004190f2246dbmr16566563edf.354.1647878554584; Mon, 21 Mar 2022 09:02:34 -0700 (PDT) Date: Mon, 21 Mar 2022 16:02:21 +0000 In-Reply-To: <20220321160221.1372398-1-gprocida@google.com> Message-Id: <20220321160221.1372398-5-gprocida@google.com> Mime-Version: 1.0 References: <20220317163858.353762-1-gprocida@google.com> <20220321160221.1372398-1-gprocida@google.com> X-Mailer: git-send-email 2.35.1.894.gb6a874cedc-goog Subject: [PATCH v4 4/4] add Linux kernel symbol namespace support To: libabigail@sourceware.org X-Spam-Status: No, score=-21.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-Patchwork-Original-From: Giuliano Procida via Libabigail From: Giuliano Procida Reply-To: Giuliano Procida Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Bug 28954 - add Linux Kernel symbol namespace support Each Linux kernel symbol can be exported to a specified named namespace or left in the global (nameless) namespace. One complexity is that the symbol values which identify a string in the __ksymtab_strings section must be interpretated differently for vmlinux and .ko loadable modules as the former has a fixed load address but the latter are relocatable. For vmlinux, the section base address needs to be subtracted to obtain a section-relative offset. The global namespace is explicitly represented as the empty string, at least when it comes to the value of __kstrtabns_FOO symbols, but the common interpretation is that such symbols lack an export namespace. I would rather not have to make use of "empty implies missing" in many places, so the code here represents namespace as optional and only the symtab reader cares about empty strings in __ksymtab_strings. * include/abg-ir.h (elf_symbol::elf_symbol): Add ns argument. (elf_symbol::create): Add ns argument. (elf_symbol::get_namespace): Declare new function. (elf_symbol::set_namespace): Declare new function. and set_namespace. * src/abg-comp-filter.cc (namespace_changed): Define new helper functions. (categorize_harmful_diff_node): Also call namespace_changed(). * src/abg-ir.cc (elf_symbol::priv): Add namespace_ member. (elf_symbol::priv::priv): Add namespace_ to initialisers. (elf_symbol::elf_symbol): Take new ns argument and pass it to priv constructor. (elf_symbol::create): Take new ns argument and pass it to elf_symbol constructor. (elf_symbol::get_namespace): Define new function. (elf_symbol::set_namespace): Define new function. * src/abg-reader.cc (build_elf_symbol): If namespace attribute is present, set symbol namespace. * src/abg-reporter-priv.cc (maybe_report_diff_for_symbol): If symbol namespaces differ, report this. * src/abg-symtab-reader.cc (symtab::load): Get ELF header to distinguish vmlinux from .ko. Try to get __ksymtab_strings metadata and data. Use these to look up __kstrtabns_FOO namespace entries. Set symbol namespace where found. * src/abg-writer.cc (write_elf_symbol): Emit namespace attribute, if symbol has a namespace. * tests/data/Makefile.am: Add new test files. * tests/data/test-abidiff/test-namespace-0.xml: New test file. * tests/data/test-abidiff/test-namespace-1.xml: Likewise * tests/data/test-abidiff/test-namespace-report.txt: Likewise. * tests/test-abidiff.cc: Add new test case. Reviewed-by: Matthias Maennich Signed-off-by: Giuliano Procida --- include/abg-ir.h | 8 +++ src/abg-comp-filter.cc | 40 +++++++++++- src/abg-ir.cc | 30 ++++++++- src/abg-reader.cc | 7 ++ src/abg-reporter-priv.cc | 18 ++++++ src/abg-symtab-reader.cc | 64 +++++++++++++++++++ src/abg-writer.cc | 3 + tests/data/Makefile.am | 3 + tests/data/test-abidiff/test-namespace-0.xml | 15 +++++ tests/data/test-abidiff/test-namespace-1.xml | 15 +++++ .../test-abidiff/test-namespace-report.txt | 17 +++++ tests/test-abidiff.cc | 6 ++ 12 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 tests/data/test-abidiff/test-namespace-0.xml create mode 100644 tests/data/test-abidiff/test-namespace-1.xml create mode 100644 tests/data/test-abidiff/test-namespace-report.txt diff --git a/include/abg-ir.h b/include/abg-ir.h index b05a8c6f..7c558828 100644 --- a/include/abg-ir.h +++ b/include/abg-ir.h @@ -922,6 +922,7 @@ private: visibility vi, bool is_in_ksymtab = false, const abg_compat::optional& crc = {}, + const abg_compat::optional& ns = {}, bool is_suppressed = false); elf_symbol(const elf_symbol&); @@ -947,6 +948,7 @@ public: visibility vi, bool is_in_ksymtab = false, const abg_compat::optional& crc = {}, + const abg_compat::optional& ns = {}, bool is_suppressed = false); const environment* @@ -1024,6 +1026,12 @@ public: void set_crc(const abg_compat::optional& crc); + const abg_compat::optional& + get_namespace() const; + + void + set_namespace(const abg_compat::optional& ns); + bool is_suppressed() const; diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index 31590284..22da5244 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -254,6 +254,43 @@ crc_changed(const diff* diff) return false; } +/// Test if there was a function or variable namespace change. +/// +/// @param f the first function or variable to consider. +/// +/// @param s the second function or variable to consider. +/// +/// @return true if the test is positive, false otherwise. +template +static bool +namespace_changed(const function_or_var_decl_sptr& f, + const function_or_var_decl_sptr& s) +{ + const auto& symbol_f = f->get_symbol(); + const auto& symbol_s = s->get_symbol(); + if (!symbol_f || !symbol_s) + return false; + return symbol_f->get_namespace() != symbol_s->get_namespace(); +} + +/// Test if the current diff tree node carries a namespace change in +/// either a function or a variable. +/// +/// @param diff the diff tree node to consider. +/// +/// @return true if the test is positive, false otherwise. +static bool +namespace_changed(const diff* diff) +{ + if (const function_decl_diff* d = + dynamic_cast(diff)) + return namespace_changed(d->first_function_decl(), + d->second_function_decl()); + if (const var_diff* d = dynamic_cast(diff)) + return namespace_changed(d->first_var(), d->second_var()); + return false; +} + /// Test if there was a function name change, but there there was no /// change in name of the underlying symbol. IOW, if the name of a /// function changed, but the symbol of the new function is equal to @@ -1781,7 +1818,8 @@ categorize_harmful_diff_node(diff *d, bool pre) || non_static_data_member_added_or_removed(d) || base_classes_added_or_removed(d) || has_harmful_enum_change(d) - || crc_changed(d))) + || crc_changed(d) + || namespace_changed(d))) category |= SIZE_OR_OFFSET_CHANGE_CATEGORY; if (has_virtual_mem_fn_change(d)) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 853b01ee..f90be2e4 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -1728,6 +1728,7 @@ struct elf_symbol::priv bool is_common_; bool is_in_ksymtab_; abg_compat::optional crc_; + abg_compat::optional namespace_; bool is_suppressed_; elf_symbol_wptr main_symbol_; elf_symbol_wptr next_alias_; @@ -1745,6 +1746,7 @@ struct elf_symbol::priv is_common_(false), is_in_ksymtab_(false), crc_(), + namespace_(), is_suppressed_(false) {} @@ -1760,6 +1762,7 @@ struct elf_symbol::priv elf_symbol::visibility vi, bool is_in_ksymtab, const abg_compat::optional& crc, + const abg_compat::optional& ns, bool is_suppressed) : env_(e), index_(i), @@ -1773,6 +1776,7 @@ struct elf_symbol::priv is_common_(c), is_in_ksymtab_(is_in_ksymtab), crc_(crc), + namespace_(ns), is_suppressed_(is_suppressed) { if (!is_common_) @@ -1818,6 +1822,8 @@ elf_symbol::elf_symbol() /// @param vi the visibility of the symbol. /// /// @param crc the CRC (modversions) value of Linux Kernel symbols +/// +/// @param ns the namespace of Linux Kernel symbols, if any elf_symbol::elf_symbol(const environment* e, size_t i, size_t s, @@ -1830,6 +1836,7 @@ elf_symbol::elf_symbol(const environment* e, visibility vi, bool is_in_ksymtab, const abg_compat::optional& crc, + const abg_compat::optional& ns, bool is_suppressed) : priv_(new priv(e, i, @@ -1843,6 +1850,7 @@ elf_symbol::elf_symbol(const environment* e, vi, is_in_ksymtab, crc, + ns, is_suppressed)) {} @@ -1886,6 +1894,8 @@ elf_symbol::create() /// /// @param crc the CRC (modversions) value of Linux Kernel symbols /// +/// @param ns the namespace of Linux Kernel symbols, if any +/// /// @return a (smart) pointer to a newly created instance of @ref /// elf_symbol. elf_symbol_sptr @@ -1901,10 +1911,11 @@ elf_symbol::create(const environment* e, visibility vi, bool is_in_ksymtab, const abg_compat::optional& crc, + const abg_compat::optional& ns, bool is_suppressed) { elf_symbol_sptr sym(new elf_symbol(e, i, s, n, t, b, d, c, ve, vi, - is_in_ksymtab, crc, is_suppressed)); + is_in_ksymtab, crc, ns, is_suppressed)); sym->priv_->main_symbol_ = sym; return sym; } @@ -1926,7 +1937,8 @@ textually_equals(const elf_symbol&l, && l.is_defined() == r.is_defined() && l.is_common_symbol() == r.is_common_symbol() && l.get_version() == r.get_version() - && l.get_crc() == r.get_crc()); + && l.get_crc() == r.get_crc() + && l.get_namespace() == r.get_namespace()); if (equals && l.is_variable()) // These are variable symbols. Let's compare their symbol size. @@ -2146,6 +2158,20 @@ void elf_symbol::set_crc(const abg_compat::optional& crc) {priv_->crc_ = crc;} +/// Getter of the 'namespace' property. +/// +/// @return the namespace for Linux Kernel symbols, if any +const abg_compat::optional& +elf_symbol::get_namespace() const +{return priv_->namespace_;} + +/// Setter of the 'namespace' property. +/// +/// @param ns the new namespace for Linux Kernel symbols, if any +void +elf_symbol::set_namespace(const abg_compat::optional& ns) +{priv_->namespace_ = ns;} + /// Getter for the 'is-suppressed' property. /// /// @return true iff the current symbol has been suppressed by a diff --git a/src/abg-reader.cc b/src/abg-reader.cc index 7a9ad1f9..f9e420f1 100644 --- a/src/abg-reader.cc +++ b/src/abg-reader.cc @@ -3265,6 +3265,13 @@ build_elf_symbol(read_context& ctxt, const xmlNodePtr node, if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "crc")) e->set_crc(strtoull(CHAR_STR(s), NULL, 0)); + if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "namespace")) + { + std::string ns; + xml::xml_char_sptr_to_string(s, ns); + e->set_namespace(ns); + } + return e; } diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc index f9dd928b..9ad733f4 100644 --- a/src/abg-reporter-priv.cc +++ b/src/abg-reporter-priv.cc @@ -1168,6 +1168,24 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1, out << std::noshowbase << std::dec << "\n"; } + + const abg_compat::optional& ns1 = symbol1->get_namespace(); + const abg_compat::optional& ns2 = symbol2->get_namespace(); + if (ns1 != ns2) + { + const std::string none = "(none)"; + out << indent << "namespace changed from "; + if (ns1.has_value()) + out << "'" << ns1.value() << "'"; + else + out << none; + out << " to "; + if (ns2.has_value()) + out << "'" << ns2.value() << "'"; + else + out << none; + out << "\n"; + } } /// For a given symbol, emit a string made of its name and version. diff --git a/src/abg-symtab-reader.cc b/src/abg-symtab-reader.cc index b42ce87d..92509054 100644 --- a/src/abg-symtab-reader.cc +++ b/src/abg-symtab-reader.cc @@ -204,6 +204,13 @@ symtab::load_(Elf* elf_handle, ir::environment* env, symbol_predicate is_suppressed) { + GElf_Ehdr ehdr_mem; + GElf_Ehdr* header = gelf_getehdr(elf_handle, &ehdr_mem); + if (!header) + { + std::cerr << "Could not load get ELF header: Skipping symtab load.\n"; + return false; + } Elf_Scn* symtab_section = elf_helpers::find_symbol_table_section(elf_handle); if (!symtab_section) @@ -232,9 +239,34 @@ symtab::load_(Elf* elf_handle, return false; } + // The __kstrtab_strings sections is basically an ELF strtab but does not + // support elf_strptr lookups. A single call to elf_getdata gives a handle to + // washed section data. + // + // The value of a __kstrtabns_FOO (or other similar) symbol is an address + // within the __kstrtab_strings section. To look up the string value, we need + // to translate from vmlinux load address to section offset by subtracting the + // base address of the section. This adjustment is not needed for loadable + // modules which are relocatable and so identifiable by ELF type ET_REL. + Elf_Scn* strings_section = elf_helpers::find_ksymtab_strings_section(elf_handle); + size_t strings_offset = 0; + const char* strings_data = nullptr; + size_t strings_size = 0; + if (strings_section) + { + GElf_Shdr strings_sheader; + gelf_getshdr(strings_section, &strings_sheader); + strings_offset = header->e_type == ET_REL ? 0 : strings_sheader.sh_addr; + Elf_Data* data = elf_getdata(strings_section, nullptr); + ABG_ASSERT(data->d_off == 0); + strings_data = reinterpret_cast(data->d_buf); + strings_size = data->d_size; + } + 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 namespaces; const bool is_arm32 = elf_helpers::architecture_is_arm32(elf_handle); const bool is_ppc64 = elf_helpers::architecture_is_ppc64(elf_handle); @@ -288,6 +320,27 @@ symtab::load_(Elf* elf_handle, ABG_ASSERT(crc_values.emplace(name.substr(6), sym->st_value).second); continue; } + if (strings_section && is_kernel && name.rfind("__kstrtabns_", 0) == 0) + { + // This symbol lives in the __ksymtab_strings section but st_value may + // be a vmlinux load address so we need to subtract the offset before + // looking it up in that section. + const size_t value = sym->st_value; + const size_t offset = value - strings_offset; + // check offset + ABG_ASSERT(offset < strings_size); + // find the terminating NULL + const char* first = strings_data + offset; + const char* last = strings_data + strings_size; + const char* limit = std::find(first, last, 0); + // check NULL found + ABG_ASSERT(limit < last); + // interpret the empty namespace name as no namespace name + if (first < limit) + ABG_ASSERT(namespaces.emplace( + name.substr(12), std::string(first, limit - first)).second); + continue; + } // filter out uninteresting entries and only keep functions/variables for // now. The rest might be interesting in the future though. @@ -402,6 +455,17 @@ symtab::load_(Elf* elf_handle, symbol->set_crc(crc_entry.second); } + // Now add the namespaces + for (const auto& namespace_entry : namespaces) + { + const auto r = name_symbol_map_.find(namespace_entry.first); + if (r == name_symbol_map_.end()) + continue; + + for (const auto& symbol : r->second) + symbol->set_namespace(namespace_entry.second); + } + // sort the symbols for deterministic output std::sort(symbols_.begin(), symbols_.end(), symbol_sort); diff --git a/src/abg-writer.cc b/src/abg-writer.cc index 0a1e7b66..03fb658b 100644 --- a/src/abg-writer.cc +++ b/src/abg-writer.cc @@ -3136,6 +3136,9 @@ write_elf_symbol(const elf_symbol_sptr& sym, << std::hex << std::showbase << sym->get_crc().value() << std::dec << std::noshowbase << "'"; + if (sym->get_namespace().has_value()) + o << " namespace='" << sym->get_namespace().value() << "'"; + o << "/>\n"; return true; diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 90bd1934..7388697b 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -93,6 +93,9 @@ test-abidiff/test-crc-2.xml \ test-abidiff/test-crc-report-0-1.txt \ test-abidiff/test-crc-report-1-0.txt \ test-abidiff/test-crc-report-1-2.txt \ +test-abidiff/test-namespace-0.xml \ +test-abidiff/test-namespace-1.xml \ +test-abidiff/test-namespace-report.txt \ test-abidiff/test-PR27985-report.txt \ test-abidiff/test-PR27985-v0.c \ test-abidiff/test-PR27985-v0.o \ diff --git a/tests/data/test-abidiff/test-namespace-0.xml b/tests/data/test-abidiff/test-namespace-0.xml new file mode 100644 index 00000000..5a9f5cd2 --- /dev/null +++ b/tests/data/test-abidiff/test-namespace-0.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/tests/data/test-abidiff/test-namespace-1.xml b/tests/data/test-abidiff/test-namespace-1.xml new file mode 100644 index 00000000..9814844b --- /dev/null +++ b/tests/data/test-abidiff/test-namespace-1.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + + diff --git a/tests/data/test-abidiff/test-namespace-report.txt b/tests/data/test-abidiff/test-namespace-report.txt new file mode 100644 index 00000000..d2c421ed --- /dev/null +++ b/tests/data/test-abidiff/test-namespace-report.txt @@ -0,0 +1,17 @@ +Functions changes summary: 0 Removed, 0 Changed, 0 Added function +Variables changes summary: 0 Removed, 4 Changed, 0 Added variables + +4 Changed variables: + + [C] 'int v1' was changed: + namespace changed from (none) to 'this' + + [C] 'int v2' was changed: + namespace changed from 'this' to 'that' + + [C] 'int v3' was changed: + namespace changed from 'that' to '' + + [C] 'int v4' was changed: + namespace changed from '' to (none) + diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc index a02bbe3d..93e44d6a 100644 --- a/tests/test-abidiff.cc +++ b/tests/test-abidiff.cc @@ -128,6 +128,12 @@ static InOutSpec specs[] = "data/test-abidiff/test-crc-report-1-2.txt", "output/test-abidiff/test-crc-report-1-2.txt" }, + { + "data/test-abidiff/test-namespace-0.xml", + "data/test-abidiff/test-namespace-1.xml", + "data/test-abidiff/test-namespace-report.txt", + "output/test-abidiff/test-namespace-report-0-1.txt" + }, { "data/test-abidiff/test-PR27616-v0.xml", "data/test-abidiff/test-PR27616-v1.xml",