Author: Yao Qi Date: 2026-09-08T20:46:47+01:00 New Revision: 4afa58e64810be046a9427d3cf7827a0dc2c235c
URL: https://github.com/llvm/llvm-project/commit/4afa58e64810be046a9427d3cf7827a0dc2c235c DIFF: https://github.com/llvm/llvm-project/commit/4afa58e64810be046a9427d3cf7827a0dc2c235c.diff LOG: [lldb] Fix infinite recursion in ArchSpec's core matching (#221899) `target create` never returns for a little-endian 32-bit MIPS ELF file, on any host whose platform advertises `armv7m`/`armv7em` compatibility. On Darwin that is every platform, since `PlatformDarwin::GetCompatibleArchs()` lists both for every arm64/arm64e host and falls through to that same list for any other host core. It is found by `lldb-target-fuzzer`. Reproduce with a 52-byte ELF32 header (`ELFCLASS32`, `ELFDATA2LSB`, `e_machine = EM_MIPS`, `e_flags = EF_MIPS_ARCH_32`, everything else zero), saved as `mips32el.elf`: ``` (lldb) target create mips32el.elf ``` This never returns. Attaching a debugger to the hung process shows the same three frames looping forever, with the call stack never growing: ``` frame #1: cores_match(core1=eCore_arm_armv7m, core2=eCore_mips32el, try_inverse=false, ...) at ArchSpec.cpp:1575 [inlined] frame #2: ArchSpec::IsMatch(rhs=..., match=CompatibleMatch) at ArchSpec.cpp:1449 frame #3: Platform::IsCompatibleArchitecture(...) at Platform.cpp:1193 ``` `cores_match()` checks `core1` against `core2`. When nothing matches and the caller did not ask for an exact match, it sets `try_inverse` and retries once with the two cores swapped. It retries because the rule tables are not symmetric: a core lists only what it accepts, not everything that accepts it. Every case that reaches this fallback must leave `try_inverse` false on the way out, so the swap happens at most once. Three cases, `eCore_arm_armv7em`, `eCore_arm_armv7m`, and `eCore_mips32el`, instead set it back to true unconditionally. When both sides of a comparison are one of these three, the swapped call re-arms `try_inverse`, and the tail call back into `cores_match()` recurses forever. This is a tail call, so an optimizing compiler turns the recursion into a loop instead of a growing stack. That is why the process spins instead of crashing with a stack overflow. The three cases cannot simply leave `try_inverse` false like their siblings. `ArchSpec("mipsel").IsCompatibleMatch(ArchSpec("mips64el"))` is true today only because of this same swap: `eCore_mips32el`'s own rule says nothing about 64-bit MIPS, but `eCore_mips64el`'s rule accepts any 32-bit MIPS core with the same endianness, and only the inverted comparison reaches that rule. Instead, split the switch into a one-directional helper that never recurses, and try both directions from a plain, non-recursive wrapper: ``` cores_match(A, B) := cores_match_one_direction(A, B) || cores_match_one_direction(B, A) ``` This keeps every existing one-directional rule exactly as it was, including the mips32el/mips64el rule above, and makes a second retry impossible no matter what any case does. Adds two regression tests to ArchSpecTest.cpp. One asserts that the previously-hanging pairs now return false right away. The other locks in the two relationships that depend on checking both directions, so a future change cannot bring the recursion back by "fixing" the three cases the naive way. Assisted-by: claude Added: Modified: lldb/source/Utility/ArchSpec.cpp lldb/unittests/Utility/ArchSpecTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp index 0c9202e3b5e43..af2299e3c7480 100644 --- a/lldb/source/Utility/ArchSpec.cpp +++ b/lldb/source/Utility/ArchSpec.cpp @@ -25,7 +25,7 @@ using namespace lldb; using namespace lldb_private; static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, - bool try_inverse, bool enforce_exact_match); + bool enforce_exact_match); namespace lldb_private { @@ -1446,7 +1446,7 @@ static bool IsCompatibleEnvironment(llvm::Triple::EnvironmentType lhs, bool ArchSpec::IsMatch(const ArchSpec &rhs, MatchType match) const { if (GetByteOrder() != rhs.GetByteOrder() || - !cores_match(GetCore(), rhs.GetCore(), true, match == ExactMatch)) + !cores_match(GetCore(), rhs.GetCore(), match == ExactMatch)) return false; const llvm::Triple &lhs_triple = GetTriple(); @@ -1570,11 +1570,12 @@ void ArchSpec::CoreUpdated(bool update_triple) { //===----------------------------------------------------------------------===// // Operators. -static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, - bool try_inverse, bool enforce_exact_match) { - if (core1 == core2) - return true; - +// Checks core1's rules only, because they are not symmetric. Must not call +// cores_match() here, or an unmatched pair would loop between the two +// directions forever. +static bool cores_match_one_direction(const ArchSpec::Core core1, + const ArchSpec::Core core2, + bool enforce_exact_match) { switch (core1) { case ArchSpec::kCore_any: return true; @@ -1642,7 +1643,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_armv7) return true; - try_inverse = true; } break; @@ -1660,7 +1660,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_armv7em) return true; - try_inverse = true; } break; @@ -1678,7 +1677,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_armv6m) return true; - try_inverse = false; } break; @@ -1692,14 +1690,12 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_armv7) return true; - try_inverse = false; } break; case ArchSpec::eCore_x86_64_x86_64h: case ArchSpec::eCore_x86_64_amd64: if (!enforce_exact_match) { - try_inverse = false; if (core2 == ArchSpec::eCore_x86_64_x86_64) return true; } @@ -1713,7 +1709,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_arm64e) return true; - try_inverse = false; } break; @@ -1725,7 +1720,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_armv8) return true; - try_inverse = false; } break; case ArchSpec::eCore_arm_aarch64: @@ -1736,7 +1730,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_arm64e) return true; - try_inverse = false; } break; @@ -1748,7 +1741,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 == ArchSpec::eCore_arm_arm64e) return true; - try_inverse = false; } break; @@ -1756,7 +1748,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, if (!enforce_exact_match) { if (core2 == ArchSpec::eCore_arm_generic) return true; - try_inverse = false; } break; @@ -1765,7 +1756,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, if (core2 >= ArchSpec::kCore_mips32_first && core2 <= ArchSpec::kCore_mips32_last) return true; - try_inverse = false; } break; @@ -1774,7 +1764,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, if (core2 >= ArchSpec::kCore_mips32el_first && core2 <= ArchSpec::kCore_mips32el_last) return true; - try_inverse = true; } break; @@ -1786,7 +1775,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, if (core2 >= ArchSpec::kCore_mips64_first && core2 <= ArchSpec::kCore_mips64_last) return true; - try_inverse = false; } break; @@ -1798,7 +1786,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, if (core2 >= ArchSpec::kCore_mips64el_first && core2 <= ArchSpec::kCore_mips64el_last) return true; - try_inverse = false; } break; @@ -1810,7 +1797,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 >= ArchSpec::kCore_mips64_first && core2 <= (core1 - 1)) return true; - try_inverse = false; } break; @@ -1822,7 +1808,6 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, return true; if (core2 >= ArchSpec::kCore_mips64el_first && core2 <= (core1 - 1)) return true; - try_inverse = false; } break; @@ -1882,11 +1867,18 @@ static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, default: break; } - if (try_inverse) - return cores_match(core2, core1, false, enforce_exact_match); return false; } +// The match rules only work in one direction, so this checks both ways. +static bool cores_match(const ArchSpec::Core core1, const ArchSpec::Core core2, + bool enforce_exact_match) { + if (core1 == core2) + return true; + return cores_match_one_direction(core1, core2, enforce_exact_match) || + cores_match_one_direction(core2, core1, enforce_exact_match); +} + bool lldb_private::operator<(const ArchSpec &lhs, const ArchSpec &rhs) { const ArchSpec::Core lhs_core = lhs.GetCore(); const ArchSpec::Core rhs_core = rhs.GetCore(); diff --git a/lldb/unittests/Utility/ArchSpecTest.cpp b/lldb/unittests/Utility/ArchSpecTest.cpp index 7027c7648a273..25ce85dd7f9f7 100644 --- a/lldb/unittests/Utility/ArchSpecTest.cpp +++ b/lldb/unittests/Utility/ArchSpecTest.cpp @@ -482,6 +482,46 @@ TEST(ArchSpecTest, Compatibility) { } } +TEST(ArchSpecTest, UnrelatedCoresCompatibleMatchTerminates) { + { + ArchSpec A("mipsel-unknown-linux"); + ArchSpec B("armv7em-apple-none"); + ASSERT_FALSE(A.IsCompatibleMatch(B)); + ASSERT_FALSE(B.IsCompatibleMatch(A)); + } + { + ArchSpec A("mipsel-unknown-linux"); + ArchSpec B("armv7m-apple-none"); + ASSERT_FALSE(A.IsCompatibleMatch(B)); + ASSERT_FALSE(B.IsCompatibleMatch(A)); + } +} + +TEST(ArchSpecTest, AsymmetricCoreRulesAreCheckedBothWays) { + // Some cores' compatibility rules only work one way: core A accepts B, + // but B's own rule says nothing about A. A compatible match must check + // both directions to find these cases. + { + // A 32-bit MIPS core has no rule about 64-bit MIPS, but 64-bit MIPS + // accepts the 32-bit family with the same endianness. + ArchSpec A("mipsel-unknown-linux"); + ArchSpec B("mips64el-unknown-linux"); + ASSERT_TRUE(A.IsCompatibleMatch(B)); + ASSERT_TRUE(B.IsCompatibleMatch(A)); + ASSERT_FALSE(A.IsExactMatch(B)); + ASSERT_FALSE(B.IsExactMatch(A)); + } + { + // The two Cortex-M cores explicitly accept each other. + ArchSpec A("armv7em-apple-none"); + ArchSpec B("armv7m-apple-none"); + ASSERT_TRUE(A.IsCompatibleMatch(B)); + ASSERT_TRUE(B.IsCompatibleMatch(A)); + ASSERT_FALSE(A.IsExactMatch(B)); + ASSERT_FALSE(B.IsExactMatch(A)); + } +} + TEST(ArchSpecTest, WasmCompatibility) { // A Wasm module encodes no vendor or OS: those are properties of the runtime // executing it. A bare wasm32 or wasm64 architecture therefore has to stay _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
