From 8b8ba810c157a6b3e3a517014198648999d7808e Mon Sep 17 00:00:00 2001 From: Benjamin Barenblat Date: Wed, 2 Aug 2023 12:58:13 -0400 Subject: Fix unit tests on ppc64 Backport a patch from upstream to fix symbolization on ppc64, and enable unit tests on that platform. --- debian/changelog | 2 + debian/patches/series | 1 + debian/patches/symbolize-ppc64.diff | 118 ++++++++++++++++++++++++++++++++++++ debian/rules | 4 +- 4 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 debian/patches/symbolize-ppc64.diff (limited to 'debian') diff --git a/debian/changelog b/debian/changelog index 6abc1938..9a6cdc08 100644 --- a/debian/changelog +++ b/debian/changelog @@ -3,6 +3,8 @@ abseil (20230125.3-2) UNRELEASED; urgency=medium * Reenable unit tests, which were accidentally disabled in the previous version. * Backport an upstream patch to allow building tests on mipsel. + * Backport a patch from upstream to correct symbolization on ppc64, and + enable unit tests on that platform. -- Benjamin Barenblat Tue, 27 Jun 2023 17:00:12 -0400 diff --git a/debian/patches/series b/debian/patches/series index 66bf33ce..000d6558 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -5,3 +5,4 @@ empty-flags-library.diff cordz-info-statistics-test.diff pkg-config-directives.diff split-hash-tests.diff +symbolize-ppc64.diff diff --git a/debian/patches/symbolize-ppc64.diff b/debian/patches/symbolize-ppc64.diff new file mode 100644 index 00000000..a11c27d4 --- /dev/null +++ b/debian/patches/symbolize-ppc64.diff @@ -0,0 +1,118 @@ +From: Benjamin Barenblat +Subject: Fix symbolization on PowerPC ELF v1 +Origin: backport, https://github.com/abseil/abseil-cpp/commit/372bfc86105728732fc115af46223d7a4e49f8d9 + +The big-endian PowerPC ELF ABI (ppc64 in Debian) relies on function descriptors +mapped in a non-executable segment. Make sure that segment is scanned during +symbolization. Also correct bounds computation for that segment. + +--- a/absl/debugging/symbolize_elf.inc ++++ b/absl/debugging/symbolize_elf.inc +@@ -648,8 +648,10 @@ + } + + // Return true if an address is inside a section. +-static bool InSection(const void *address, const ElfW(Shdr) * section) { +- const char *start = reinterpret_cast(section->sh_addr); ++static bool InSection(const void *address, ptrdiff_t relocation, ++ const ElfW(Shdr) * section) { ++ const char *start = reinterpret_cast( ++ section->sh_addr + static_cast(relocation)); + size_t size = static_cast(section->sh_size); + return start <= address && address < (start + size); + } +@@ -689,8 +691,8 @@ + // starting address. However, we do not always want to use the real + // starting address because we sometimes want to symbolize a function + // pointer into the .opd section, e.g. FindSymbol(&foo,...). +- const bool pc_in_opd = +- kPlatformUsesOPDSections && opd != nullptr && InSection(pc, opd); ++ const bool pc_in_opd = kPlatformUsesOPDSections && opd != nullptr && ++ InSection(pc, relocation, opd); + const bool deref_function_descriptor_pointer = + kPlatformUsesOPDSections && opd != nullptr && !pc_in_opd; + +@@ -730,7 +732,7 @@ + #endif + + if (deref_function_descriptor_pointer && +- InSection(original_start_address, opd)) { ++ InSection(original_start_address, /*relocation=*/0, opd)) { + // The opd section is mapped into memory. Just dereference + // start_address to get the first double word, which points to the + // function entry. +@@ -1326,7 +1328,7 @@ + const int phnum = obj->elf_header.e_phnum; + const int phentsize = obj->elf_header.e_phentsize; + auto phoff = static_cast(obj->elf_header.e_phoff); +- size_t num_executable_load_segments = 0; ++ size_t num_interesting_load_segments = 0; + for (int j = 0; j < phnum; j++) { + ElfW(Phdr) phdr; + if (!ReadFromOffsetExact(obj->fd, &phdr, sizeof(phdr), phoff)) { +@@ -1335,23 +1337,35 @@ + return false; + } + phoff += phentsize; +- constexpr int rx = PF_X | PF_R; +- if (phdr.p_type != PT_LOAD || (phdr.p_flags & rx) != rx) { +- // Not a LOAD segment, or not executable code. ++ ++#if defined(__powerpc__) && !(_CALL_ELF > 1) ++ // On the PowerPC ELF v1 ABI, function pointers actually point to function ++ // descriptors. These descriptors are stored in an .opd section, which is ++ // mapped read-only. We thus need to look at all readable segments, not ++ // just the executable ones. ++ constexpr int interesting = PF_R; ++#else ++ constexpr int interesting = PF_X | PF_R; ++#endif ++ ++ if (phdr.p_type != PT_LOAD ++ || (phdr.p_flags & interesting) != interesting) { ++ // Not a LOAD segment, not executable code, and not a function ++ // descriptor. + continue; + } +- if (num_executable_load_segments < obj->phdr.size()) { +- memcpy(&obj->phdr[num_executable_load_segments++], &phdr, sizeof(phdr)); ++ if (num_interesting_load_segments < obj->phdr.size()) { ++ memcpy(&obj->phdr[num_interesting_load_segments++], &phdr, sizeof(phdr)); + } else { + ABSL_RAW_LOG( +- WARNING, "%s: too many executable LOAD segments: %zu >= %zu", +- obj->filename, num_executable_load_segments, obj->phdr.size()); ++ WARNING, "%s: too many interesting LOAD segments: %zu >= %zu", ++ obj->filename, num_interesting_load_segments, obj->phdr.size()); + break; + } + } +- if (num_executable_load_segments == 0) { +- // This object has no "r-x" LOAD segments. That's unexpected. +- ABSL_RAW_LOG(WARNING, "%s: no executable LOAD segments", obj->filename); ++ if (num_interesting_load_segments == 0) { ++ // This object has no interesting LOAD segments. That's unexpected. ++ ABSL_RAW_LOG(WARNING, "%s: no interesting LOAD segments", obj->filename); + return false; + } + } +@@ -1379,8 +1393,8 @@ + // X in the file will have a start address of [true relocation]+X. + relocation = static_cast(start_addr - obj->offset); + +- // Note: some binaries have multiple "rx" LOAD segments. We must +- // find the right one. ++ // Note: some binaries have multiple LOAD segments that can contain ++ // function pointers. We must find the right one. + ElfW(Phdr) *phdr = nullptr; + for (size_t j = 0; j < obj->phdr.size(); j++) { + ElfW(Phdr) &p = obj->phdr[j]; +@@ -1390,7 +1404,7 @@ + ABSL_RAW_CHECK(p.p_type == PT_NULL, "unexpected p_type"); + break; + } +- if (pc < reinterpret_cast(start_addr + p.p_memsz)) { ++ if (pc < reinterpret_cast(start_addr + p.p_vaddr + p.p_memsz)) { + phdr = &p; + break; + } diff --git a/debian/rules b/debian/rules index e835a80d..7f76c033 100755 --- a/debian/rules +++ b/debian/rules @@ -19,10 +19,8 @@ export DEB_BUILD_MAINT_OPTIONS = hardening=+bindnow reproducible=+fixfilepath # Unit tests require more than 2 GB of RAM, so disable them on mipsel. # -# Unit tests are not yet passing on ppc64, so disable them there as well. -# # Disable unit tests unconditionally if nocheck is set. -ifneq ($(filter $(DEB_HOST_ARCH),mipsel ppc64),) +ifeq ($(DEB_HOST_ARCH),mipsel) ABSL_RUN_TESTS=OFF else ifneq ($(filter nocheck,$(DEB_BUILD_OPTIONS)),) ABSL_RUN_TESTS=OFF -- cgit v1.2.3