Skip to content

Conversation

tobias-stadler
Copy link
Contributor

YAML IO mapRequired expects a null-terminated const char * Key, so
we can't legally pass a StringRef to it. We should add StringRef Key
support to YAML IO, but for now just copy the key into a correctly
null-terminated string.

Created using spr 1.3.7-wip

[skip ci]
Created using spr 1.3.7-wip
Copy link
Contributor

@fhahn fhahn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Copy link
Contributor

@jroelofs jroelofs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tricky. LGTM.

@jroelofs
Copy link
Contributor

Version of this that switches everything over to StringRef: #160397

Created using spr 1.3.7-wip

[skip ci]
Created using spr 1.3.7-wip
@tobias-stadler tobias-stadler changed the base branch from users/tobias-stadler/spr/main.remarks-yamlremarkserializer-fix-stringref-out-of-bounds-read to main September 23, 2025 23:12
@tobias-stadler tobias-stadler merged commit 3ddb549 into main Sep 23, 2025
12 of 15 checks passed
@tobias-stadler tobias-stadler deleted the users/tobias-stadler/spr/remarks-yamlremarkserializer-fix-stringref-out-of-bounds-read branch September 23, 2025 23:12
@tobias-stadler
Copy link
Contributor Author

Merged for the test. Proper fix by #160397. Thanks!

llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Sep 23, 2025
…s read (#159759)

YAML IO `mapRequired` expects a null-terminated `const char *` Key, so
we can't legally pass a StringRef to it. We should add StringRef Key
support to YAML IO, but for now just copy the key into a correctly
null-terminated string.

Pull Request: llvm/llvm-project#159759
jroelofs added a commit that referenced this pull request Sep 24, 2025
…unds read footguns (#160397)

In #159759, Tobias identified that because YAML IO `mapRequired`
expected a null-terminated `const char * Key`, we couldn't legally pass
a `StringRef` to it, as that might be length-terminated and not
null-terminated. In this patch, we move all of the YAML IO functions
that accept a `const char *` over to `StringRef`, avoiding that footgun
altogether.
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…unds read footguns (llvm#160397)

In llvm#159759, Tobias identified that because YAML IO `mapRequired`
expected a null-terminated `const char * Key`, we couldn't legally pass
a `StringRef` to it, as that might be length-terminated and not
null-terminated. In this patch, we move all of the YAML IO functions
that accept a `const char *` over to `StringRef`, avoiding that footgun
altogether.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants