From patchwork Wed Jan 27 12:58:33 2021 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: 41834 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 80562397180F; Wed, 27 Jan 2021 12:59:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 80562397180F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1611752350; bh=ouGA+svTxtCqv16xg+O/JtB7qHzY9TAcj05WgEGae2E=; h=Date:In-Reply-To:References:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Help:List-Subscribe:From:Reply-To:Cc:From; b=muRaUxJmc8xJveEEGJO/VDLpf0kjG82cXJ/LbNPsVvHCZpQBI77FkcCdZO0LPRhGS Hh8oPRT9UKMKCwCU9B9BgmzfXNRVlN7we+JjFoDbXFnCvn0wI6CKTaKTs/40pg6IzY svZtsdL/F6zlCgoLx0XWAVu4BCJpRAe8yfFaVEAw= X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org Received: from mail-qk1-x74a.google.com (mail-qk1-x74a.google.com [IPv6:2607:f8b0:4864:20::74a]) by sourceware.org (Postfix) with ESMTPS id 95801382D830 for ; Wed, 27 Jan 2021 12:59:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 95801382D830 Received: by mail-qk1-x74a.google.com with SMTP id u66so1372287qkd.13 for ; Wed, 27 Jan 2021 04:59:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=ouGA+svTxtCqv16xg+O/JtB7qHzY9TAcj05WgEGae2E=; b=LnMLQiVZdzZmz2tIb7JW2q6RJLXlq4s+mQ+KZemxKNRRONZewynXF0ueM+trS3G/vV LK9xB4V7dUnTgm1/Cdjoh/8AY7N4uunMBO07lvflogTaUQXGwxzJHQnh4beOTzM7okXP N1Yy7Eq86d0mqhpTIos93XmVWpd/rZ0laICwVZrM33YcWbLSJKDcqjkAEZgJ9srFow58 ZqWZ1szatWtnqvHClryfxtIIa7RDfcPtRbLQM0JkGe3Pa+xx5kYZPUKebxexLaJq4JEO dnkKGZG2n+sVKIqhne4mRtz/jS6KyXsQMj9kVM9vjO7q0VLdoapR6HEJKhVYBOZ3vtax HsHg== X-Gm-Message-State: AOAM530tFhJMBzcUY4QQgQ0HB6CDKkkOhnrXxSWHx/kiew/hg9g0lylq fMZGXTuryWsz7UNKm9V44LJtOVV7fy5j6mc6N5zeCx6RuIRmk6DQdMyET3cS7m2HORKdDhDgEuK XI/oVhKh/h/32xLnWyF5yTZ8fcObBYRDavA6zIqqM4eKeRxJeU/MC0/i24CbbbdN1UsHJw/M= X-Google-Smtp-Source: ABdhPJwrOPelyom7oa6znio05evWkVFljlKSl3FolJrUfAN8hbl4EoRkVwe0IFTfuIjv5bPDNuTKSQGOYK9G6g== X-Received: from lux.lon.corp.google.com ([2a00:79e0:d:210:7220:84ff:fe09:a3aa]) (user=maennich job=sendgmr) by 2002:a0c:a819:: with SMTP id w25mr10167162qva.6.1611752346072; Wed, 27 Jan 2021 04:59:06 -0800 (PST) Date: Wed, 27 Jan 2021 12:58:33 +0000 In-Reply-To: <20200619214305.562-1-maennich@google.com> Message-Id: <20210127125853.886677-1-maennich@google.com> Mime-Version: 1.0 References: <20200619214305.562-1-maennich@google.com> X-Mailer: git-send-email 2.30.0.280.ga3ce27912f-goog Subject: [PATCH 00/20] Refactor (k)symtab reader To: libabigail@sourceware.org X-Spam-Status: No, score=-16.9 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, 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" The current implementation that reads the symtab and the ksymtab has grown over time from simple symtab reading to way more complex ksymtab reading including taking care of little details like position relative relocations, symbol namespaces, etc. Yet, more features are coming to the Linux kernels that make this parsing even more tricky: Further changes to the ksymtab layout and different needs to lookup symbols caused by features like LTO (causing RELA relocations in the ksymtab entries) and CFI (causing additional jump table symbols) that are highly confusing the meaning of ksymtab entries and make it increasingly challenging for a static analysis tool like libabigail to properly process the ksymtab values. This added complexity also adds more and more responsibilities to the read_context that already has a lot of different tasks to juggle. It gets increasingly difficult to ensure, further development in the dwarf reader can be done without subtly regressing existing functionality. Hence, attempt a refactoring (one could argue: rewrite, but a lot of functionality is just migrated out) of the symtab reading code. The first commit sets up some prerequisites, a partial backport of std::optional. Commit 2 and 3 modify abg-ir's elf_symbol to be able to carry 'is_suppressed' and 'is_in_ksymtab'. Commit 4 and 5 implement the new symtab reader. The abg-symtab-reader has been introduced as an instance decoupled from dwarf readers' read_context. This reduces the responsibilities of the dwarf reader and separates the functionality into a new compilation unit. It contains several components to make the main component 'symtab' easy to access and to query. Refer to the extensive commit message there for details. The actual core of the symtab reading has been taken as a base, but refactored where useful. The ksymtab reading could be simplified by processing the corresponding __ksymtab_* entries directly from symtab without the need to interpret the binary ksymtab sections. That also resolves issues with wrong ksymtab reading: Mapping from the ksymtab symbol address to the symtab entry might leave us with a non-main symbol and hence leads to incorrect results. E.g. symbols like strlen are implemented as __pi_strlen and are aliases to strlen in the kernel. Only by reading the ksymtab entries we can decide which symbol to keep. Otherwise we get indeterministic results. Furthermore, symbol whitelists might list one or the other leading to issues of suppressed symbols for which we might just see the wrong symbol and therefore suppress both from analysis. In addition, detecting the format of the ksymtab, requires the first entry to be a valid elf_symbol, which is not the case if filtered out via whitelist or suppression. Finally, features like CFI require name based lookup into the ksymtab and LTO with clang on aarch64 might make the ksymtab contain relocatable entries. This is additional complexity hitting the dwarf reader. Those are subtle issues that motivated this series. Conceptionally, the new reader works quite similar. Except for the way suppressions are applied: Instead of discarding symbols while reading, we flag symbols as suppressed and keep them around for lookup purposes. That resolves issues when dealing with symbol aliases. Commit 6 integrates the new symtab reader into the existing code - side by side with the current implementation. Commits 7 - 11 migrate more and more symtab users over to the new symtab reader, including the ksymtab functionality in commit 12 where the old implementation could be obsoleted. Commits 12 and 13 re-add the ppc64 support for ELFv1 binaries. Commits 14 and 15 remove now obsolete functionality and remove the now old implementation. Commit 16 adds additional tests for whitelisted symbols. Commits 17 - 19 address some flaws with aliasing in combination with suppressions (e.g. triggered from whitelists). Those are issues I was seeing in the current implementation as well, but could solve a bit easier with the new symtab reader. Commit 20 adds support for MODVERSIONS in the Linux kernel (CRC values). Performance testing has been done with an 'allmodconfig' kernel config. That is the worst case for kernels and representing the 'distribution kernel' use case. During those tests, no significant performance impact could be measured. In addition, various Android Kernels in various configurations have been tested with this. In fact, Android Kernels use this patch series since last July for all their libabigail based analysis. The earlier added tests for reading symtab and ksymtab oviously pass. v2 -> v3: - removed symtab_reader::symtab_filter_builder - moved around the documentation accordingly - eliminated sptr use in interfaces where possible and appropriate - Some simplifications due to C++11 use (range for, etc). - minor fixes and cleanups were absorbed - some patches have been reorganized v1 -> v2: - Commit 16 now also drops the obsolete ksymtab enums. - Commit 17+ are new (alias improvements, modversions) - rebased on top of current master Cheers, Matthias Giuliano Procida (1): abg-writer.cc: fix write_elf_symbol_reference loop Matthias Maennich (19): abg-cxx-compat: add simplified version of std::optional abg-ir: elf_symbol: add is_in_ksymtab field abg-ir: elf_symbol: add is_suppressed field dwarf-reader split: create abg-symtab-reader.{h,cc} and test case Refactor ELF symbol table reading by adding a new symtab reader Integrate new symtab reader into corpus and read_context corpus: make get_(undefined_)?_(var|fun)_symbols use the new symtab corpus: make get_unreferenced_(function|variable)_symbols use the new symtab abg-reader: avoid using the (var|function)_symbol_map dwarf-reader: read_context: use new symtab in *_symbols_is_exported Switch kernel stuff over to new symtab and drop unused code abg-elf-helpers: migrate ppc64 specific helpers symtab_reader: add support for ppc64 ELFv1 binaries abg-corpus: remove symbol maps and their setters dwarf reader: drop (now) unused code related to symbol table reading test-symtab: add tests for whitelisted functions symtab/dwarf-reader: allow hinting of main symbols for aliases dwarf-reader/writer: consider aliases when dealing with suppressions symtab: Add support for MODVERSIONS (CRC checksums) include/abg-corpus.h | 24 +- include/abg-cxx-compat.h | 84 + include/abg-dwarf-reader.h | 6 - include/abg-fwd.h | 8 + include/abg-ir.h | 53 +- src/Makefile.am | 2 + src/abg-comp-filter.cc | 39 +- src/abg-corpus-priv.h | 57 +- src/abg-corpus.cc | 609 ++-- src/abg-dwarf-reader.cc | 2443 ++--------------- src/abg-elf-helpers.cc | 186 ++ src/abg-elf-helpers.h | 8 + src/abg-ir.cc | 187 +- src/abg-reader.cc | 57 +- src/abg-reporter-priv.cc | 18 +- src/abg-symtab-reader.cc | 511 ++++ src/abg-symtab-reader.h | 310 +++ src/abg-tools-utils.cc | 13 - src/abg-writer.cc | 68 +- tests/Makefile.am | 4 + tests/data/Makefile.am | 38 +- .../test5-fn-changed-report-0.txt | 4 - .../test5-fn-changed-report-1.txt | 4 - .../test-abidiff-exit/test-crc-report.txt | 15 + tests/data/test-abidiff-exit/test-crc-v0.abi | 29 + tests/data/test-abidiff-exit/test-crc-v1.abi | 30 + .../test-missing-alias-report.txt | 0 .../test-abidiff-exit/test-missing-alias.abi | 12 + .../test-missing-alias.suppr | 4 + ...ibtirpc.so.report.txt => empty-report.txt} | 0 .../test-abidiff/test-PR24552-report0.txt | 3 - tests/data/test-abidiff/test-crc-0.xml | 1601 +++++++++++ tests/data/test-abidiff/test-crc-1.xml | 1601 +++++++++++ tests/data/test-abidiff/test-crc-2.xml | 1601 +++++++++++ tests/data/test-abidiff/test-crc-report.txt | 9 + .../test-abidiff/test-empty-corpus-report.txt | 3 - .../data/test-annotate/test15-pr18892.so.abi | 918 +++---- ...19-pr19023-libtcmalloc_and_profiler.so.abi | 60 +- tests/data/test-annotate/test2.so.abi | 12 +- tests/data/test-annotate/test3.so.abi | 6 +- tests/data/test-diff-dwarf/test12-report.txt | 7 + .../test36-ppc64-aliases-report-0.txt | 4 +- .../test42-PR21296-clanggcc-report0.txt | 6 +- .../test31-pr18535-libstdc++-report-0.txt | 6 +- .../test31-pr18535-libstdc++-report-1.txt | 6 +- ...bb-4.3-3.20141204.fc23.x86_64-report-0.txt | 6 +- ...bb-4.3-3.20141204.fc23.x86_64-report-1.txt | 6 +- .../test23-alias-filter-4.suppr | 2 +- .../test23-alias-filter-report-0.txt | 4 +- .../test23-alias-filter-report-2.txt | 4 +- .../data/test-diff-suppr/test31-report-0.txt | 3 + .../data/test-diff-suppr/test32-report-1.txt | 2 +- .../PR22015-libboost_iostreams.so.abi | 48 +- .../test-read-dwarf/PR22122-libftdc.so.abi | 6 +- .../data/test-read-dwarf/PR25007-sdhci.ko.abi | 77 +- .../test-read-dwarf/libtest24-drop-fns.so.abi | 314 +-- .../test-read-dwarf/test-suppressed-alias.c | 16 + .../test-read-dwarf/test-suppressed-alias.o | Bin 0 -> 2848 bytes .../test-suppressed-alias.o.abi | 16 + .../test-suppressed-alias.suppr | 7 + .../test-read-dwarf/test10-pr18818-gcc.so.abi | 192 +- .../test-read-dwarf/test11-pr18828.so.abi | 516 ++-- .../test-read-dwarf/test12-pr18844.so.abi | 66 +- .../test-read-dwarf/test15-pr18892.so.abi | 918 +++---- .../test-read-dwarf/test16-pr18904.so.abi | 990 +++---- ...19-pr19023-libtcmalloc_and_profiler.so.abi | 60 +- tests/data/test-read-dwarf/test2.so.abi | 12 +- tests/data/test-read-dwarf/test2.so.hash.abi | 12 +- .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 1042 +++---- .../test-read-dwarf/test3-alias-1.so.hash.abi | 14 + .../data/test-read-dwarf/test3-alias-1.suppr | 3 + .../test-read-dwarf/test3-alias-2.so.hash.abi | 18 + .../data/test-read-dwarf/test3-alias-2.suppr | 3 + .../test-read-dwarf/test3-alias-3.so.hash.abi | 14 + .../data/test-read-dwarf/test3-alias-3.suppr | 3 + .../test-read-dwarf/test3-alias-4.so.hash.abi | 8 + .../data/test-read-dwarf/test3-alias-4.suppr | 3 + tests/data/test-read-dwarf/test3.so.abi | 6 +- tests/data/test-read-dwarf/test3.so.hash.abi | 6 +- tests/data/test-read-write/test-crc.xml | 10 + tests/data/test-symtab/basic/aliases.c | 13 + tests/data/test-symtab/basic/aliases.so | Bin 0 -> 17176 bytes tests/data/test-symtab/basic/no_debug_info.c | 2 +- tests/data/test-symtab/basic/no_debug_info.so | Bin 15360 -> 15544 bytes .../one_function_one_variable_all.whitelist | 3 + ...e_function_one_variable_function.whitelist | 2 + ...function_one_variable_irrelevant.whitelist | 2 + ...e_function_one_variable_variable.whitelist | 2 + .../test-symtab/kernel-modversions/Makefile | 19 + .../kernel-modversions/one_of_each.c | 1 + .../kernel-modversions/one_of_each.ko | Bin 0 -> 131760 bytes tests/test-abidiff-exit.cc | 22 + tests/test-abidiff.cc | 34 +- tests/test-cxx-compat.cc | 52 + tests/test-read-dwarf.cc | 40 + tests/test-read-write.cc | 6 + tests/test-symtab-reader.cc | 15 + tests/test-symtab.cc | 195 +- tools/abidw.cc | 2 - 99 files changed, 10001 insertions(+), 5482 deletions(-) create mode 100644 src/abg-symtab-reader.cc create mode 100644 src/abg-symtab-reader.h create mode 100644 tests/data/test-abidiff-exit/test-crc-report.txt create mode 100644 tests/data/test-abidiff-exit/test-crc-v0.abi create mode 100644 tests/data/test-abidiff-exit/test-crc-v1.abi create mode 100644 tests/data/test-abidiff-exit/test-missing-alias-report.txt create mode 100644 tests/data/test-abidiff-exit/test-missing-alias.abi create mode 100644 tests/data/test-abidiff-exit/test-missing-alias.suppr rename tests/data/test-abidiff/{test-PR18166-libtirpc.so.report.txt => empty-report.txt} (100%) delete mode 100644 tests/data/test-abidiff/test-PR24552-report0.txt create mode 100644 tests/data/test-abidiff/test-crc-0.xml create mode 100644 tests/data/test-abidiff/test-crc-1.xml create mode 100644 tests/data/test-abidiff/test-crc-2.xml create mode 100644 tests/data/test-abidiff/test-crc-report.txt delete mode 100644 tests/data/test-abidiff/test-empty-corpus-report.txt create mode 100644 tests/data/test-read-dwarf/test-suppressed-alias.c create mode 100644 tests/data/test-read-dwarf/test-suppressed-alias.o create mode 100644 tests/data/test-read-dwarf/test-suppressed-alias.o.abi create mode 100644 tests/data/test-read-dwarf/test-suppressed-alias.suppr create mode 100644 tests/data/test-read-dwarf/test3-alias-1.so.hash.abi create mode 100644 tests/data/test-read-dwarf/test3-alias-1.suppr create mode 100644 tests/data/test-read-dwarf/test3-alias-2.so.hash.abi create mode 100644 tests/data/test-read-dwarf/test3-alias-2.suppr create mode 100644 tests/data/test-read-dwarf/test3-alias-3.so.hash.abi create mode 100644 tests/data/test-read-dwarf/test3-alias-3.suppr create mode 100644 tests/data/test-read-dwarf/test3-alias-4.so.hash.abi create mode 100644 tests/data/test-read-dwarf/test3-alias-4.suppr create mode 100644 tests/data/test-read-write/test-crc.xml create mode 100644 tests/data/test-symtab/basic/aliases.c create mode 100755 tests/data/test-symtab/basic/aliases.so create mode 100644 tests/data/test-symtab/basic/one_function_one_variable_all.whitelist create mode 100644 tests/data/test-symtab/basic/one_function_one_variable_function.whitelist create mode 100644 tests/data/test-symtab/basic/one_function_one_variable_irrelevant.whitelist create mode 100644 tests/data/test-symtab/basic/one_function_one_variable_variable.whitelist create mode 100644 tests/data/test-symtab/kernel-modversions/Makefile create mode 120000 tests/data/test-symtab/kernel-modversions/one_of_each.c create mode 100644 tests/data/test-symtab/kernel-modversions/one_of_each.ko create mode 100644 tests/test-symtab-reader.cc