[PATCH] D132918: [clang] Fix a crash in constant evaluation

Wed Aug 31 01:17:19 PDT 2022

kadircet added a comment.

sorry, i missed the other comments around having a lit test. reverting the change.

================
Comment at: clang/lib/AST/ExprConstant.cpp:4797
     }
+    // Can't access properties of an incomplete type.
+    if (!RD->hasDefinition()) {
----------------
shafik wrote:
> erichkeane wrote:
> > It seems to me that we shouldn't GET to this function with an incomplete type.  I suspect whoever calls this is doing so incorrectly.
> Also note we only check in `ExprConstant.cpp` for `hasDefinition()` in one other place in `findCompleteObject` and that is around extern see: https://github.com/llvm/llvm-project/commit/c0d04a2567c22631595bed8092bc042bb91ea4ee#diff-255a21a02a8966766225831836d482547787baf9a770fbf67178ebb7d7347e27
> It seems to me that we shouldn't GET to this function with an incomplete type. I suspect whoever calls this is doing so incorrectly.

Agreed, that's also my assumption. but we've been unable to get a minimal crasher. i am not a fan of landing these changes without reproducers but this was clearly fixing the issue we had (moreover, it's happening on invalid code).

moreover we're checking for recorddecl being invalid up above, so it felt quite likely to hit this code path with incomplete types as well (or there were some changes up the callstack that forgot to update the implementation here).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132918