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

SIGSEGV under DenseMapBase::erase after std::bad_alloc #85281

Open
cuviper opened this issue Mar 14, 2024 · 4 comments
Open

SIGSEGV under DenseMapBase::erase after std::bad_alloc #85281

cuviper opened this issue Mar 14, 2024 · 4 comments

Comments

@cuviper
Copy link
Member

cuviper commented Mar 14, 2024

Ref: rust-lang/rust#121305

In that Rust issue, running on an i686 host, I found that DenseMap::allocateBuckets -> llvm::allocate_buffer -> operator new was throwing std::bad_alloc. Then when unwinding runs the destructors into LLVMContextDispose, we get back into DenseMapBase::erase and hit SIGSEGV, presumably in the same instance that failed allocation.

Maybe allocate_buffer should catch and call report_bad_alloc_error like the safe_*alloc functions do? But even so, it seems that some part of DenseMap is not exception safe.

@nikic
Copy link
Contributor

nikic commented Mar 14, 2024

As far as I know LLVM generally does not try to be exception-safe. It is compiled with -fno-exceptions by default.

In Fedora we enable LLVM_ENABLE_EH for unknown reasons -- this seems like a good motivation to disable it and see if anything breaks.

@nikic
Copy link
Contributor

nikic commented Mar 15, 2024

In Fedora we enable LLVM_ENABLE_EH for unknown reasons -- this seems like a good motivation to disable it and see if anything breaks.

Hm, no, I misremembered: We use LLVM_ENABLE_RTTI, not LLVM_ENABLE_EH.

Whether operator new throws an exception is determined by whether libstdc++ has been built with -fno-exceptions, not LLVM.

With that in mind, your idea of catching the exception in allocate_buffer and explicitly aborting sounds reasonable to me.

cuviper added a commit to cuviper/llvm-project that referenced this issue Mar 15, 2024
Previously, it called `::operator new` which may throw `std::bad_alloc`,
regardless of whether LLVM itself was built with exception handling, and
this can cause safety issues if outside code has destructors that will
call back into LLVM. Now we use `::operator new(..., nothrow)` and call
`llvm::report_bad_alloc_error` when allocation fails, which will abort
when LLVM is built without exceptions.

Ref: llvm#85281
@cuviper
Copy link
Member Author

cuviper commented Mar 15, 2024

See #85449.

However, I just noticed that LLVM does have a std::new_handler that reports too, which should be installed by InitLLVM -- but Rust doesn't call that. Do you think we should? (rust-lang/rust#79153 looks related)

@nikic
Copy link
Contributor

nikic commented Mar 15, 2024

See #85449.

However, I just noticed that LLVM does have a std::new_handler that reports too, which should be installed by InitLLVM -- but Rust doesn't call that. Do you think we should? (rust-lang/rust#79153 looks related)

Oh, that's a great find! So that's how OOM is supposed to be handled.

I'm not sure we really want everything in InitLLVM, but it looks like we should at least be calling install_out_of_memory_new_handler().

bors added a commit to rust-lang-ci/rust that referenced this issue Mar 16, 2024
Register LLVM handlers for bad-alloc / OOM

LLVM's default bad-alloc handler may throw if exceptions are enabled,
and `operator new` isn't hooked at all by default. Now we register our
own handler that prints a message similar to fatal errors, then aborts.
We also call the function that registers the C++ `std::new_handler`.

Fixes rust-lang#121305
Cc llvm/llvm-project#85281
r? `@nikic`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2024
Register LLVM handlers for bad-alloc / OOM

LLVM's default bad-alloc handler may throw if exceptions are enabled,
and `operator new` isn't hooked at all by default. Now we register our
own handler that prints a message similar to fatal errors, then aborts.
We also call the function that registers the C++ `std::new_handler`.

Fixes rust-lang#121305
Cc llvm/llvm-project#85281
r? `@nikic`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Mar 17, 2024
Register LLVM handlers for bad-alloc / OOM

LLVM's default bad-alloc handler may throw if exceptions are enabled,
and `operator new` isn't hooked at all by default. Now we register our
own handler that prints a message similar to fatal errors, then aborts.
We also call the function that registers the C++ `std::new_handler`.

Fixes rust-lang#121305
Cc llvm/llvm-project#85281
r? ``@nikic``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 17, 2024
Rollup merge of rust-lang#122574 - cuviper:llvm-oom, r=nikic

Register LLVM handlers for bad-alloc / OOM

LLVM's default bad-alloc handler may throw if exceptions are enabled,
and `operator new` isn't hooked at all by default. Now we register our
own handler that prints a message similar to fatal errors, then aborts.
We also call the function that registers the C++ `std::new_handler`.

Fixes rust-lang#121305
Cc llvm/llvm-project#85281
r? ``@nikic``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants