[BOLT] Turn non-empty CFI StateStack assert into a warning

[aaupov]

clang-15 can produce binaries with mismatched RememberState/RestoreState
CFIs. This is benign for unwinding, so replace an assert with a warning.

[llvmbot]

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/102216.diff

1 Files Affected:

• (modified) bolt/lib/Core/BinaryFunction.cpp (+4-1)

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index ea09371b57e8a..e5c6d551e42a7 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -2502,7 +2502,10 @@ void BinaryFunction::annotateCFIState() {
     }
   }
 
-  assert(StateStack.empty() && "corrupt CFI stack");
+  if (!StateStack.empty()) {
+    BC.errs() << "BOLT-WARNING: non-empty CFI stack at the end of " << *this
+              << '\n';
+  }
 }
 
 namespace {

[dcci]

Thanks for working on this. I don't understand CFI enough to comment on the correctness of this, hopefully @rafaelauler can chime in.

As a general point, I'm not entirely sure we should disable this assertion, without understanding.

In particular:

1. Is this something that the compiler produced that's corrupted?
   Should that be the case, we might consider either:
   a) fixing the compiler
   b) making this an error
   c) allow this to be a warning (if someone passes a flag to override at their own risk)

If 1) doesn't hold, is BOLT corrupting the stack somehow?

[rafaelauler]

The opposite, popping more items than pushed, would be a real concern. Since this assertion checks whether the CFI stack is inconsistent because it pushed more items than popped, it should be OK to convert the assertion into a warning.

[dcci]

Fine with this then. Maybe add a description/improve the commit message and then it's fine to merge.