Skip to content

Commit e395f08

Browse files
committed
test(pack): split the PE case so it can localise its own crash
The combined test died with SIGSEGV on the macOS ARM64 runner and nowhere else — not under ASan+UBSan with clang 22 + libc++, not as a clang module on x86_64 Linux, not under gcc. A single test that builds a fixture, identifies it and parses it cannot say which of the three it was, so each hypothesis costs a CI round. Three tests now: the fixture is well-formed, identify(), and needed_names().
1 parent 44f58bd commit e395f08

1 file changed

Lines changed: 31 additions & 5 deletions

File tree

tests/unit/test_pack_binfmt.cpp

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,25 +229,51 @@ TEST(PackBinfmt, AnElfWithNoDynamicSectionHasZeroDepsAndIsNotAnError) {
229229
EXPECT_TRUE(names->empty());
230230
}
231231

232-
TEST(PackBinfmt, IdentifiesPeAndReadsBothImportDirectories) {
232+
// SPLIT INTO THREE ON PURPOSE. The combined version died with SIGSEGV on the
233+
// macOS ARM64 runner and nowhere else, and a single test that builds a
234+
// fixture, identifies it and parses it cannot say WHICH of the three it was.
235+
// A test that cannot localise its own failure is a test that costs a CI round
236+
// per hypothesis.
237+
TEST(PackBinfmt, ThePeFixtureItselfIsWellFormed) {
233238
std::array<std::string_view, 2> imports{"KERNEL32.dll", "vcruntime140.dll"};
234239
std::array<std::string_view, 1> delayed{"dbghelp.dll"};
235-
TempFile f{"pe", pe_with_imports(imports, delayed)};
240+
auto bytes = pe_with_imports(imports, delayed);
241+
ASSERT_GT(bytes.size(), 0x400u);
242+
EXPECT_EQ(bytes.substr(0, 2), "MZ");
243+
EXPECT_EQ(bytes.substr(0x40, 4), std::string("PE\0\0", 4));
244+
// The names have to be IN the image, or every assertion below is about
245+
// the fixture rather than about the reader.
246+
for (auto want : {"KERNEL32.dll", "vcruntime140.dll", "dbghelp.dll"})
247+
EXPECT_NE(bytes.find(want), std::string::npos) << want;
248+
}
249+
250+
TEST(PackBinfmt, IdentifiesPe) {
251+
std::array<std::string_view, 2> imports{"KERNEL32.dll", "vcruntime140.dll"};
252+
std::array<std::string_view, 1> delayed{"dbghelp.dll"};
253+
TempFile f{"peid", pe_with_imports(imports, delayed)};
236254

237255
auto id = bf::identify(f.path);
238256
EXPECT_EQ(id.format, bf::Format::Pe);
239257
EXPECT_EQ(id.arch, "x86_64");
240258
EXPECT_TRUE(id.is64);
259+
}
260+
261+
TEST(PackBinfmt, ReadsBothPeImportDirectories) {
262+
std::array<std::string_view, 2> imports{"KERNEL32.dll", "vcruntime140.dll"};
263+
std::array<std::string_view, 1> delayed{"dbghelp.dll"};
264+
TempFile f{"peimp", pe_with_imports(imports, delayed)};
241265

242266
auto names = bf::needed_names(f.path);
243267
ASSERT_TRUE(names.has_value()) << names.error();
244268
// The DELAY-loaded one is a dependency too, and leaving it out is worse
245269
// than leaving out an ordinary import: a missing delay-load does not fail
246270
// at startup, it fails at the first call through it.
247271
EXPECT_EQ(names->size(), 3u);
248-
for (auto want : {"KERNEL32.dll", "vcruntime140.dll", "dbghelp.dll"})
249-
EXPECT_NE(std::ranges::find(*names, want), names->end())
250-
<< want << " missing from the closure";
272+
for (auto want : {"KERNEL32.dll", "vcruntime140.dll", "dbghelp.dll"}) {
273+
bool found = false;
274+
for (auto const& n : *names) found = found || n == want;
275+
EXPECT_TRUE(found) << want << " missing from the closure";
276+
}
251277
}
252278

253279
TEST(PackBinfmt, ADosStubWithoutAPeSignatureIsNotAPe) {

0 commit comments

Comments
 (0)