<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 9, 2017 at 3:56 PM Bob Haarman via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">inglorion created this revision.<br>
Herald added a subscriber: hiraditya.<br>
<br>
zturner suggested that mapped_file_region::init() on Windows seems to<br>
create mappings that are larger than they need to be: Offset+Size<br>
instead of Size. Indeed, that appears to be the case. I confirmed that<br>
tests pass with mappings of just Size bytes, and fail with Size-1<br>
bytes, suggesting that Size is indeed the correct value.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D39876" rel="noreferrer" target="_blank">https://reviews.llvm.org/D39876</a><br>
<br>
Files:<br>
  llvm/lib/Support/Windows/Path.inc<br>
<br>
<br>
Index: llvm/lib/Support/Windows/Path.inc<br>
===================================================================<br>
--- llvm/lib/Support/Windows/Path.inc<br>
+++ llvm/lib/Support/Windows/Path.inc<br>
@@ -734,8 +734,8 @@<br>
<br>
   HANDLE FileMappingHandle =<br>
       ::CreateFileMappingW(FileHandle, 0, flprotect,<br>
-                           (Offset + Size) >> 32,<br>
-                           (Offset + Size) & 0xffffffff,<br>
+                           Size >> 32,<br>
+                           Size & 0xffffffff,</blockquote><div><br></div><div>Oops!  Turns out this is undefined behavior when building x86 where size_t is 32-bits, and you're shifting right by 32.  I think we can replace this with Hi_32 and Lo_32 instead.  I'll fix it, just wanted to point it out.</div></div></div>