Abort in cleanup leads to unwinding failure

[nikic]

Consider the following example:

#include <cstdlib>
#include <cstdio>

struct Abort {
  ~Abort() {
    puts("cleanup");
    abort();
  }
};

__attribute__((noinline))
static void abort_in_dtor() {
  Abort abort;
  throw "test";
}

int main() {
  try {
    abort_in_dtor();
  } catch (...) {
    puts("caught");
  }
}

This should print "cleanup" followed by abort. Instead this happens when using Clang 17:

terminate called after throwing an instance of 'char const*'
Aborted (core dumped)

The reason for this is that we now (correctly) infer that abort_in_dtor() is nounwind. However, as phase 1 unwind skips cleanups, this means that the unwind walk will try to go past a nounwind frame, which may not have an LSDA entry, resulting in some form of unwinding failure. (In this specific case we return _URC_END_OF_STACK, but a similar case in Rust results in _URC_FATAL_PHASE1_ERROR.)

[llvmbot]

@llvm/issue-subscribers-clang-codegen

[nikic]

cc @efriedma-quic for unwinding

This regressed due to https://reviews.llvm.org/D145210, but that's only because we are now better at inferring nounwind. The problem itself is not new.

[efriedma-quic]

From C++ [except.handle]: If no matching handler is found, the function std​::​terminate is invoked; whether or not the stack is unwound before this invocation of std​::​terminate is implementation-defined

I think libunwind prefers to terminate before unwinding.

Given that, I think we should not infer nounwind for abort_in_dtor; even though control flow never actually unwinds past it, the first pass of unwinding does try to look through it.

[nikic]

Given that, I think we should not infer nounwind for abort_in_dtor; even though control flow never actually unwinds past it, the first pass of unwinding does try to look through it.

Yeah, that sounds like a pragmatic fix. Would it be fine to just special-case cleanup landingpads in FunctionAttrs inference, without actually making them mayThrow()?

[efriedma-quic]

Special-case seems fine; it's not actually control flow in the sense most users of mayThrow() care about.

[nikic]

Candidate patch: https://reviews.llvm.org/D147694