[PATCH] D38891: Implement part of the flock functionality for Windows in compiler-rt

Zachary Turner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 13 09:43:51 PDT 2017


zturner edited subscribers, added: llvm-commits, zturner; removed: dberris.
zturner added inline comments.


================
Comment at: lib/profile/WindowsMMap.c:131-132
+    OVERLAPPED overlapped = { 0 };
+    if (!LockFileEx(handle, LOCKFILE_EXCLUSIVE_LOCK, 0, MAXDWORD, MAXDWORD,
+                    &overlapped))
+      return -1;
----------------
There is a subtle incompatibility here.  If `fd` was originally opened for asynchronous IO, then this function will return immediately.  This is basically the same as if you had called `flock` with `LOCK_NB`, but the difference is that with `flock`, the caller has to opt into it, where as with `LockFileEx`, it's an inherent property of the file descriptor.

Actually, it's a bit more than just a semantic difference.  Since the function will return immediately, the OS will be holding onto destroyed stack memory (i.e. the `OVERLAPPED` structure).

I don't know if you want to handle this, but you should at least be aware, and perhaps leave a comment.


================
Comment at: lib/profile/WindowsMMap.c:146
+  default:
+    return -1; /* Not supported. */
+  }
----------------
I know you're only implementing part of the functionality, but why not all?  Seems easy enough to handle `LOCK_SH`, just change `LOCKFILE_EXCLUSIVE_LOCK` to `LOCKFILE_SHARED_LOCK` and the code is the same.


https://reviews.llvm.org/D38891





More information about the llvm-commits mailing list