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 cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits