Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BOLT] Fix C++ exceptions when LPStart is specified #72737

Merged
merged 2 commits into from
Nov 21, 2023

Conversation

maksfb
Copy link
Contributor

@maksfb maksfb commented Nov 18, 2023

Whenever LPStartEncoding was different from DW_EH_PE_omit, we used to miscalculate LPStart. As a result, landing pads were assigned wrong addresses. Fix that.

Whenever LPStartEncoding was different from DW_EH_PE_omit, we used to
miscalculate LPStart. As a result, landing pads were assigned wrong
addresses. Fix that.
@aaupov
Copy link
Contributor

aaupov commented Nov 18, 2023

The new test failed in pre-merge checks.

@maksfb
Copy link
Contributor Author

maksfb commented Nov 18, 2023

Yes. But passing locally…

@maksfb maksfb added the BOLT label Nov 19, 2023

# Test case compiled from:
#
# int main() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the build command to produce main code and exception table?

Copy link
Contributor Author

@maksfb maksfb Nov 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang++ input.cpp -o output.s -S

if (LPOffset) {
if (LandingPad) {
// Verify if landing pad code is located outside current function
// Support landing pad to builtin_unreachable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you mean the comment, it's moved from a different code location.

@maksfb
Copy link
Contributor Author

maksfb commented Nov 19, 2023

FYI: the reason the test is failing is unrelated to exceptions or the changes. Will post a separate PR.

@maksfb
Copy link
Contributor Author

maksfb commented Nov 20, 2023

Rebuilt the test case with -O1 to get rid of the __builtin_unreachable() problem described in #72899.

@maksfb maksfb merged commit 8460206 into llvm:main Nov 21, 2023
3 checks passed
@maksfb maksfb deleted the gh-fix-exceptions branch November 22, 2023 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants