search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.1k stars 12.01k forks source link

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

Open cuviper opened 8 months ago

cuviper commented 8 months ago

Ref: https://github.com/rust-lang/rust/issues/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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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().