Two bugs:
- This tries to take the address of the last symbol plus the length of the last symbol. However, the sorted vector is cuPtrVector, not cuVector. Also, cuPtrVector has tombstone values removed and cuVector doesn't. If there was a stripped value at the end, the "last" element's value was UINT64_MAX, which meant the sentinel value was one less than the length of that "last" dead symbol.
- We have to subtract in.header->addr. For 64-bit binaries that's (1 << 32) and functionAddress is 32-bit so this is a no-op, but for 32-bit binaries the sentinel's value was too large.
Other than incorrectly claimed in the commit message, this does
make a difference in practice. The first-level
binary search code in libunwind (in UnwindCursor.hpp) does:
uint32_t low = 0;
uint32_t high = sectionHeader.indexCount();
uint32_t last = high - 1;
while (low < high) {
uint32_t mid = (low + high) / 2;
if (topIndex.functionOffset(mid) <= targetFunctionOffset) {
if ((mid == last) ||
(topIndex.functionOffset(mid + 1) > targetFunctionOffset)) {
low = mid;
break;
} else {
low = mid + 1;
}
} else {
high = mid;
}
}Imagine there's one real second-level page and one sentinel page.
low=0, high=2, mid=1. Before this change, the sentinel page at
mid could have an address smaller than the target address,
leading the binary search in the wrong direction. After the
change, the sentinel address will be larger than the address of
every function with unwind info, so the search will correctly look
in the one page.
(No test since I can't think of a way to make FileCheck check that one
number is larger than another. But the included assert covers at least
the first bug.)
Do you want to keep the static_cast?