Skip to content

Conversation

@al45tair
Copy link
Contributor

@al45tair al45tair commented Nov 3, 2023

Thanks to a missing - sign, we were returning garbage rather than indicating that the memory region in question was inaccessible. This mainly affects register dumps (since that's the time we expect to have to cope with out-of-bounds reads).

rdar://117900760

Thanks to a missing `-` sign, we were returning garbage rather than
indicating that the memory region in question was inaccessible.  This
mainly affects register dumps (since that's the time we expect to have
to cope with out-of-bounds reads).

rdar://117900760
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.9 labels Nov 3, 2023
@al45tair al45tair requested a review from a team as a code owner November 3, 2023 11:19
@al45tair
Copy link
Contributor Author

al45tair commented Nov 3, 2023

Explanation: The memserver_read() function is supposed to return negative numbers on failure. For normal execution on Linux, it uses process_vm_readv when possible because that is faster and doesn't require using signal handlers to trap failed reads, but when we can't do that (for instance in Docker), it falls back on a memcpy() based alternative. Unfortunately it looks like I fat-fingered the return code when it fails and returned 1 instead of -1, with the result that the memory server would return garbage instead of indicating that the read failed.
Risk: Low. Only really affects register dump output since that's the only time we're really relying on being able to attempt reads to random addresses.
Original PR: #69635
Reviewed by: @tomerd
Resolves: rdar://117900760
Tests: There isn't really a test in the repo for this specific issue.

@al45tair
Copy link
Contributor Author

al45tair commented Nov 3, 2023

@swift-ci Please test

@al45tair
Copy link
Contributor Author

al45tair commented Nov 3, 2023

@swift-ci Please test Linux platform

@al45tair al45tair merged commit fc38a5d into swiftlang:release/5.9 Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🍒 release cherry pick Flag: Release branch cherry picks swift 5.9

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants