DHowett-MSFT created this revision.
Herald added a project: All.
DHowett-MSFT updated this revision to Diff 495195.
DHowett-MSFT added a comment.
DHowett-MSFT published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
- Add a test
DHowett-MSFT added a comment.
This is a fix for https://github.com/llvm/llvm-project/issues/60543.
I originally approached this fix by moving `PragmaPackOptions` up to
`Lex/Token.h` to live near `PragmaLoopHintInfo`; however, that required moving
much more of the `PragmaMsStack` machinery than I was comfortable with.
There is precedent for some pragma infos living in `Sema` as well as
`ASTReader`/`ASTWriter` poking into `Sema` to do their jobs.
I've included a test that both validates that the frontend no longer crashes
_and_ that the resultant structures are packed as expected.
Without this, GCH serialization of late template expansions will
encounter an assertion failure.
Changes were required to ASTReader::ReadString to add support for
reading a string from a RecordDataImpl (which is the type consumed by
ReadToken) rather than a RecordData.
`#pragma pack` slot names are read into the preexisting string storage
PragmaAlignPackStrings.
This requires incidental changes [Parse,Sema] to move PragmaPackInfo to Sema
There is precedent for ASTReader/ASTWriter to poke into Sema.
Fixes https://github.com/llvm/llvm-project/issues/60543
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143410
Files:
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Parse/ParsePragma.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/PCH/delayed-template-with-pragma-pack.cpp
Index: clang/test/PCH/delayed-template-with-pragma-pack.cpp
===================================================================
--- /dev/null
+++ clang/test/PCH/delayed-template-with-pragma-pack.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -fdelayed-template-parsing -emit-pch -o %t.delayed.pch %s
+// RUN: %clang_cc1 -DMAIN_FILE \
+// RUN: -include-pch %t.pch \
+// RUN: -emit-llvm -verify -o - %s | FileCheck %s
+// RUN: %clang_cc1 -DMAIN_FILE -fdelayed-template-parsing \
+// RUN: -include-pch %t.delayed.pch \
+// RUN: -emit-llvm -verify -o - %s | FileCheck %s
+
+#ifndef MAIN_FILE
+
+template <typename T>
+int func() {
+#pragma pack(push, 1)
+ struct s { short a; T b; };
+#pragma pack(pop)
+ return sizeof(s);
+}
+
+#else
+
+// CHECK-LABEL: define linkonce_odr dso_local noundef i32 @"??$func@D@@YAHXZ"(
+// CHECK: ret i32 3
+int foo() {
+ return func<char>();
+}
+
+// CHECK-LABEL: define linkonce_odr dso_local noundef i32 @"??$func@F@@YAHXZ"(
+// CHECK: ret i32 4
+int bar() {
+ return func<short>();
+}
+
+// expected-no-diagnostics
+
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4412,6 +4412,14 @@
AddToken(T, Record);
break;
}
+ case tok::annot_pragma_pack: {
+ auto *Info =
+ static_cast<Sema::PragmaPackInfo *>(Tok.getAnnotationValue());
+ Record.push_back(static_cast<unsigned>(Info->Action));
+ AddString(Info->SlotLabel, Record);
+ AddToken(Info->Alignment, Record);
+ break;
+ }
// Some annotation tokens do not use the PtrData field.
case tok::annot_pragma_openmp:
case tok::annot_pragma_openmp_end:
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -1687,6 +1687,15 @@
Tok.setAnnotationValue(static_cast<void *>(Info));
break;
}
+ case tok::annot_pragma_pack: {
+ auto *Info = new (PP.getPreprocessorAllocator()) Sema::PragmaPackInfo;
+ Info->Action = static_cast<Sema::PragmaMsStackAction>(Record[Idx++]);
+ PragmaAlignPackStrings.push_back(ReadString(Record, Idx));
+ Info->SlotLabel = PragmaAlignPackStrings.back();
+ Info->Alignment = ReadToken(F, Record, Idx);
+ Tok.setAnnotationValue(static_cast<void *>(Info));
+ break;
+ }
// Some annotation tokens do not use the PtrData field.
case tok::annot_pragma_openmp:
case tok::annot_pragma_openmp_end:
@@ -9089,7 +9098,7 @@
}
// Read a string
-std::string ASTReader::ReadString(const RecordData &Record, unsigned &Idx) {
+std::string ASTReader::ReadString(const RecordDataImpl &Record, unsigned &Idx) {
unsigned Len = Record[Idx++];
std::string Result(Record.data() + Idx, Record.data() + Idx + Len);
Idx += Len;
Index: clang/lib/Parse/ParsePragma.cpp
===================================================================
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -665,18 +665,10 @@
Actions.ActOnPragmaVisibility(VisType, VisLoc);
}
-namespace {
-struct PragmaPackInfo {
- Sema::PragmaMsStackAction Action;
- StringRef SlotLabel;
- Token Alignment;
-};
-} // end anonymous namespace
-
void Parser::HandlePragmaPack() {
assert(Tok.is(tok::annot_pragma_pack));
- PragmaPackInfo *Info =
- static_cast<PragmaPackInfo *>(Tok.getAnnotationValue());
+ Sema::PragmaPackInfo *Info =
+ static_cast<Sema::PragmaPackInfo *>(Tok.getAnnotationValue());
SourceLocation PragmaLoc = Tok.getLocation();
ExprResult Alignment;
if (Info->Alignment.is(tok::numeric_constant)) {
@@ -2110,8 +2102,8 @@
return;
}
- PragmaPackInfo *Info =
- PP.getPreprocessorAllocator().Allocate<PragmaPackInfo>(1);
+ Sema::PragmaPackInfo *Info =
+ PP.getPreprocessorAllocator().Allocate<Sema::PragmaPackInfo>(1);
Info->Action = Action;
Info->SlotLabel = SlotLabel;
Info->Alignment = Alignment;
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -2236,7 +2236,7 @@
unsigned &Idx, LocSeq *Seq = nullptr);
// Read a string
- static std::string ReadString(const RecordData &Record, unsigned &Idx);
+ static std::string ReadString(const RecordDataImpl &Record, unsigned &Idx);
// Skip a string
static void SkipString(const RecordData &Record, unsigned &Idx) {
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -483,6 +483,12 @@
PSK_Pop_Set = PSK_Pop | PSK_Set, // #pragma (pop[, id], value)
};
+ struct PragmaPackInfo {
+ PragmaMsStackAction Action;
+ StringRef SlotLabel;
+ Token Alignment;
+ };
+
// #pragma pack and align.
class AlignPackInfo {
public:
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits