Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics.

[rajkumarananthu]

The issue #53952 is reported indicating clang is giving a crashing pch file, when hasErrors is been passed incorrectly to WriteAST method.

To fix the issue, I have a added an assertion to make sure the given value of ASTHasCompilerErrors is matching with Preprocessor diagnostics. And this assertion will get triggered inside Debug builds.

For release builds, based on the conditional check, forcefully set the ASTHasCompilerErrors member variable to a valid value from Preprocessor.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Changes

The issue #53952 is reported indicating clang is giving a crashing pch file, when hasErrors is been passed incorrectly to WriteAST method.

To fix the issue, I have a added an assertion to make sure the given value of ASTHasCompilerErrors is matching with Preprocessor diagnostics. And this assertion will get triggered inside Debug builds.

For release builds, based on the conditional check, forcefully set the ASTHasCompilerErrors member variable to a valid value from Preprocessor.

---

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

1 Files Affected:

• (modified) clang/lib/Serialization/ASTWriter.cpp (+6)

diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 201e2fcaaec91aa..35f37de9b1850ad 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4628,6 +4628,12 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
+  bool trueHasErrors = SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();
+  assert(ASTHasCompilerErrors == trueHasErrors);
+  if (trueHasErrors != ASTHasCompilerErrors) {
+      // forcing the compiler errors flag to be set correctly
+      ASTHasCompilerErrors = trueHasErrors;
+  }
 
   // Emit the file header.
   Stream.Emit((unsigned)'C', 8);

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[shafik]

The fix does not look correct but perhaps someone else will have more insight.

[AaronBallman]

Thank you for the fix, the changes LGTM! The precommit CI failure appears to be unrelated to these changes. I don't think this requires test coverage or a release note.

[rajkumarananthu]

Hi @AaronBallman I could see a revert of my change here: https://reviews.llvm.org/rGa6acf3fd49a20c570a390af2a3c84e10b9545b68

Can you please help me confirm whether the change is accepted or reverted. Should we resubmit the changes here?

Thanks
Rajkumar Ananthu

[phyBrackets]

Not sure but the issue in the reverted commit seems like a backporting issue, so you may need to backport the changes in case. But I'd say wait for Aaron comment.

[AaronBallman]

There was a mix-up. I landed the original changes, @kazutakahirata saw what appeared to be a linker error after pulling those changes down and did a revert, but that turned out to be an issue local to their machine so I've reverted the revert (re-landing the original changes). So the code should be in now and should hopefully stay in,.

[rajkumarananthu]

Thanks for taking care of this @AaronBallman

[kazutakahirata]

Thank you @AaronBallman!