[libunwind] fix pc range condition check bug #154902
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libunwind Author: Wu Yingcong (yingcong-wu) ChangesThere is an off-by-one error with current condition check for PC fallen into the range or not. There is another check within libunwind that use the correct checks in llvm-project/libunwind/src/UnwindCursor.hpp Line 2757 in 5050da7 Full diff: https://github.com/llvm/llvm-project/pull/154902.diff 1 Files Affected:
diff --git a/libunwind/src/DwarfParser.hpp b/libunwind/src/DwarfParser.hpp
index 7e85025dd054d..25250e0810987 100644
--- a/libunwind/src/DwarfParser.hpp
+++ b/libunwind/src/DwarfParser.hpp
@@ -273,7 +273,7 @@ bool CFI_Parser<A>::findFDE(A &addressSpace, pint_t pc, pint_t ehSectionStart,
pint_t pcRange = addressSpace.getEncodedP(
p, nextCFI, cieInfo->pointerEncoding & 0x0F);
// Test if pc is within the function this FDE covers.
- if ((pcStart < pc) && (pc <= pcStart + pcRange)) {
+ if ((pcStart <= pc) && (pc < pcStart + pcRange)) {
// parse rest of info
fdeInfo->lsda = 0;
// check for augmentation length
|
trigger ci
|
For stage3 (bootstrapping-build, llvm-premerge-libcxx-next-runners), I don't think the failure is related to my change. As for the buildkite failures, I don't have a clue, but it should not relate to this change either. I see this from the log |
|
We should add a regression test that fails before and passes after this change |
|
Added an e2e regression test. I have to manually mark the .eh_frame_hdr as omitted in order to make libunwind do the linear search, which will invoke the affected logic. I assume the function address is located at the beginning of an FDE record(I think that should be a fair assumption). Local test result: And here is the result using libgcc_s's unwind implementation |
fmayer
approved these changes
Aug 28, 2025
fmayer
reviewed
Aug 29, 2025
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
As this test result shows, the test case also fails without my change to libunwind: https://github.com/llvm/llvm-project/actions/runs/17313128556/job/49152917070?pr=154902 (head is be13b4d8). |
fmayer
approved these changes
Aug 29, 2025
|
Hi @fmayer , do you think this PR is ready to merge based on the CI results? I don't think the failed checks are related to my change. In my opinion, it looks more like infrastructure issues. |
|
Seems so to me. The same errors in a different change: https://buildkite.com/llvm-project/libcxx-ci/builds/68485/steps/canvas?sid=0198efcd-f547-4db6-a3b3-bc7b498b4e62 |
|
@yingcong-wu The new test fails on the Arm bots. Please fix it soon or revert to get the precommit CI green again (finally). |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/193/builds/10240 Here is the relevant piece of the build log for the reference |
Sure, I will take a quick look now. |
|
Looks like there is no objcopy available for that build, I think adding the unsupport for that build would do it, I will open a PR for it. |
|
Never mind, it is a cross build setup, which builds on Windows env and the binary will be copied to and run on Linux env. |
|
Open PR #156383 to unsupport this case. |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Sep 2, 2025
…ch may have cross toolchain build (#156383) In llvm/llvm-project#154902, the test failed with llvm-clang-win-x-aarch64(it is a cross-build, which builds on Windows and run on Linux, "Win to Aarch64 Linux Ubuntu Cross Toolchain"), and objdump is not available on Windows(the build env). Set to require x86 Linux instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There is an off-by-one error with current condition check for PC fallen into the range or not. There is another check within libunwind that use the correct checks in
llvm-project/libunwind/src/UnwindCursor.hpp
Line 2757 in 5050da7