cxxabi.h suggested for __forced_unwind not in code

[lshamis]

I'm running iwyu over my code base, and there are a few files where it insists I add

#include <cxxabi.h>  // for __forced_unwind

I was able to ablate code and find that it seems related to cv.wait(mu).

Specifically, the no predicate version.

If I remove cv.wait(mu), iwyu no longer suggests cxxabi.h.

What is this __forced_unwind function, why is iwyu recommending it, and can this safely be ignored?

Minimal test case:

// foo.cpp

#include <condition_variable>
#include <mutex>

void foo(std::condition_variable_any* cv, std::mutex* mu) {
  cv->wait(*mu);
}

src/foo.cpp should add these lines:
#include <cxxabi.h>            // for __forced_unwind

src/foo.cpp should remove these lines:

The full include-list for src/foo.cpp:
#include <cxxabi.h>            // for __forced_unwind
#include <condition_variable>  // for condition_variable_any
#include <mutex>               // for mutex
---

[kimgr]

I'm pretty sure it can be safely ignored.

What is this __forced_unwind function, why is iwyu recommending it, and can this safely be ignored?

I tried to read up on forced unwind, and it appears to be a GCC runtime thing invoked when the runtime knows that unwind needs to be performed, but the language can't see it, e.g. longjmp: http://www.ucw.cz/~hubicka/papers/abi/node25.html.

condition_variable catches it to make sure a mutex is released on unwind, but it definitely shouldn't need to be included in user code using condition_variable.

[kimgr]

On a whim, I moved the VisitCXXCatchStmt from the base visitor to the derived visitor:

diff --git a/iwyu.cc b/iwyu.cc
index 92837a8..785bdc5 100644
--- a/iwyu.cc
+++ b/iwyu.cc
@@ -1890,22 +1890,6 @@ class IwyuBaseAstVisitor : public BaseAstVisitor<Derived> {
   //------------------------------------------------------------
   // Visitors of types derived from clang::Stmt.
 
-  // Catch statements always require the full type to be visible,
-  // no matter if we're catching by value, reference or pointer.
-  bool VisitCXXCatchStmt(clang::CXXCatchStmt* stmt) {
-    if (CanIgnoreCurrentASTNode()) return true;
-
-    if (const Type* caught_type = stmt->getCaughtType().getTypePtrOrNull()) {
-      // Strip off pointers/references to get to the 'base' type.
-      caught_type = RemovePointersAndReferencesAsWritten(caught_type);
-      ReportTypeUse(CurrentLoc(), caught_type);
-    } else {
-      // catch(...): no type to act on here.
-    }
-
-    return Base::VisitCXXCatchStmt(stmt);
-  }
-
   // The type of the for-range-init expression is fully required, because the
   // compiler generates method calls to it, e.g. 'for (auto t : ts)' translates
   // roughly into 'for (auto i = std::begin(ts); i != std::end(ts); ++i)'.
@@ -3939,6 +3923,21 @@ class IwyuAstConsumer
   }
 
   // --- Visitors of types derived from clang::Stmt.
+  // Catch statements always require the full type to be visible,
+  // no matter if we're catching by value, reference or pointer.
+  bool VisitCXXCatchStmt(clang::CXXCatchStmt* stmt) {
+    if (CanIgnoreCurrentASTNode()) return true;
+
+    if (const Type* caught_type = stmt->getCaughtType().getTypePtrOrNull()) {
+      // Strip off pointers/references to get to the 'base' type.
+      caught_type = RemovePointersAndReferencesAsWritten(caught_type);
+      ReportTypeUse(CurrentLoc(), caught_type);
+    } else {
+      // catch(...): no type to act on here.
+    }
+
+    return Base::VisitCXXCatchStmt(stmt);
+  }
 
   // Called whenever a variable, function, enum, etc is used.
   bool VisitDeclRefExpr(clang::DeclRefExpr* expr) {

... and that seems to have fixed the problem without breaking any tests.

That tells me there must be negative test coverage missing for try/catch in templates. But it's also interesting, cause there's probably more visits in the base visitor that shouldn't apply to templates.

Not sure, because I find it hard to reason about user templates vs system templates, but this patch might work around your local problem.

[progheal]

Did a quick grep through the include directory of g++ 8.5.0, and found the following places have the offending __catch(__cxxabiv1::__forced_unwind):

• <bitset>, for >> into std::bitset
• <condition_variable>, inside std::condition_variable (like in OP), specifically the "scoped unlock" helper inside it
• <future>, for std::future, specifically the internal structure storing the task, and the async state
• <iomanip>, for << std::put_time(), >> std::get_time(), << std::put_money() and >> std::get_money(). (I encounter this problem in one of the project I worked on when using std::put_time.)

There are some internal headers that also have this construct, but I can't really decipher what these implementation are for so I'll leave them out.

Hopefully this will make people easier to find out where your code is "using" it and how to work around that.

[kimgr]

Fixed on master as of ffe6c3f