https://github.com/Fznamznon updated https://github.com/llvm/llvm-project/pull/135957
>From b194133f44768a61a7af6486dc40deae2de73f9a Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <mariya.podchishcha...@intel.com> Date: Wed, 16 Apr 2025 05:59:24 -0700 Subject: [PATCH 1/2] [clang] Implement StmtPrinter for EmbedExpr Tries to avoid memory leaks caused by saving filename earlier by allocating memory in the preprocessor. Fixes https://github.com/llvm/llvm-project/issues/132641 --- clang/include/clang/AST/Expr.h | 4 +++ clang/include/clang/Lex/Preprocessor.h | 3 +- clang/include/clang/Sema/Sema.h | 2 +- clang/lib/AST/StmtPrinter.cpp | 6 +++- clang/lib/Lex/PPDirectives.cpp | 13 ++++++-- clang/lib/Parse/ParseInit.cpp | 3 +- clang/lib/Sema/SemaExpr.cpp | 4 ++- clang/test/Preprocessor/embed_weird.cpp | 40 +++++++++++++++++++++++++ 8 files changed, 68 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h index 529c6228bfa19..a83320a7ddec2 100644 --- a/clang/include/clang/AST/Expr.h +++ b/clang/include/clang/AST/Expr.h @@ -4961,6 +4961,9 @@ class SourceLocExpr final : public Expr { /// Stores data related to a single #embed directive. struct EmbedDataStorage { StringLiteral *BinaryData; + // FileName string already includes braces, i.e. it is <files/my_file> for a + // directive #embed <files/my_file>. + StringRef FileName; size_t getDataElementCount() const { return BinaryData->getByteLength(); } }; @@ -5007,6 +5010,7 @@ class EmbedExpr final : public Expr { SourceLocation getEndLoc() const { return EmbedKeywordLoc; } StringLiteral *getDataStringLiteral() const { return Data->BinaryData; } + StringRef getFileName() const { return Data->FileName; } EmbedDataStorage *getData() const { return Data; } unsigned getStartingElementPos() const { return Begin; } diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h index f8f2f567f9171..10260c61bdf11 100644 --- a/clang/include/clang/Lex/Preprocessor.h +++ b/clang/include/clang/Lex/Preprocessor.h @@ -2761,7 +2761,7 @@ class Preprocessor { const FileEntry *LookupFromFile = nullptr); void HandleEmbedDirectiveImpl(SourceLocation HashLoc, const LexEmbedParametersResult &Params, - StringRef BinaryContents); + StringRef BinaryContents, StringRef FileName); // File inclusion. void HandleIncludeDirective(SourceLocation HashLoc, Token &Tok, @@ -3065,6 +3065,7 @@ class EmptylineHandler { /// preprocessor to the parser through an annotation token. struct EmbedAnnotationData { StringRef BinaryData; + StringRef FileName; }; /// Registry of pragma handlers added by plugins diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 1f23b754a69cb..a757f4c6430ae 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7256,7 +7256,7 @@ class Sema final : public SemaBase { // #embed ExprResult ActOnEmbedExpr(SourceLocation EmbedKeywordLoc, - StringLiteral *BinaryData); + StringLiteral *BinaryData, StringRef FileName); // Build a potentially resolved SourceLocExpr. ExprResult BuildSourceLocExpr(SourceLocIdentKind Kind, QualType ResultTy, diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp index aae10fd3bd885..2993d7d0ee865 100644 --- a/clang/lib/AST/StmtPrinter.cpp +++ b/clang/lib/AST/StmtPrinter.cpp @@ -1284,7 +1284,11 @@ void StmtPrinter::VisitSourceLocExpr(SourceLocExpr *Node) { } void StmtPrinter::VisitEmbedExpr(EmbedExpr *Node) { - llvm::report_fatal_error("Not implemented"); + // Embed parameters are not reflected in the AST, so there is no way to print + // them yet. + OS << "#embed "; + OS << Node->getFileName(); + OS << NL; } void StmtPrinter::VisitConstantExpr(ConstantExpr *Node) { diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 21ec83b437ef4..cd90dfd885007 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -3909,7 +3909,7 @@ Preprocessor::LexEmbedParameters(Token &CurTok, bool ForHasEmbed) { void Preprocessor::HandleEmbedDirectiveImpl( SourceLocation HashLoc, const LexEmbedParametersResult &Params, - StringRef BinaryContents) { + StringRef BinaryContents, StringRef FileName) { if (BinaryContents.empty()) { // If we have no binary contents, the only thing we need to emit are the // if_empty tokens, if any. @@ -3940,6 +3940,7 @@ void Preprocessor::HandleEmbedDirectiveImpl( EmbedAnnotationData *Data = new (BP) EmbedAnnotationData; Data->BinaryData = BinaryContents; + Data->FileName = FileName; Toks[CurIdx].startToken(); Toks[CurIdx].setKind(tok::annot_embed); @@ -4049,5 +4050,13 @@ void Preprocessor::HandleEmbedDirective(SourceLocation HashLoc, Token &EmbedTok, if (Callbacks) Callbacks->EmbedDirective(HashLoc, Filename, isAngled, MaybeFileRef, *Params); - HandleEmbedDirectiveImpl(HashLoc, *Params, BinaryContents); + // getSpelling may return a string that is actually longer than + // FilenameTok.getLength(), so get the string of real length in + // OriginalFilename and then allocate in the preprocessor to make the memory + // live longer since we need filename to pretty print AST nodes later. + void *Mem = BP.Allocate(OriginalFilename.size(), alignof(char *)); + memcpy(Mem, OriginalFilename.data(), OriginalFilename.size()); + StringRef FilenameToGo = + StringRef(static_cast<char *>(Mem), OriginalFilename.size()); + HandleEmbedDirectiveImpl(HashLoc, *Params, BinaryContents, FilenameToGo); } diff --git a/clang/lib/Parse/ParseInit.cpp b/clang/lib/Parse/ParseInit.cpp index 471b3eaf28287..ca2314419e5e4 100644 --- a/clang/lib/Parse/ParseInit.cpp +++ b/clang/lib/Parse/ParseInit.cpp @@ -435,6 +435,7 @@ ExprResult Parser::createEmbedExpr() { ExprResult Res; ASTContext &Context = Actions.getASTContext(); SourceLocation StartLoc = ConsumeAnnotationToken(); + if (Data->BinaryData.size() == 1) { Res = IntegerLiteral::Create( Context, llvm::APInt(CHAR_BIT, (unsigned char)Data->BinaryData.back()), @@ -451,7 +452,7 @@ ExprResult Parser::createEmbedExpr() { StringLiteral *BinaryDataArg = CreateStringLiteralFromStringRef( Data->BinaryData, Context.UnsignedCharTy); - Res = Actions.ActOnEmbedExpr(StartLoc, BinaryDataArg); + Res = Actions.ActOnEmbedExpr(StartLoc, BinaryDataArg, Data->FileName); } return Res; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 6830bb5c01c7d..ae7b3feddd245 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -16903,9 +16903,11 @@ ExprResult Sema::BuildSourceLocExpr(SourceLocIdentKind Kind, QualType ResultTy, } ExprResult Sema::ActOnEmbedExpr(SourceLocation EmbedKeywordLoc, - StringLiteral *BinaryData) { + StringLiteral *BinaryData, + StringRef FileName) { EmbedDataStorage *Data = new (Context) EmbedDataStorage; Data->BinaryData = BinaryData; + Data->FileName = FileName; return new (Context) EmbedExpr(Context, EmbedKeywordLoc, Data, /*NumOfElements=*/0, Data->getDataElementCount()); diff --git a/clang/test/Preprocessor/embed_weird.cpp b/clang/test/Preprocessor/embed_weird.cpp index 9a984e40d4aa2..e44aff55c59e2 100644 --- a/clang/test/Preprocessor/embed_weird.cpp +++ b/clang/test/Preprocessor/embed_weird.cpp @@ -136,3 +136,43 @@ constexpr struct HasChar c = { c-error {{constexpr initializer evaluates to 255 which is not exactly representable in type 'signed char'}} }; + +#if __cplusplus +namespace std { +typedef decltype(sizeof(int)) size_t; + +template <class _E> class initializer_list { + const _E *__begin_; + size_t __size_; + + constexpr initializer_list(const _E *__b, size_t __s) + : __begin_(__b), __size_(__s) {} + +public: + constexpr initializer_list() : __begin_(nullptr), __size_(0) {} +}; +} // namespace std + +class S2 { +public: + constexpr S2(std::initializer_list<char>) { // cxx-error {{constexpr constructor never produces a constant expression}} + 1/0; // cxx-warning {{division by zero is undefined}} + // cxx-warning@-1 {{unused}} + // cxx-note@-2 4{{division by zero}} + } +}; + + +constexpr S2 s2 { // cxx-error {{must be initialized by a constant expression}} + // cxx-note-re@-1 {{in call to 'S2{{.*}} #embed <jk.txt>}} +#embed <jk.txt> prefix(0x2c, 0x20, )limit(5) +}; +constexpr S2 s3 {1, // cxx-error {{must be initialized by a constant expression}} + // cxx-note-re@-1 {{in call to 'S2{{.*}} #embed "jk.txt"}} +#embed "jk.txt" +}; +constexpr S2 s4 { // cxx-error {{must be initialized by a constant expression}} + // cxx-note-re@-1 {{in call to 'S2{{.*}}"jk"}} +#embed "jk.txt" +}; +#endif >From fc0473def6db3eb602aec193a3025fad354fd6bd Mon Sep 17 00:00:00 2001 From: Mariya Podchishchaeva <mariya.podchishcha...@intel.com> Date: Wed, 16 Apr 2025 16:59:15 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Aaron Ballman <aa...@aaronballman.com> --- clang/lib/AST/StmtPrinter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp index 2993d7d0ee865..447fea69eb120 100644 --- a/clang/lib/AST/StmtPrinter.cpp +++ b/clang/lib/AST/StmtPrinter.cpp @@ -1284,7 +1284,7 @@ void StmtPrinter::VisitSourceLocExpr(SourceLocExpr *Node) { } void StmtPrinter::VisitEmbedExpr(EmbedExpr *Node) { - // Embed parameters are not reflected in the AST, so there is no way to print + // FIXME: Embed parameters are not reflected in the AST, so there is no way to print // them yet. OS << "#embed "; OS << Node->getFileName(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
