-
Notifications
You must be signed in to change notification settings - Fork 1.6k
<filesystem>: Fix filesystem::copy() for junctions on x86
#6064
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.
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
Open
StephanTLavavej
wants to merge
4
commits into
microsoft:main
Choose a base branch
from
StephanTLavavej:junk-tion
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+4
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This doesn't appear to impact correctness, but https://learn.microsoft.com/en-us/windows-hardware/drivers/ifs/fsctl-get-reparse-point says: "Must be at least REPARSE_DATA_BUFFER_HEADER_SIZE plus the size of the expected user-defined data; and must be less than or equal to MAXIMUM_REPARSE_DATA_BUFFER_SIZE." Adding `sizeof(wchar_t)` appears to be totally unjustified and unnecessary.
https://learn.microsoft.com/en-us/windows-hardware/drivers/ifs/fsctl-set-reparse-point "For a REPARSE_DATA_BUFFER structure, this value must be at least REPARSE_DATA_BUFFER_HEADER_SIZE, plus the size of the user-defined data, and it must be less than or equal to MAXIMUM_REPARSE_DATA_BUFFER_SIZE." Annoyingly, this constant appears in the WDK, but not the normal WinSDK. However, it can easily be computed, and visually verified against our structure definition. The correct constant is always 8 regardless of architecture (I checked x64 and x86). Because the totally bogus `sizeof(_Buffer)` was measuring the size of the pointer `const __std_fs_reparse_data_buffer* const _Buffer`, this bug was cloaked on x64 and ARM64 with their 8-byte pointers. This bug reproed only for x86 with its 4-byte pointers.
AlexGuteniev
reviewed
Feb 2, 2026
davidmrdavid
approved these changes
Feb 5, 2026
Member
davidmrdavid
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, learned about 'junctions' 🤓
Member
Author
|
I'm mirroring this to the MSVC-internal repo. Please notify me if any further changes are pushed, otherwise no action is required. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the correct fix for the bug identified by #5976. Thanks @SiliconA-Z for finding this, @Morddin for identifying the correct constant to use (which I double-checked), and Fulgen on the STL Discord for providing the PowerShell incantation to create a junction.
Commits
_Read_reparse_data()allocates.FSCTL_GET_REPARSE_POINTcontrol code in the Windows Drivers documentation says:sizeof(wchar_t)appears to be totally unjustified and unnecessary.__std_fs_write_reparse_data_buffer().FSCTL_SET_REPARSE_POINTcontrol code in the Windows Drivers documentation says:filesystem.cpp:__std_fs_write_reparse_data_buffer()should use the size of the buffer, not buffer pointer #5976 (comment)). However, it can easily be computed and visually verified against our structure definition. The correct constant is always 8 regardless of architecture (I checked x64 and x86).sizeof(_Buffer)was measuring the size of the pointerconst __std_fs_reparse_data_buffer* const _Buffer, this bug was cloaked on x64 and ARM64 with their 8-byte pointers. This bug reproed only for x86 with its 4-byte pointers.filesystem.cpp.<xfilesystem_abi.h>where it was visible to users.Repro
Notes
I've manually verified that the fix makes both x64 and x86 succeed.
Because junctions are weird and we aren't frequently changing this code, I'm not updating our automated test coverage for this.