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.

Sign up for GitHub

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

Jump to bottom

[7.0 Regression] libc++abi incorrectly aligns exceptions in 32 bit builds #38399

Closed
llvmbot opened this issue Sep 22, 2018 · 11 comments
Closed

[7.0 Regression] libc++abi incorrectly aligns exceptions in 32 bit builds #38399

llvmbot opened this issue Sep 22, 2018 · 11 comments
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. regression

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 22, 2018

Bugzilla Link 39051
Resolution FIXED
Resolved on Oct 22, 2018 09:29
Version 7.0
OS Linux
Blocks #38454
Reporter LLVM Bugzilla Contributor
CC @zmodem,@ldionne,@mclow,@tstellar
Fixed by commit(s) r342815 r344917

Extended Description

Libc++ commit r339431 moved the _LIBCPP_HAS_NO_ALIGNED_ALLOCATION logic from to <__config>. As a result, it was defined when building libc++abi's fallback_malloc.cpp, which made libc++abi think posix_memalign was unavailable.

This meant that libc++abi was silently falling back to using malloc. In 32 bit builds, malloc does not return correctly aligned memory for the exception header.

This is a regression in the 7.0 release.

The issues was fixed in libc++abi commit r342815. This change should be merged into the 7.1 release.

@​Marshall, Louis, can you sign off on this?
@ldionne
Copy link
Member

ldionne commented Sep 22, 2018

Signed off. How did you discover the bug? We should have testers for this, no?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 22, 2018

How did you discover the bug? We should have testers for this, no?

We do. The libc++abi 32 bit buildbot has been failing for months due to this bug [1].

I suspect it's also the cause of #37821 .

[1] http://lab.llvm.org:8011/builders/libcxx-libcxxabi-x86_64-linux-ubuntu-32bit/builds/424

@ldionne
Copy link
Member

ldionne commented Sep 22, 2018

How did you discover the bug? We should have testers for this, no?
We do. The libc++abi 32 bit buildbot has been failing for months due to this bug [1].

How embarrassing -- that one's on me.

I suspect it's also the cause of #37821 .

I don't think so: my reproduction for #37821 still fails after applying your patch.

@llvmbot
Copy link
Collaborator Author

llvmbot commented Sep 22, 2018

I suspect it's also the cause of #37821 .

I don't think so: my reproduction for #37821 still fails after applying your patch.

Yeah, looking further, I think the analysis about the separate unwind headers on that bug seems correct.

@tstellar
Copy link
Collaborator

Marshall, is this OK to merge?

@mclow
Copy link
Contributor

mclow commented Oct 19, 2018

Marshall, is this OK to merge?

Yes; this is OK to merge for 7.1

@mclow
Copy link
Contributor

mclow commented Oct 19, 2018

Is there any reason that this bug should not be closed?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Oct 20, 2018

Have we merged the commit? That would be the reason to keep this open.

@ldionne
Copy link
Member

ldionne commented Oct 22, 2018

I don't think the commit has been merged to the release branch for LLVM 7.1

@tstellar
Copy link
Collaborator

Merged: r344917

@tstellar
Copy link
Collaborator

mentioned in issue #38454

@tstellar tstellar mentioned this issue Sep 27, 2018
Closed
@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. regression
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tstellar @mclow @ldionne @llvmbot