[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I like the implementation strategy, but I have one corner case, which is probably worth a test.



================
Comment at: clang/lib/CodeGen/CGCleanup.cpp:848
 
       // Build a switch-out if we need it:
       //   - if there are branch-afters threaded through the scope
----------------
Are we sure this is the right set of predicates? It seems like we'll get the wrong flag in cases like:
```
  __try {
    mayThrow();
    goto out;  // AbnormalTermination should be 1
  } __finally {
    tearDown();
  }
out:
  return 0;
```

I think these conditions are designed to avoid the switch when there is no normal fallthrough destination (!HasFallthrough). I see clang emits no switch for this C++ input:

```
$ cat t.cpp
void mayThrow();
struct Dtor {
  ~Dtor();
};
int f() {
  do {
    Dtor o;
    mayThrow();
    break;
  } while (0);
  return 0;
}

$ clang t.cpp  -S -emit-llvm -fno-exceptions -o - | grep switch
# empty
```


================
Comment at: clang/lib/CodeGen/CGCleanup.cpp:863-864
 
+        // pass the abnormal exit flag to Fn (SEH cleanup)
+        cleanupFlags.setHasSehAbnormalExits();
+
----------------
This flag is set for any cleanup that needs to switch-out to multiple normal destinations. You have named this an "seh abnormal exit", an exit that is abnormal for the purposes of SEH, but I think it would be clearer to give the flag a more general name.

Based on the existing terminology, maybe (F_)HasBranchAfter would be the closest to what we are looking for?


================
Comment at: clang/lib/CodeGen/EHScopeStack.h:184
 
+      bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+      void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
----------------
Please add a comment describing the accessor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77936/new/

https://reviews.llvm.org/D77936