DHowett-MSFT updated this revision to Diff 495285.
DHowett-MSFT added a comment.
@mikerice Alright, it turns out that PragmaPackAlignStorage isn't suitable for
this (actually, I'm not sure what it _is_
suitable for) because it's deleted by the time the template gets parsed again.
The `push, slot` case failed nondeterministically because the slot name is
being UAF'd.
For now (and I am not sure this is correct), I've chosen to store the slot name
in the preprocessor storage.
I suspect that other serialized pack info (not from the pragma, but the attr
version that is stored in PragmaPackAlignStorage) is subject to the same
limitation...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143410/new/
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,78 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-pch -o %t.pch %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fdelayed-template-parsing -emit-pch -o %t.delayed.pch %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -DMAIN_FILE \
+// RUN: -include-pch %t.pch \
+// RUN: -emit-llvm -verify -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -DMAIN_FILE -fdelayed-template-parsing \
+// RUN: -include-pch %t.delayed.pch \
+// RUN: -emit-llvm -verify -o - %s | FileCheck %s
+
+#ifndef MAIN_FILE
+
+extern "C" void consume(int b);
+
+template <int I>
+void function() {
+#pragma pack(push, 1)
+ struct packedAt1 {
+ char a;
+ unsigned long long b;
+ char c;
+ unsigned long long d;
+ // 18 bytes total
+ };
+#pragma pack(push, slot1, 2)
+ struct packedAt2 {
+ char a; // +1 byte of padding
+ unsigned long long b;
+ char c; // +1 byte of padding
+ unsigned long long d;
+ // 20 bytes total
+ };
+#pragma pack(push, 4)
+ struct packedAt4 {
+ char a; // +3 bytes of padding
+ unsigned long long b;
+ char c; // +3 bytes of padding
+ unsigned long long d;
+ // 24 bytes total
+ };
+#pragma pack(push, 16)
+ struct packedAt16 {
+ char a; // +7 bytes of padding
+ unsigned long long b;
+ char c; // +7 bytes of padding
+ unsigned long long d;
+ // 32 bytes total
+ };
+#pragma pack(pop, slot1) // This should return packing to 1 (established before push(slot1))
+ struct packedAfterPopBackTo1 {
+ char a;
+ unsigned long long b;
+ char c;
+ unsigned long long d;
+ };
+#pragma pack(pop)
+
+ consume(sizeof(packedAt1)); // 18
+ consume(sizeof(packedAt2)); // 20
+ consume(sizeof(packedAt4)); // 24
+ consume(sizeof(packedAt16)); // 32
+ consume(sizeof(packedAfterPopBackTo1)); // 18 again
+}
+
+#else
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"??$function@$0A@@@YAXXZ"(
+// CHECK: call void @consume(i32 noundef 18)
+// CHECK-NEXT: call void @consume(i32 noundef 20)
+// CHECK-NEXT: call void @consume(i32 noundef 24)
+// CHECK-NEXT: call void @consume(i32 noundef 32)
+// CHECK-NEXT: call void @consume(i32 noundef 18)
+void foo() {
+ function<0>();
+}
+
+// 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
@@ -120,6 +120,7 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SaveAndRestore.h"
+#include "llvm/Support/StringSaver.h"
#include "llvm/Support/TimeProfiler.h"
#include "llvm/Support/Timer.h"
#include "llvm/Support/VersionTuple.h"
@@ -1672,6 +1673,8 @@
Tok.setFlag((Token::TokenFlags)Record[Idx++]);
if (Tok.isAnnotation()) {
+ llvm::StringSaver PreprocessorStringStorage(PP.getPreprocessorAllocator());
+
Tok.setAnnotationEndLoc(ReadSourceLocation(F, Record, Idx));
switch (Tok.getKind()) {
case tok::annot_pragma_loop_hint: {
@@ -1687,6 +1690,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++]);
+ auto SlotLabel = PreprocessorStringStorage.save(ReadString(Record, Idx));
+ Info->SlotLabel = SlotLabel;
+ 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 +9101,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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
