From patchwork Thu Oct 29 12:20:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Matthias_M=C3=A4nnich?= X-Patchwork-Id: 40917 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 056653985423; Thu, 29 Oct 2020 12:21:16 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 056653985423 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1603974076; bh=+0tNvtPWNyMi7Ro7/K3gc9UisW1/GL9VuBqkTKwTPMc=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:Cc:From; b=vzTlZjG2xLtZ4DVVKdcdCpsxdBkWU1RbSBvuC6DzZiWmOR0QyoujgKs8BUIfqtDOo 5Qcm8c3Wh2+CPvQAv8L16lzhLYTz5ZoglpOR3DLradqQsgzBNPwVZ0xd8hwigs9QPz oOdswB1AvZi4ZbpDROm5QUcc0YvmycXsZJ6nTjuQ= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-wr1-x44a.google.com (mail-wr1-x44a.google.com [IPv6:2a00:1450:4864:20::44a]) by sourceware.org (Postfix) with ESMTPS id 1AA383985423 for ; Thu, 29 Oct 2020 12:21:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 1AA383985423 Received: by mail-wr1-x44a.google.com with SMTP id t17so1210012wrm.13 for ; Thu, 29 Oct 2020 05:21:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:message-id:mime-version:subject:from :to:cc:content-transfer-encoding; bh=+0tNvtPWNyMi7Ro7/K3gc9UisW1/GL9VuBqkTKwTPMc=; b=V6+pUQEYWvnLUBDYOfU76fM7izrb+QPfWXMw0MTilumyaO2XXJp2yxN3vHS0ZUOAxr Fj5XwMtlrgjxjIVm15msVkfZjdZjXYw8RIQ9mFM661jtbZBgYM4WkDELSTTPAjA4crPM G8DyNnEOMXiuucB7DwCrczu2XRdBqzbcgnaLTkA7Sf/TOosPJtVgfEq3xkeEIwDRyvzz fc1UkTAGesWJ6i6WE6QorbnJtuADbUpyZlDKD2rXfay/HkwIcBwsfWWqZE16m3OJLmtk hdXnhciA2bk2WxVTbkKbPwX0SuCn3OlbwGmVeDacJq0aBKuNjTT3Ippe9xmYZ8kWVtLq Xuew== X-Gm-Message-State: AOAM5311iQAsYQU9g2V/R0wjoP1kgkJqupjSj9ihWDonFL/NNn7fbb/1 8hfUoXwbq/mG7wo5yC5XmNu507VCt1rawsHmTXt5zMSDIwYh7ftUZqcy7atWV5Q0GMe5lnD3e/m 18qQ1GAofZpYemBJ/S74bk6iJQ6zHQgq2V8bZhx8uExihDHu4ZUxkBpq2qgpAHCvutLgD2n0= X-Google-Smtp-Source: ABdhPJy1QO7m5HhMQVo8K7tWZ63swX3eWfSNG2FaPicHeNfvkwVdK9Sc8YaR3qWCTXoihuUwlG5+mn32f35+lw== X-Received: from lux.lon.corp.google.com ([2a00:79e0:d:210:7220:84ff:fe09:a3aa]) (user=maennich job=sendgmr) by 2002:a7b:c345:: with SMTP id l5mr4071732wmj.123.1603974067732; Thu, 29 Oct 2020 05:21:07 -0700 (PDT) Date: Thu, 29 Oct 2020 12:20:58 +0000 Message-Id: <20201029122100.765143-1-maennich@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.29.1.341.ge80a0c044ae-goog Subject: [PATCH 1/3] Improve and stabilise sort of member functions To: libabigail@sourceware.org X-Spam-Status: No, score=-22.9 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, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Matthias Maennich via Libabigail From: =?utf-8?q?Matthias_M=C3=A4nnich?= Reply-To: Matthias Maennich Cc: maennich@google.com, kernel-team@android.com Errors-To: libabigail-bounces@sourceware.org Sender: "Libabigail" From: Giuliano Procida The functor virtual_member_function_less_than did not take into account linkage name which can be the only difference when multiple destructors with differing mangled names are present. This change adds a check for linkage names and also flattens the control flow in the comparison method to make the logic clearer. Lastly, this change also uses std::stable_sort, in case all that remains is insertion order. * src/abg-ir.cc (virtual_member_function_less_than::operator()): Name temporaries like offsets and symbols to reduce repetition; test each pair of elements (including symbol presence) and return immediately if there's a difference; add a comparison of linkage name just after comparing symbol names. (sort_virtual_member_functions): Use stable_sort instead of sort. * tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi: Update with new ordering of member functions. * tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi: Likewise. Signed-off-by: Giuliano Procida Signed-off-by: Matthias Maennich --- src/abg-ir.cc | 82 +++++++++---------- .../PR22015-libboost_iostreams.so.abi | 8 +- .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 78 +++++++++--------- 3 files changed, 80 insertions(+), 88 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index e491528922d3..29711db6057c 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -20487,53 +20487,45 @@ struct virtual_member_function_less_than ABG_ASSERT(get_member_function_is_virtual(f)); ABG_ASSERT(get_member_function_is_virtual(s)); - if (get_member_function_vtable_offset(f) - == get_member_function_vtable_offset(s)) + ssize_t f_offset = get_member_function_vtable_offset(f); + ssize_t s_offset = get_member_function_vtable_offset(s); + if (f_offset != s_offset) return f_offset < s_offset; + + string fn, sn; + + // If the functions have symbols, then compare their symbol-id + // string. + elf_symbol_sptr f_sym = f.get_symbol(); + elf_symbol_sptr s_sym = s.get_symbol(); + if ((!f_sym) != (!s_sym)) return !f_sym; + if (f_sym && s_sym) { - string fn, sn; - - // If the functions have symbols, then compare their symbol-id - // string. - if (f.get_symbol() && s.get_symbol()) - { - fn = f.get_symbol()->get_id_string(); - sn = s.get_symbol()->get_id_string(); - } - else if (f.get_symbol()) - return false; - else if (s.get_symbol()) - return true; - else - { - // None of the functions have symbols, so compare their - // pretty representation. - if (fn.empty()) - { - fn = f.get_pretty_representation(); - sn = s.get_pretty_representation(); - } - } - - /// If it's just the file paths that are different then sort - /// them too. - if (fn == sn) - { - string fn_filepath, sn_filepath; - unsigned line = 0, column = 0; - location fn_loc = f.get_location(), sn_loc = s.get_location(); - if (fn_loc) - fn_loc.expand(fn_filepath, line, column); - if (sn_loc) - sn_loc.expand(sn_filepath, line, column); - - if (!fn_filepath.empty() && !sn_filepath.empty()) - return fn_filepath < sn_filepath; - } - return fn < sn; + fn = f_sym->get_id_string(); + sn = s_sym->get_id_string(); + if (fn != sn) return fn < sn; } - return (get_member_function_vtable_offset(f) - < get_member_function_vtable_offset(s)); + // Try the linkage names (important for destructors). + fn = f.get_linkage_name(); + sn = s.get_linkage_name(); + if (fn != sn) return fn < sn; + + // None of the functions have symbols or linkage names that + // distinguish them, so compare their pretty representation. + fn = f.get_pretty_representation(); + sn = s.get_pretty_representation(); + if (fn != sn) return fn < sn; + + /// If it's just the file paths that are different then sort them + /// too. + string fn_filepath, sn_filepath; + unsigned line = 0, column = 0; + location fn_loc = f.get_location(), sn_loc = s.get_location(); + if (fn_loc) + fn_loc.expand(fn_filepath, line, column); + if (sn_loc) + sn_loc.expand(sn_filepath, line, column); + return fn_filepath < sn_filepath; } /// The less than operator. First, it sorts the methods by their @@ -20560,7 +20552,7 @@ static void sort_virtual_member_functions(class_decl::member_functions& mem_fns) { virtual_member_function_less_than lt; - std::sort(mem_fns.begin(), mem_fns.end(), lt); + std::stable_sort(mem_fns.begin(), mem_fns.end(), lt); } /// Add a member function to the current instance of @ref class_or_union. diff --git a/tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi b/tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi index c4b1e75499f2..00921cd53016 100644 --- a/tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi +++ b/tests/data/test-read-dwarf/PR22015-libboost_iostreams.so.abi @@ -1031,13 +1031,13 @@ - + - + @@ -1072,13 +1072,13 @@ - + - + diff --git a/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi b/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi index b6faf148d63e..0c3611ebbf7b 100644 --- a/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi +++ b/tests/data/test-read-dwarf/test22-pr19097-libstdc++.so.6.0.17.so.abi @@ -4518,14 +4518,14 @@ - + - + @@ -4547,14 +4547,14 @@ - + - + @@ -5081,14 +5081,14 @@ - + - + @@ -28550,14 +28550,14 @@ - + - + @@ -28722,14 +28722,14 @@ - + - + @@ -30889,14 +30889,14 @@ - + - + @@ -32608,13 +32608,6 @@ - - - - - - - @@ -32622,12 +32615,11 @@ - - + + - - - + + @@ -32638,11 +32630,12 @@ - - + + - - + + + @@ -32652,12 +32645,11 @@ - - + + - - - + + @@ -32668,6 +32660,14 @@ + + + + + + + + @@ -32955,14 +32955,14 @@ - + - + @@ -33042,14 +33042,14 @@ - + - + @@ -35377,14 +35377,14 @@ - + - +