[clang] Fixes and closes #53952. Setting the ASTHasCompilerErrors member variable correctly based on the PP diagnostics. (PR #68127)

[Rajkumar Ananthu via cfe-commits]

https://github.com/rajkumarananthu updated https://github.com/llvm/llvm-project/pull/68127

>From b76f4d351b3e55e2095e3be088a50fdb76d6b7f9 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu <rajkumar.ananthu108 at gmail.com>
Date: Tue, 3 Oct 2023 21:53:59 +0530
Subject: [PATCH 1/2] Fixes and closes #53952

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.
---
 clang/lib/Serialization/ASTWriter.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

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);

>From bd8a37df74b5bcac05d7eb7d74986d8c5095bb10 Mon Sep 17 00:00:00 2001
From: Rajkumar Ananthu <rajkumar.ananthu108 at gmail.com>
Date: Thu, 5 Oct 2023 01:29:32 +0530
Subject: [PATCH 2/2] Removing hasErrors argument from ASTWriter::WriteAST
 method as suggested by the review

---
 clang/include/clang/Serialization/ASTWriter.h |  1 -
 clang/lib/Frontend/ASTUnit.cpp                | 17 +++++------------
 clang/lib/Serialization/ASTWriter.cpp         | 10 ++--------
 clang/lib/Serialization/GeneratePCH.cpp       |  8 ++------
 4 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/clang/include/clang/Serialization/ASTWriter.h b/clang/include/clang/Serialization/ASTWriter.h
index f2c7c03ff093607..98445d40ebd82c3 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -613,7 +613,6 @@ class ASTWriter : public ASTDeserializationListener,
   /// the module but currently is merely a random 32-bit number.
   ASTFileSignature WriteAST(Sema &SemaRef, StringRef OutputFile,
                             Module *WritingModule, StringRef isysroot,
-                            bool hasErrors = false,
                             bool ShouldCacheASTInMemory = false);
 
   /// Emit a token.
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 016f88a43a56ddd..85157c3b21b6648 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -2341,12 +2341,9 @@ bool ASTUnit::Save(StringRef File) {
   return false;
 }
 
-static bool serializeUnit(ASTWriter &Writer,
-                          SmallVectorImpl<char> &Buffer,
-                          Sema &S,
-                          bool hasErrors,
-                          raw_ostream &OS) {
-  Writer.WriteAST(S, std::string(), nullptr, "", hasErrors);
+static bool serializeUnit(ASTWriter &Writer, SmallVectorImpl<char> &Buffer,
+                          Sema &S, raw_ostream &OS) {
+  Writer.WriteAST(S, std::string(), nullptr, "");
 
   // Write the generated bitstream to "Out".
   if (!Buffer.empty())
@@ -2356,18 +2353,14 @@ static bool serializeUnit(ASTWriter &Writer,
 }
 
 bool ASTUnit::serialize(raw_ostream &OS) {
-  // For serialization we are lenient if the errors were only warn-as-error kind.
-  bool hasErrors = getDiagnostics().hasUncompilableErrorOccurred();
-
   if (WriterData)
-    return serializeUnit(WriterData->Writer, WriterData->Buffer,
-                         getSema(), hasErrors, OS);
+    return serializeUnit(WriterData->Writer, WriterData->Buffer, getSema(), OS);
 
   SmallString<128> Buffer;
   llvm::BitstreamWriter Stream(Buffer);
   InMemoryModuleCache ModuleCache;
   ASTWriter Writer(Stream, Buffer, ModuleCache, {});
-  return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS);
+  return serializeUnit(Writer, Buffer, getSema(), OS);
 }
 
 using SLocRemap = ContinuousRangeMap<unsigned, int, 2>;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 35f37de9b1850ad..0acd86de06ba404 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -4622,18 +4622,12 @@ time_t ASTWriter::getTimestampForOutput(const FileEntry *E) const {
 
 ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, StringRef OutputFile,
                                      Module *WritingModule, StringRef isysroot,
-                                     bool hasErrors,
                                      bool ShouldCacheASTInMemory) {
   llvm::TimeTraceScope scope("WriteAST", 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;
-  }
+  ASTHasCompilerErrors =
+      SemaRef.PP.getDiagnostics().hasUncompilableErrorOccurred();
 
   // Emit the file header.
   Stream.Emit((unsigned)'C', 8);
diff --git a/clang/lib/Serialization/GeneratePCH.cpp b/clang/lib/Serialization/GeneratePCH.cpp
index 601a24b4aec46ad..cf8084333811f13 100644
--- a/clang/lib/Serialization/GeneratePCH.cpp
+++ b/clang/lib/Serialization/GeneratePCH.cpp
@@ -65,12 +65,8 @@ void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {
 
   // Emit the PCH file to the Buffer.
   assert(SemaPtr && "No Sema?");
-  Buffer->Signature =
-      Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot,
-                      // For serialization we are lenient if the errors were
-                      // only warn-as-error kind.
-                      PP.getDiagnostics().hasUncompilableErrorOccurred(),
-                      ShouldCacheASTInMemory);
+  Buffer->Signature = Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot,
+                                      ShouldCacheASTInMemory);
 
   Buffer->IsComplete = true;
 }