From 9f5b49644e1cc67a6e315fa7805bfe0c688def11 Mon Sep 17 00:00:00 2001 From: Benjamin Barenblat Date: Thu, 14 Apr 2022 13:19:44 -0400 Subject: Reenable unit tests on hppa --- debian/changelog | 1 + debian/control | 2 +- debian/patches/hppa-symbolize.diff | 103 +++++++++++++++++++++ debian/patches/series | 2 + debian/patches/str-format-convert-test-printf.diff | 103 +++++++++++++++++++++ debian/rules | 5 +- 6 files changed, 212 insertions(+), 4 deletions(-) create mode 100644 debian/patches/hppa-symbolize.diff create mode 100644 debian/patches/str-format-convert-test-printf.diff diff --git a/debian/changelog b/debian/changelog index d20492e2..a9ff3c79 100644 --- a/debian/changelog +++ b/debian/changelog @@ -2,6 +2,7 @@ abseil (0~20210324.2-3) UNRELEASED; urgency=medium * Backport an upstream patch to disable a problematic unit test. (Closes: #1007136) + * Reenable unit tests on hppa. -- Benjamin Barenblat Thu, 31 Mar 2022 14:10:30 -0400 diff --git a/debian/control b/debian/control index dde19493..b31cbce9 100644 --- a/debian/control +++ b/debian/control @@ -18,7 +18,7 @@ Maintainer: Benjamin Barenblat Build-Depends: cmake (>= 3.5), debhelper-compat (= 12), - googletest (>= 1.10.0.20200926) [!hppa !mipsel !ppc64] + googletest (>= 1.10.0.20200926) [!mipsel !ppc64] Rules-Requires-Root: no Standards-Version: 4.6.0 Section: libs diff --git a/debian/patches/hppa-symbolize.diff b/debian/patches/hppa-symbolize.diff new file mode 100644 index 00000000..8c039df9 --- /dev/null +++ b/debian/patches/hppa-symbolize.diff @@ -0,0 +1,103 @@ +From: Benjamin Barenblat +Subject: Support symbolization on PA-RISC +Forwarded: yes +Applied-Upstream: https://github.com/abseil/abseil-cpp/commit/7f850b3167fb38e6b4a9ce1824e6fabd733b5d62 + +Null out supervisor bits in PA-RISC addresses before symbolizing, and +handle function descriptor tables correctly. + +Change symbolize_test.cc to use 32-bit aligned addresses, allowing that +test to pass on PA-RISC. + +The author works at Google. Upstream applied this patch as Piper +revision 428590564 and exported it to GitHub; the Applied-Upstream URL +above points to the exported commit. + +--- a/absl/debugging/symbolize_elf.inc ++++ b/absl/debugging/symbolize_elf.inc +@@ -319,6 +319,7 @@ + const ptrdiff_t relocation, + char *out, int out_size, + char *tmp_buf, int tmp_buf_size); ++ const char *GetUncachedSymbol(const void *pc); + + enum { + SYMBOL_BUF_SIZE = 3072, +@@ -1329,13 +1330,7 @@ + // they are called here as well. + // To keep stack consumption low, we would like this function to not + // get inlined. +-const char *Symbolizer::GetSymbol(const void *const pc) { +- const char *entry = FindSymbolInCache(pc); +- if (entry != nullptr) { +- return entry; +- } +- symbol_buf_[0] = '\0'; +- ++const char *Symbolizer::GetUncachedSymbol(const void *pc) { + ObjFile *const obj = FindObjFile(pc, 1); + ptrdiff_t relocation = 0; + int fd = -1; +@@ -1423,6 +1418,42 @@ + return InsertSymbolInCache(pc, symbol_buf_); + } + ++const char *Symbolizer::GetSymbol(const void *pc) { ++ const char *entry = FindSymbolInCache(pc); ++ if (entry != nullptr) { ++ return entry; ++ } ++ symbol_buf_[0] = '\0'; ++ ++#ifdef __hppa__ ++ { ++ // In some contexts (e.g., return addresses), PA-RISC uses the lowest two ++ // bits of the address to indicate the privilege level. Clear those bits ++ // before trying to symbolize. ++ const auto pc_bits = reinterpret_cast(pc); ++ const auto address = pc_bits & ~0x3; ++ entry = GetUncachedSymbol(reinterpret_cast(address)); ++ if (entry != nullptr) { ++ return entry; ++ } ++ ++ // In some contexts, PA-RISC also uses bit 1 of the address to indicate that ++ // this is a cross-DSO function pointer. Such function pointers actually ++ // point to a procedure label, a struct whose first 32-bit (pointer) element ++ // actually points to the function text. With no symbol found for this ++ // address so far, try interpreting it as a cross-DSO function pointer and ++ // see how that goes. ++ if (pc_bits & 0x2) { ++ return GetUncachedSymbol(*reinterpret_cast(address)); ++ } ++ ++ return nullptr; ++ } ++#else ++ return GetUncachedSymbol(pc); ++#endif ++} ++ + bool RemoveAllSymbolDecorators(void) { + if (!g_decorators_mu.TryLock()) { + // Someone else is using decorators. Get out. +--- a/absl/debugging/symbolize_test.cc ++++ b/absl/debugging/symbolize_test.cc +@@ -378,12 +378,14 @@ + DummySymbolDecorator, &c_message), + 0); + +- char *address = reinterpret_cast(1); +- EXPECT_STREQ("abc", TrySymbolize(address++)); ++ // Use addresses 4 and 8 here to ensure that we always use valid addresses ++ // even on systems that require instructions to be 32-bit aligned. ++ char *address = reinterpret_cast(4); ++ EXPECT_STREQ("abc", TrySymbolize(address)); + + EXPECT_TRUE(absl::debugging_internal::RemoveSymbolDecorator(ticket_b)); + +- EXPECT_STREQ("ac", TrySymbolize(address++)); ++ EXPECT_STREQ("ac", TrySymbolize(address + 4)); + + // Cleanup: remove all remaining decorators so other stack traces don't + // get mystery "ac" decoration. diff --git a/debian/patches/series b/debian/patches/series index 6a2a842b..f4bed7f4 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -14,3 +14,5 @@ big-endian-random2.diff big-endian-random3.diff big-endian-random4.diff disable-nominalcpufrequency-test.diff +hppa-symbolize.diff +str-format-convert-test-printf.diff diff --git a/debian/patches/str-format-convert-test-printf.diff b/debian/patches/str-format-convert-test-printf.diff new file mode 100644 index 00000000..1808f2e1 --- /dev/null +++ b/debian/patches/str-format-convert-test-printf.diff @@ -0,0 +1,103 @@ +From: Benjamin Barenblat +Subject: Check printf format strings in str_format_convert_test +Forwarded: yes +Applied-Upstream: https://github.com/abseil/abseil-cpp/commit/9fed77a6fea29b8c8468bd41c6259c7f67163a65 + +Add ABSL_PRINTF_ATTRIBUTE to appropriate functions in +strings/internal/str_format/convert_test. Correct +TypedFormatConvertTest.Char, which was accidentally passing values of +types larger than int to StrPrint. + +The author works at Google. Upstream applied this patch as Piper +revision 439388148 and exported it to GitHub; the Applied-Upstream URL +above points to the exported commit. + +--- a/absl/strings/BUILD.bazel ++++ b/absl/strings/BUILD.bazel +@@ -787,6 +787,7 @@ + deps = [ + ":str_format_internal", + ":strings", ++ "//absl/base:core_headers", + "//absl/base:raw_logging_internal", + "//absl/types:optional", + "@com_google_googletest//:gtest_main", +--- a/absl/strings/CMakeLists.txt ++++ b/absl/strings/CMakeLists.txt +@@ -492,6 +492,7 @@ + DEPS + absl::strings + absl::str_format_internal ++ absl::core_headers + absl::raw_logging_internal + absl::int128 + gmock_main +--- a/absl/strings/internal/str_format/convert_test.cc ++++ b/absl/strings/internal/str_format/convert_test.cc +@@ -24,6 +24,7 @@ + + #include "gmock/gmock.h" + #include "gtest/gtest.h" ++#include "absl/base/attributes.h" + #include "absl/base/internal/raw_logging.h" + #include "absl/strings/internal/str_format/bind.h" + #include "absl/strings/match.h" +@@ -124,6 +125,7 @@ + delete[] buf; + } + ++void StrAppend(std::string *, const char *, ...) ABSL_PRINTF_ATTRIBUTE(2, 3); + void StrAppend(std::string *out, const char *format, ...) { + va_list ap; + va_start(ap, format); +@@ -131,6 +133,7 @@ + va_end(ap); + } + ++std::string StrPrint(const char *, ...) ABSL_PRINTF_ATTRIBUTE(1, 2); + std::string StrPrint(const char *format, ...) { + va_list ap; + va_start(ap, format); +@@ -452,21 +455,32 @@ + } + + TYPED_TEST_P(TypedFormatConvertTest, Char) { ++ // Pass a bunch of values of type TypeParam to both FormatPack and libc's ++ // vsnprintf("%c", ...) (wrapped in StrPrint) to make sure we get the same ++ // value. + typedef TypeParam T; + using remove_volatile_t = typename std::remove_volatile::type; +- static const T kMin = std::numeric_limits::min(); +- static const T kMax = std::numeric_limits::max(); +- T kVals[] = { +- remove_volatile_t(1), remove_volatile_t(2), remove_volatile_t(10), +- remove_volatile_t(-1), remove_volatile_t(-2), remove_volatile_t(-10), +- remove_volatile_t(0), +- kMin + remove_volatile_t(1), kMin, +- kMax - remove_volatile_t(1), kMax ++ std::vector vals = { ++ remove_volatile_t(1), remove_volatile_t(2), remove_volatile_t(10), // ++ remove_volatile_t(-1), remove_volatile_t(-2), remove_volatile_t(-10), // ++ remove_volatile_t(0), + }; +- for (const T &c : kVals) { ++ ++ // We'd like to test values near std::numeric_limits::min() and ++ // std::numeric_limits::max(), too, but vsnprintf("%c", ...) can't handle ++ // anything larger than an int. Add in the most extreme values we can without ++ // exceeding that range. ++ static const T kMin = ++ static_cast(std::numeric_limits::min()); ++ static const T kMax = ++ static_cast(std::numeric_limits::max()); ++ vals.insert(vals.end(), {kMin + 1, kMin, kMax - 1, kMax}); ++ ++ for (const T c : vals) { + const FormatArgImpl args[] = {FormatArgImpl(c)}; + UntypedFormatSpecImpl format("%c"); +- EXPECT_EQ(StrPrint("%c", c), FormatPack(format, absl::MakeSpan(args))); ++ EXPECT_EQ(StrPrint("%c", static_cast(c)), ++ FormatPack(format, absl::MakeSpan(args))); + } + } + diff --git a/debian/rules b/debian/rules index ef27843d..536ca72a 100755 --- a/debian/rules +++ b/debian/rules @@ -19,9 +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 hppa or ppc64, so disable them there as -# well. -ifneq ($(filter $(DEB_HOST_ARCH),hppa mipsel ppc64),) +# Unit tests are not yet passing on ppc64, so disable them there as well. +ifneq ($(filter $(DEB_HOST_ARCH),mipsel ppc64),) ABSL_RUN_TESTS=OFF else ABSL_RUN_TESTS=ON -- cgit v1.2.3