[Lldb-commits] [PATCH] D59911: Don't abort() in lldb_assert and document why.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: jingham, clayborg, labath, zturner, jasonmolenda.
Herald added a project: LLDB.

See the changes to the website for the full rationale.

Having a policy document that explains when to use what method of error handling in LLDB will make on-boarding new community members easier and help make sense of all the different options. Let me know if there is a better location for this text and/or if you disagree with the contents!

rdar://problem/49356014


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59911

Files:
  lldb/source/Utility/LLDBAssert.cpp
  lldb/www/source.html


Index: lldb/www/source.html
===================================================================
--- lldb/www/source.html
+++ lldb/www/source.html
@@ -62,7 +62,51 @@
             </ul>
             For anything not explicitly listed here, assume that LLDB follows the LLVM policy.
           </p>
-        </div>
+                                </div>
+                        </div>
+			<div class="post">
+                        <h1 class ="postheader">Error handling and use of assertions in LLDB</h1>
+			<div class="postcontent">
+                        <p>
+                        Contrary to clang, which is typically a short-lived process, LLDB debuggers
+                        stay up and running for a long time, often serving multiple debug sessions
+                        initiated by an IDE. For this reason LLDB code needs to be extra thoughtful
+                        about how to handle errors. Below are a couple rule of thumbs:
+                        <ul>
+                          <li>Fatal errors.
+                            Aborting LLDB's process using <tt>llvm::report_fatal_error()</tt> or <tt>abort</tt>
+                            should be avoided at all costs.  It's acceptable to use
+                            <a href="http://llvm.org/doxygen/Support_2ErrorHandling_8h.html"><tt>llvm_unreachable()</tt></a>
+                            for actually unreachable code such as the default in an otherwise exhaustive switch statement.
+                          </li>
+                          <li>Assertions.
+                            Assertions (from <tt>assert.h</tt>) should be used liberally to assert internal consistency.
+                            Assertions shall <em>never</em> be used to detect invalid user input, such as malformed DWARF.
+                            An assertion should be placed to assert invariants that the developer is convinced will
+                            always hold, regardless what an end-user does with LLDB.
+                          </li>
+                          <li>Invalid input.
+                            To deal with invalid input, such as malformed DWARF, missing object files, or otherwise
+                            inconsistent debug info, LLVM's error handling types such as
+                            <a href="http://llvm.org/doxygen/classllvm_1_1Expected.html"><tt>llvm::Expected<T></tt></a>
+                            should be used. Functions that may fail should return an
+                            <a href="http://llvm.org/doxygen/classllvm_1_1Optional.html"><tt>llvm::Optional<T></tt></a>
+                            result.
+                          </li>
+                          <li>Soft assertions.
+                            LLDB provides <tt>lldb_assert()</tt> as a soft alternative to cover the middle ground
+                            of situations that really indicate a bug in LLDB, but could conceivably happen.
+                            In a Debug configuration <tt>lldb_assert()</tt> behaves like
+                            <tt>assert()</tt>. 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. Use these sparingly and only if error handling is not otherwise feasible.
+                          </li>
+                        </ul>
+                        Overall, please keep in mind that the debugger is often used as a last resort,
+                        and a crash in the debugger is rarely appreciated by the end-user.
+                        </p>
+                        </div>
+                        </div>
 				<div class="postfooter"></div>
 			</div>
 		</div>
Index: lldb/source/Utility/LLDBAssert.cpp
===================================================================
--- lldb/source/Utility/LLDBAssert.cpp
+++ lldb/source/Utility/LLDBAssert.cpp
@@ -27,5 +27,4 @@
   llvm::sys::PrintStackTrace(errs());
   errs() << "please file a bug report against lldb reporting this failure "
             "log, and as many details as possible\n";
-  abort();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59911.192543.patch
Type: text/x-patch
Size: 4200 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190327/f906acca/attachment-0001.bin>