Skip to content

[Remarks] YAMLRemarkSerializer: StringRef-ize apis to avoid out-of-bounds read footguns #160397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 24, 2025
Merged

[Remarks] YAMLRemarkSerializer: StringRef-ize apis to avoid out-of-bounds read footguns #160397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 26 additions & 27 deletions llvm/include/llvm/Support/YAMLTraits.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,20 +705,20 @@ class LLVM_ABI IO {
virtual bool mapTag(StringRef Tag, bool Default = false) = 0;
virtual void beginMapping() = 0;
virtual void endMapping() = 0;
virtual bool preflightKey(const char *, bool, bool, bool &, void *&) = 0;
virtual bool preflightKey(StringRef, bool, bool, bool &, void *&) = 0;
virtual void postflightKey(void *) = 0;
virtual std::vector<StringRef> keys() = 0;

virtual void beginFlowMapping() = 0;
virtual void endFlowMapping() = 0;

virtual void beginEnumScalar() = 0;
virtual bool matchEnumScalar(const char *, bool) = 0;
virtual bool matchEnumScalar(StringRef, bool) = 0;
virtual bool matchEnumFallback() = 0;
virtual void endEnumScalar() = 0;

virtual bool beginBitSetScalar(bool &) = 0;
virtual bool bitSetMatch(const char *, bool) = 0;
virtual bool bitSetMatch(StringRef, bool) = 0;
virtual void endBitSetScalar() = 0;

virtual void scalarString(StringRef &, QuotingType) = 0;
Expand All @@ -731,16 +731,15 @@ class LLVM_ABI IO {
virtual std::error_code error() = 0;
virtual void setAllowUnknownKeys(bool Allow);

template <typename T>
void enumCase(T &Val, const char *Str, const T ConstVal) {
template <typename T> void enumCase(T &Val, StringRef Str, const T ConstVal) {
if (matchEnumScalar(Str, outputting() && Val == ConstVal)) {
Val = ConstVal;
}
}

// allow anonymous enum values to be used with LLVM_YAML_STRONG_TYPEDEF
template <typename T>
void enumCase(T &Val, const char *Str, const uint32_t ConstVal) {
void enumCase(T &Val, StringRef Str, const uint32_t ConstVal) {
if (matchEnumScalar(Str, outputting() && Val == static_cast<T>(ConstVal))) {
Val = ConstVal;
}
Expand All @@ -757,28 +756,28 @@ class LLVM_ABI IO {
}

template <typename T>
void bitSetCase(T &Val, const char *Str, const T ConstVal) {
void bitSetCase(T &Val, StringRef Str, const T ConstVal) {
if (bitSetMatch(Str, outputting() && (Val & ConstVal) == ConstVal)) {
Val = static_cast<T>(Val | ConstVal);
}
}

// allow anonymous enum values to be used with LLVM_YAML_STRONG_TYPEDEF
template <typename T>
void bitSetCase(T &Val, const char *Str, const uint32_t ConstVal) {
void bitSetCase(T &Val, StringRef Str, const uint32_t ConstVal) {
if (bitSetMatch(Str, outputting() && (Val & ConstVal) == ConstVal)) {
Val = static_cast<T>(Val | ConstVal);
}
}

template <typename T>
void maskedBitSetCase(T &Val, const char *Str, T ConstVal, T Mask) {
void maskedBitSetCase(T &Val, StringRef Str, T ConstVal, T Mask) {
if (bitSetMatch(Str, outputting() && (Val & Mask) == ConstVal))
Val = Val | ConstVal;
}

template <typename T>
void maskedBitSetCase(T &Val, const char *Str, uint32_t ConstVal,
void maskedBitSetCase(T &Val, StringRef Str, uint32_t ConstVal,
uint32_t Mask) {
if (bitSetMatch(Str, outputting() && (Val & Mask) == ConstVal))
Val = Val | ConstVal;
Expand All @@ -787,29 +786,29 @@ class LLVM_ABI IO {
void *getContext() const;
void setContext(void *);

template <typename T> void mapRequired(const char *Key, T &Val) {
template <typename T> void mapRequired(StringRef Key, T &Val) {
EmptyContext Ctx;
this->processKey(Key, Val, true, Ctx);
}

template <typename T, typename Context>
void mapRequired(const char *Key, T &Val, Context &Ctx) {
void mapRequired(StringRef Key, T &Val, Context &Ctx) {
this->processKey(Key, Val, true, Ctx);
}

template <typename T> void mapOptional(const char *Key, T &Val) {
template <typename T> void mapOptional(StringRef Key, T &Val) {
EmptyContext Ctx;
mapOptionalWithContext(Key, Val, Ctx);
}

template <typename T, typename DefaultT>
void mapOptional(const char *Key, T &Val, const DefaultT &Default) {
void mapOptional(StringRef Key, T &Val, const DefaultT &Default) {
EmptyContext Ctx;
mapOptionalWithContext(Key, Val, Default, Ctx);
}

template <typename T, typename Context>
void mapOptionalWithContext(const char *Key, T &Val, Context &Ctx) {
void mapOptionalWithContext(StringRef Key, T &Val, Context &Ctx) {
if constexpr (has_SequenceTraits<T>::value) {
// omit key/value instead of outputting empty sequence
if (this->canElideEmptySequence() && Val.begin() == Val.end())
Expand All @@ -819,14 +818,14 @@ class LLVM_ABI IO {
}

template <typename T, typename Context>
void mapOptionalWithContext(const char *Key, std::optional<T> &Val,
void mapOptionalWithContext(StringRef Key, std::optional<T> &Val,
Context &Ctx) {
this->processKeyWithDefault(Key, Val, std::optional<T>(),
/*Required=*/false, Ctx);
}

template <typename T, typename Context, typename DefaultT>
void mapOptionalWithContext(const char *Key, T &Val, const DefaultT &Default,
void mapOptionalWithContext(StringRef Key, T &Val, const DefaultT &Default,
Context &Ctx) {
static_assert(std::is_convertible<DefaultT, T>::value,
"Default type must be implicitly convertible to value type!");
Expand All @@ -836,12 +835,12 @@ class LLVM_ABI IO {

private:
template <typename T, typename Context>
void processKeyWithDefault(const char *Key, std::optional<T> &Val,
void processKeyWithDefault(StringRef Key, std::optional<T> &Val,
const std::optional<T> &DefaultValue,
bool Required, Context &Ctx);

template <typename T, typename Context>
void processKeyWithDefault(const char *Key, T &Val, const T &DefaultValue,
void processKeyWithDefault(StringRef Key, T &Val, const T &DefaultValue,
bool Required, Context &Ctx) {
void *SaveInfo;
bool UseDefault;
Expand All @@ -857,7 +856,7 @@ class LLVM_ABI IO {
}

template <typename T, typename Context>
void processKey(const char *Key, T &Val, bool Required, Context &Ctx) {
void processKey(StringRef Key, T &Val, bool Required, Context &Ctx) {
void *SaveInfo;
bool UseDefault;
if (this->preflightKey(Key, Required, false, UseDefault, SaveInfo)) {
Expand Down Expand Up @@ -1332,7 +1331,7 @@ class LLVM_ABI Input : public IO {
bool mapTag(StringRef, bool) override;
void beginMapping() override;
void endMapping() override;
bool preflightKey(const char *, bool, bool, bool &, void *&) override;
bool preflightKey(StringRef Key, bool, bool, bool &, void *&) override;
void postflightKey(void *) override;
std::vector<StringRef> keys() override;
void beginFlowMapping() override;
Expand All @@ -1346,11 +1345,11 @@ class LLVM_ABI Input : public IO {
void postflightFlowElement(void *) override;
void endFlowSequence() override;
void beginEnumScalar() override;
bool matchEnumScalar(const char *, bool) override;
bool matchEnumScalar(StringRef, bool) override;
bool matchEnumFallback() override;
void endEnumScalar() override;
bool beginBitSetScalar(bool &) override;
bool bitSetMatch(const char *, bool) override;
bool bitSetMatch(StringRef, bool) override;
void endBitSetScalar() override;
void scalarString(StringRef &, QuotingType) override;
void blockScalarString(StringRef &) override;
Expand Down Expand Up @@ -1483,7 +1482,7 @@ class LLVM_ABI Output : public IO {
bool mapTag(StringRef, bool) override;
void beginMapping() override;
void endMapping() override;
bool preflightKey(const char *key, bool, bool, bool &, void *&) override;
bool preflightKey(StringRef Key, bool, bool, bool &, void *&) override;
void postflightKey(void *) override;
std::vector<StringRef> keys() override;
void beginFlowMapping() override;
Expand All @@ -1497,11 +1496,11 @@ class LLVM_ABI Output : public IO {
void postflightFlowElement(void *) override;
void endFlowSequence() override;
void beginEnumScalar() override;
bool matchEnumScalar(const char *, bool) override;
bool matchEnumScalar(StringRef, bool) override;
bool matchEnumFallback() override;
void endEnumScalar() override;
bool beginBitSetScalar(bool &) override;
bool bitSetMatch(const char *, bool) override;
bool bitSetMatch(StringRef, bool) override;
void endBitSetScalar() override;
void scalarString(StringRef &, QuotingType) override;
void blockScalarString(StringRef &) override;
Expand Down Expand Up @@ -1558,7 +1557,7 @@ class LLVM_ABI Output : public IO {
};

template <typename T, typename Context>
void IO::processKeyWithDefault(const char *Key, std::optional<T> &Val,
void IO::processKeyWithDefault(StringRef Key, std::optional<T> &Val,
const std::optional<T> &DefaultValue,
bool Required, Context &Ctx) {
assert(!DefaultValue && "std::optional<T> shouldn't have a value!");
Expand Down
10 changes: 4 additions & 6 deletions llvm/lib/Remarks/YAMLRemarkSerializer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,13 @@ template <> struct MappingTraits<Argument> {
static void mapping(IO &io, Argument &A) {
assert(io.outputting() && "input not yet implemented");

// A.Key.data() is not necessarily null-terminated, so we must make a copy,
// otherwise we potentially read out of bounds.
// FIXME: Add support for StringRef Keys in YAML IO.
std::string Key(A.Key);
// NB: A.Key.data() is not necessarily null-terminated, as the StringRef may
// be a span into the middle of a string.
if (StringRef(A.Val).count('\n') > 1) {
StringBlockVal S(A.Val);
io.mapRequired(Key.c_str(), S);
io.mapRequired(A.Key, S);
} else {
io.mapRequired(Key.c_str(), A.Val);
io.mapRequired(A.Key, A.Val);
}
io.mapOptional("DebugLoc", A.Loc);
}
Expand Down
14 changes: 7 additions & 7 deletions llvm/lib/Support/YAMLTraits.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ std::vector<StringRef> Input::keys() {
return Ret;
}

bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault,
bool Input::preflightKey(StringRef Key, bool Required, bool, bool &UseDefault,
void *&SaveInfo) {
UseDefault = false;
if (EC)
Expand All @@ -168,7 +168,7 @@ bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault,
UseDefault = true;
return false;
}
MN->ValidKeys.push_back(Key);
MN->ValidKeys.push_back(Key.str());
HNode *Value = MN->Mapping[Key].first;
if (!Value) {
if (Required)
Expand Down Expand Up @@ -266,7 +266,7 @@ void Input::beginEnumScalar() {
ScalarMatchFound = false;
}

bool Input::matchEnumScalar(const char *Str, bool) {
bool Input::matchEnumScalar(StringRef Str, bool) {
if (ScalarMatchFound)
return false;
if (ScalarHNode *SN = dyn_cast<ScalarHNode>(CurrentNode)) {
Expand Down Expand Up @@ -302,7 +302,7 @@ bool Input::beginBitSetScalar(bool &DoClear) {
return true;
}

bool Input::bitSetMatch(const char *Str, bool) {
bool Input::bitSetMatch(StringRef Str, bool) {
if (EC)
return false;
if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
Expand Down Expand Up @@ -541,7 +541,7 @@ std::vector<StringRef> Output::keys() {
report_fatal_error("invalid call");
}

bool Output::preflightKey(const char *Key, bool Required, bool SameAsDefault,
bool Output::preflightKey(StringRef Key, bool Required, bool SameAsDefault,
bool &UseDefault, void *&SaveInfo) {
UseDefault = false;
SaveInfo = nullptr;
Expand Down Expand Up @@ -666,7 +666,7 @@ void Output::beginEnumScalar() {
EnumerationMatchFound = false;
}

bool Output::matchEnumScalar(const char *Str, bool Match) {
bool Output::matchEnumScalar(StringRef Str, bool Match) {
if (Match && !EnumerationMatchFound) {
newLineCheck();
outputUpToEndOfLine(Str);
Expand Down Expand Up @@ -695,7 +695,7 @@ bool Output::beginBitSetScalar(bool &DoClear) {
return true;
}

bool Output::bitSetMatch(const char *Str, bool Matches) {
bool Output::bitSetMatch(StringRef Str, bool Matches) {
if (Matches) {
if (NeedBitValueComma)
output(", ");
Expand Down
Loading