Skip to content

Commit f0ab281

Browse files
FznamznonAaronBallman
authored andcommitted
[clang] Implement StmtPrinter for EmbedExpr (llvm#135957)
Tries to avoid memory leaks previously caused by saving filename by allocating memory in the preprocessor. Fixes llvm#132641 Fixes llvm#107869 --------- Co-authored-by: Aaron Ballman <[email protected]>
1 parent d735c89 commit f0ab281

File tree

9 files changed

+72
-7
lines changed

9 files changed

+72
-7
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,9 @@ Bug Fixes in This Version
426426
using C++23 "deducing this" did not have a diagnostic location (#GH135522)
427427

428428
- Fixed a crash when a ``friend`` function is redefined as deleted. (#GH135506)
429+
- Fixed a crash when ``#embed`` appears as a part of a failed constant
430+
evaluation. The crashes were happening during diagnostics emission due to
431+
unimplemented statement printer. (#GH132641)
429432

430433
Bug Fixes to Compiler Builtins
431434
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/AST/Expr.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4961,6 +4961,9 @@ class SourceLocExpr final : public Expr {
49614961
/// Stores data related to a single #embed directive.
49624962
struct EmbedDataStorage {
49634963
StringLiteral *BinaryData;
4964+
// FileName string already includes braces, i.e. it is <files/my_file> for a
4965+
// directive #embed <files/my_file>.
4966+
StringRef FileName;
49644967
size_t getDataElementCount() const { return BinaryData->getByteLength(); }
49654968
};
49664969

@@ -5007,6 +5010,7 @@ class EmbedExpr final : public Expr {
50075010
SourceLocation getEndLoc() const { return EmbedKeywordLoc; }
50085011

50095012
StringLiteral *getDataStringLiteral() const { return Data->BinaryData; }
5013+
StringRef getFileName() const { return Data->FileName; }
50105014
EmbedDataStorage *getData() const { return Data; }
50115015

50125016
unsigned getStartingElementPos() const { return Begin; }

clang/include/clang/Lex/Preprocessor.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2762,7 +2762,7 @@ class Preprocessor {
27622762
const FileEntry *LookupFromFile = nullptr);
27632763
void HandleEmbedDirectiveImpl(SourceLocation HashLoc,
27642764
const LexEmbedParametersResult &Params,
2765-
StringRef BinaryContents);
2765+
StringRef BinaryContents, StringRef FileName);
27662766

27672767
// File inclusion.
27682768
void HandleIncludeDirective(SourceLocation HashLoc, Token &Tok,
@@ -3066,6 +3066,7 @@ class EmptylineHandler {
30663066
/// preprocessor to the parser through an annotation token.
30673067
struct EmbedAnnotationData {
30683068
StringRef BinaryData;
3069+
StringRef FileName;
30693070
};
30703071

30713072
/// Registry of pragma handlers added by plugins

clang/include/clang/Sema/Sema.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7256,7 +7256,7 @@ class Sema final : public SemaBase {
72567256

72577257
// #embed
72587258
ExprResult ActOnEmbedExpr(SourceLocation EmbedKeywordLoc,
7259-
StringLiteral *BinaryData);
7259+
StringLiteral *BinaryData, StringRef FileName);
72607260

72617261
// Build a potentially resolved SourceLocExpr.
72627262
ExprResult BuildSourceLocExpr(SourceLocIdentKind Kind, QualType ResultTy,

clang/lib/AST/StmtPrinter.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,11 @@ void StmtPrinter::VisitSourceLocExpr(SourceLocExpr *Node) {
12841284
}
12851285

12861286
void StmtPrinter::VisitEmbedExpr(EmbedExpr *Node) {
1287-
llvm::report_fatal_error("Not implemented");
1287+
// FIXME: Embed parameters are not reflected in the AST, so there is no way to
1288+
// print them yet.
1289+
OS << "#embed ";
1290+
OS << Node->getFileName();
1291+
OS << NL;
12881292
}
12891293

12901294
void StmtPrinter::VisitConstantExpr(ConstantExpr *Node) {

clang/lib/Lex/PPDirectives.cpp

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3909,7 +3909,7 @@ Preprocessor::LexEmbedParameters(Token &CurTok, bool ForHasEmbed) {
39093909

39103910
void Preprocessor::HandleEmbedDirectiveImpl(
39113911
SourceLocation HashLoc, const LexEmbedParametersResult &Params,
3912-
StringRef BinaryContents) {
3912+
StringRef BinaryContents, StringRef FileName) {
39133913
if (BinaryContents.empty()) {
39143914
// If we have no binary contents, the only thing we need to emit are the
39153915
// if_empty tokens, if any.
@@ -3940,6 +3940,7 @@ void Preprocessor::HandleEmbedDirectiveImpl(
39403940

39413941
EmbedAnnotationData *Data = new (BP) EmbedAnnotationData;
39423942
Data->BinaryData = BinaryContents;
3943+
Data->FileName = FileName;
39433944

39443945
Toks[CurIdx].startToken();
39453946
Toks[CurIdx].setKind(tok::annot_embed);
@@ -4049,5 +4050,16 @@ void Preprocessor::HandleEmbedDirective(SourceLocation HashLoc, Token &EmbedTok,
40494050
if (Callbacks)
40504051
Callbacks->EmbedDirective(HashLoc, Filename, isAngled, MaybeFileRef,
40514052
*Params);
4052-
HandleEmbedDirectiveImpl(HashLoc, *Params, BinaryContents);
4053+
// getSpelling() may return a buffer from the token itself or it may use the
4054+
// SmallString buffer we provided. getSpelling() may also return a string that
4055+
// is actually longer than FilenameTok.getLength(), so we first pass a
4056+
// locally created buffer to getSpelling() to get the string of real length
4057+
// and then we allocate a long living buffer because the buffer we used
4058+
// previously will only live till the end of this function and we need
4059+
// filename info to live longer.
4060+
void *Mem = BP.Allocate(OriginalFilename.size(), alignof(char *));
4061+
memcpy(Mem, OriginalFilename.data(), OriginalFilename.size());
4062+
StringRef FilenameToGo =
4063+
StringRef(static_cast<char *>(Mem), OriginalFilename.size());
4064+
HandleEmbedDirectiveImpl(HashLoc, *Params, BinaryContents, FilenameToGo);
40534065
}

clang/lib/Parse/ParseInit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ ExprResult Parser::createEmbedExpr() {
451451

452452
StringLiteral *BinaryDataArg = CreateStringLiteralFromStringRef(
453453
Data->BinaryData, Context.UnsignedCharTy);
454-
Res = Actions.ActOnEmbedExpr(StartLoc, BinaryDataArg);
454+
Res = Actions.ActOnEmbedExpr(StartLoc, BinaryDataArg, Data->FileName);
455455
}
456456
return Res;
457457
}

clang/lib/Sema/SemaExpr.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16903,9 +16903,10 @@ ExprResult Sema::BuildSourceLocExpr(SourceLocIdentKind Kind, QualType ResultTy,
1690316903
}
1690416904

1690516905
ExprResult Sema::ActOnEmbedExpr(SourceLocation EmbedKeywordLoc,
16906-
StringLiteral *BinaryData) {
16906+
StringLiteral *BinaryData, StringRef FileName) {
1690716907
EmbedDataStorage *Data = new (Context) EmbedDataStorage;
1690816908
Data->BinaryData = BinaryData;
16909+
Data->FileName = FileName;
1690916910
return new (Context)
1691016911
EmbedExpr(Context, EmbedKeywordLoc, Data, /*NumOfElements=*/0,
1691116912
Data->getDataElementCount());

clang/test/Preprocessor/embed_weird.cpp

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,43 @@ constexpr struct HasChar c = {
136136
c-error {{constexpr initializer evaluates to 255 which is not exactly representable in type 'signed char'}}
137137

138138
};
139+
140+
#if __cplusplus
141+
namespace std {
142+
typedef decltype(sizeof(int)) size_t;
143+
144+
template <class _E> class initializer_list {
145+
const _E *__begin_;
146+
size_t __size_;
147+
148+
constexpr initializer_list(const _E *__b, size_t __s)
149+
: __begin_(__b), __size_(__s) {}
150+
151+
public:
152+
constexpr initializer_list() : __begin_(nullptr), __size_(0) {}
153+
};
154+
} // namespace std
155+
156+
class S2 {
157+
public:
158+
constexpr S2(std::initializer_list<char>) { // cxx-error {{constexpr constructor never produces a constant expression}}
159+
1/0; // cxx-warning {{division by zero is undefined}}
160+
// cxx-warning@-1 {{unused}}
161+
// cxx-note@-2 4{{division by zero}}
162+
}
163+
};
164+
165+
166+
constexpr S2 s2 { // cxx-error {{must be initialized by a constant expression}}
167+
// cxx-note-re@-1 {{in call to 'S2{{.*}} #embed <jk.txt>}}
168+
#embed <jk.txt> prefix(0x2c, 0x20, )limit(5)
169+
};
170+
constexpr S2 s3 {1, // cxx-error {{must be initialized by a constant expression}}
171+
// cxx-note-re@-1 {{in call to 'S2{{.*}} #embed "jk.txt"}}
172+
#embed "jk.txt"
173+
};
174+
constexpr S2 s4 { // cxx-error {{must be initialized by a constant expression}}
175+
// cxx-note-re@-1 {{in call to 'S2{{.*}}"jk"}}
176+
#embed "jk.txt"
177+
};
178+
#endif

0 commit comments

Comments
 (0)