Page MenuHomePhabricator

[15/N] [libcxx] Implement the canonical function for windows
Needs ReviewPublic

Authored by mstorsjo on Tue, Nov 10, 8:36 AM.

Details

Reviewers
amccarth
EricWF
Group Reviewers
Restricted Project

Diff Detail

Event Timeline

mstorsjo created this revision.Tue, Nov 10, 8:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptTue, Nov 10, 8:36 AM
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald Transcript
mstorsjo updated this revision to Diff 305082.Fri, Nov 13, 3:45 AM
mstorsjo added a reviewer: amccarth.

Rebased on top of updated patches.

amccarth requested changes to this revision.Thu, Dec 3, 2:56 PM
amccarth added inline comments.
libcxx/src/filesystem/operations.cpp
868

This return value needs to be checked more carefully.

If the final path is longer than the buffer, then GetFinalPathNameByHandleW will return the actual length needed, which will convince this if statement that the call succeeded when it didn't, and then we'll process the uninitialized buffer.

How can the final path require a buffer longer than MAX_PATH? Two ways:

  1. The function always returns the \\?\ prefix, so a path that (with it's terminating L'\0') is exactly MAX_PATH will require MAX_PATH+4 when the prefix is added.
  1. MAX_PATH applies to most Windows APIs, but not to the actual filesystem. It's possible to have a final path that's longer than MAX_PATH (especially since it's using the \\?\ prefix). If we don't think it's worth handling those, fine, but we'd need to detect that case so that we can return an error. (A common way this happens is when the user uses subst to replace a long prefix with a fictitious drive letter. This is, in fact, how we get certain LLDB source paths under the MAX_PATH limit on some of the Windows build bots.)
This revision now requires changes to proceed.Thu, Dec 3, 2:56 PM
mstorsjo added inline comments.Fri, Dec 4, 4:05 AM
libcxx/src/filesystem/operations.cpp
868

Thanks, good catch. Will update with a stack buffer that should fit all practical cases (MAX_PATH+10) and fall back on dynamic allocation if that wasn't big enough.

mstorsjo updated this revision to Diff 309509.Fri, Dec 4, 4:07 AM

Check the return value of GetFinalPathNameFromHandleW if it overflowed the buffer size and handle it appropriately.