[clang] [clang] Prevent dangling StringRefs (PR #98699)

[Youngsuk Kim via cfe-commits]

https://github.com/JOE1994 updated https://github.com/llvm/llvm-project/pull/98699

>From 07648c7efab15786c36dbcd265015d945b627725 Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <youngsuk.kim at hpe.com>
Date: Fri, 12 Jul 2024 17:40:59 -0500
Subject: [PATCH 1/2] [clang] Prevent dangling StringRefs

Fix locations where dangling StringRefs are created.

* `ConstraintSatisfaction::SubstitutionDiagnostic`:
   typedef of `std::pair<SourceLocation, StringRef>`

* `concepts::Requirement::SubstitutionDiagnostic`:
   struct whose 1st and 3rd data members are `StringRef`s

Fixes #98667
---
 clang/lib/Serialization/ASTReaderStmt.cpp | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 6ccb4b01a036a..da67a4fcab5cb 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -796,10 +796,13 @@ readConstraintSatisfaction(ASTRecordReader &Record) {
       if (/* IsDiagnostic */Record.readInt()) {
         SourceLocation DiagLocation = Record.readSourceLocation();
         std::string DiagMessage = Record.readString();
+        char *DBuf = new (Record.getContext()) char[DiagMessage.size()];
+        std::copy(DiagMessage.begin(), DiagMessage.end(), DBuf);
+
         Satisfaction.Details.emplace_back(
             new (Record.getContext())
                 ConstraintSatisfaction::SubstitutionDiagnostic(DiagLocation,
-                                                               DiagMessage));
+                                                               StringRef(DBuf, DiagMessage.size())));
       } else
         Satisfaction.Details.emplace_back(Record.readExpr());
     }
@@ -821,11 +824,18 @@ void ASTStmtReader::VisitConceptSpecializationExpr(
 static concepts::Requirement::SubstitutionDiagnostic *
 readSubstitutionDiagnostic(ASTRecordReader &Record) {
   std::string SubstitutedEntity = Record.readString();
+  char *SBuf = new (Record.getContext()) char[SubstitutedEntity.size()];
+  std::copy(SubstitutedEntity.begin(), SubstitutedEntity.end(), SBuf);
+
   SourceLocation DiagLoc = Record.readSourceLocation();
   std::string DiagMessage = Record.readString();
+  char *DBuf = new (Record.getContext()) char[DiagMessage.size()];
+  std::copy(DiagMessage.begin(), DiagMessage.end(), DBuf);
+
   return new (Record.getContext())
-      concepts::Requirement::SubstitutionDiagnostic{SubstitutedEntity, DiagLoc,
-                                                    DiagMessage};
+      concepts::Requirement::SubstitutionDiagnostic{
+          StringRef(SBuf, SubstitutedEntity.size()), DiagLoc,
+          StringRef(DBuf, DiagMessage.size())};
 }
 
 void ASTStmtReader::VisitRequiresExpr(RequiresExpr *E) {

>From 3b4dc1fe464bb914ac76d460d96dc39f75afd79a Mon Sep 17 00:00:00 2001
From: Youngsuk Kim <youngsuk.kim at hpe.com>
Date: Sat, 13 Jul 2024 10:04:37 -0500
Subject: [PATCH 2/2] Extract mem alloc & string copy logic to helper function

Add Clang release note item
---
 clang/docs/ReleaseNotes.rst               |  2 ++
 clang/lib/Serialization/ASTReaderStmt.cpp | 28 ++++++++++++-----------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5dc0f8b7e0bbb..055b426860df2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -830,6 +830,8 @@ Bug Fixes in This Version
 - ``__is_trivially_equality_comparable`` no longer returns true for types which
   have a constrained defaulted comparison operator (#GH89293).
 
+- Fixed Clang from generating dangling StringRefs when deserializing Exprs & Stmts (#GH98667)
+
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index da67a4fcab5cb..b2b0a30457d23 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -785,6 +785,12 @@ void ASTStmtReader::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *E) {
   E->setRParenLoc(readSourceLocation());
 }
 
+static StringRef saveStrToCtx(const std::string &S, ASTContext &Ctx) {
+  char *Buf = new (Ctx) char[S.size()];
+  std::copy(S.begin(), S.end(), Buf);
+  return StringRef(Buf, S.size());
+}
+
 static ConstraintSatisfaction
 readConstraintSatisfaction(ASTRecordReader &Record) {
   ConstraintSatisfaction Satisfaction;
@@ -795,14 +801,13 @@ readConstraintSatisfaction(ASTRecordReader &Record) {
     for (unsigned i = 0; i != NumDetailRecords; ++i) {
       if (/* IsDiagnostic */Record.readInt()) {
         SourceLocation DiagLocation = Record.readSourceLocation();
-        std::string DiagMessage = Record.readString();
-        char *DBuf = new (Record.getContext()) char[DiagMessage.size()];
-        std::copy(DiagMessage.begin(), DiagMessage.end(), DBuf);
+        StringRef DiagMessage =
+            saveStrToCtx(Record.readString(), Record.getContext());
 
         Satisfaction.Details.emplace_back(
             new (Record.getContext())
                 ConstraintSatisfaction::SubstitutionDiagnostic(DiagLocation,
-                                                               StringRef(DBuf, DiagMessage.size())));
+                                                               DiagMessage));
       } else
         Satisfaction.Details.emplace_back(Record.readExpr());
     }
@@ -823,19 +828,16 @@ void ASTStmtReader::VisitConceptSpecializationExpr(
 
 static concepts::Requirement::SubstitutionDiagnostic *
 readSubstitutionDiagnostic(ASTRecordReader &Record) {
-  std::string SubstitutedEntity = Record.readString();
-  char *SBuf = new (Record.getContext()) char[SubstitutedEntity.size()];
-  std::copy(SubstitutedEntity.begin(), SubstitutedEntity.end(), SBuf);
+  StringRef SubstitutedEntity =
+      saveStrToCtx(Record.readString(), Record.getContext());
 
   SourceLocation DiagLoc = Record.readSourceLocation();
-  std::string DiagMessage = Record.readString();
-  char *DBuf = new (Record.getContext()) char[DiagMessage.size()];
-  std::copy(DiagMessage.begin(), DiagMessage.end(), DBuf);
+  StringRef DiagMessage =
+      saveStrToCtx(Record.readString(), Record.getContext());
 
   return new (Record.getContext())
-      concepts::Requirement::SubstitutionDiagnostic{
-          StringRef(SBuf, SubstitutedEntity.size()), DiagLoc,
-          StringRef(DBuf, DiagMessage.size())};
+      concepts::Requirement::SubstitutionDiagnostic{SubstitutedEntity, DiagLoc,
+                                                    DiagMessage};
 }
 
 void ASTStmtReader::VisitRequiresExpr(RequiresExpr *E) {