lldb_assert: abort when assertions are enabled.

[JDevlieghere]

Summary

We had a long discussion in D59911 about lldb_assert and what it means. The result was the assert manifesto on https://lldb.llvm.org/resources/source.html:

Soft assertions. LLDB provides lldb_assert() as a soft alternative to cover the middle ground of situations that indicate a recoverable bug in LLDB. In a Debug configuration lldb_assert() behaves like assert(). In a Release configuration it will print a warning and encourage the user to file a bug report, similar to LLVM’s crash handler, and then return execution.

However, currently lldb_assert doesn't behave they way it's being described there: it doesn't abort in a debug/assert build.

[JDevlieghere]

Add comments with the exact wording from the docs.

[teemperor]

Would it be possible to assert after the printing the stack trace? Having stack traces in bug reports is always nice to have and displaying the "Assertion failed: ..." message seems also useful for the user.

[JDevlieghere]

In D64267#1571960, @teemperor wrote:

Would it be possible to assert after the printing the stack trace? Having stack traces in bug reports is always nice to have and displaying the "Assertion failed: ..." message seems also useful for the user.

The stack trace will be printed when the assert is hit, because we have a call to LLVM's pretty stack trace printer in the signal handler. That's the reason I moved the assert *before* the PrintStackTrace call.

Regarding the Assertion failed message, I wasn't sure if we wanted to print that twice, once coming form lldb_assert (with the real function, file and line) and once from the assert (with the function, file and line pointing to the lldb_assert implementation). If you think it's useful I'm happy to move the assertion to just after that print statement.

[teemperor]

In D64267#1571962, @JDevlieghere wrote:

In D64267#1571960, @teemperor wrote:

Would it be possible to assert after the printing the stack trace? Having stack traces in bug reports is always nice to have and displaying the "Assertion failed: ..." message seems also useful for the user.

The stack trace will be printed when the assert is hit, because we have a call to LLVM's pretty stack trace printer in the signal handler. That's the reason I moved the assert *before* the PrintStackTrace call.

Regarding the Assertion failed message, I wasn't sure if we wanted to print that twice, once coming form lldb_assert (with the real function, file and line) and once from the assert (with the function, file and line pointing to the lldb_assert implementation). If you think it's useful I'm happy to move the assertion to just after that print statement.

Ah, thanks for the explanation! The message was more of a positive side-effect of moving it afterwards, but I don't think it's worth moving this just for that message. So I think the current version is good to go then.

[davide]

lldb/source/Utility/LLDBAssert.cpp

25–26

llvm_unreachable?

[labath]

lldbassert(x) already expands to assert(x) in a debug (LLDB_CONFIGURATION_DEBUG) build. It sounds like you want this to assert in a non-debug build which has assertions enabled (Release+Assertions) ? If that's the case, then I think we should just change the #ifdef LLDB_CONFIGURATION_DEBUG in LLDBAssert.h into #ifdef _DEBUG..

[aprantl]

Did you see this recent commit?

commit 515d1306ffb1e159c65b19d4cbe6c2f0997dfbf6
Author: Adrian Prantl <[e-mail]>
Date:   Fri Mar 29 16:12:27 2019 +0000

    Don't abort() in lldb_assert and document why.
    
    rdar://problem/49356014
    
    Differential Revision: https://reviews.llvm.org/D59911
    
    llvm-svn: 357268

[aprantl]

Actually, it looks like this patch fixed an oversight in that commit. So all is good :-)

lldb/source/Utility/LLDBAssert.cpp

 using namespace lldb_private;
 
 void lldb_private::lldb_assert(bool expression, const char *expr_text,
                                const char *func, const char *file,
                                unsigned int line) {
   if (LLVM_LIKELY(expression))
     return;
 
+  // In a Debug configuration lldb_assert() behaves like assert().
+  assert(false && "lldb_assert failed");
+
+  // In a Release configuration it will print a warning and encourage the user
+  // to file a bug report, similar to LLVM’s crash handler, and then return
+  // execution.
   errs() << format("Assertion failed: (%s), function %s, file %s, line %u\n",
                    expr_text, func, file, line);
   errs() << "backtrace leading to the failure:\n";
   llvm::sys::PrintStackTrace(errs());
   errs() << "please file a bug report against lldb reporting this failure "
             "log, and as many details as possible\n";
 }