[PATCH] D58513: [libFuzzer][Windows] Port fork mode to Windows

[Jonathan Metzman via Phabricator via llvm-commits]

metzman marked 2 inline comments as done.
metzman added inline comments.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerIOWindows.cpp:82-83
+  // is very slow on Windows.
+  return (FileAttributes &
+          (FILE_ATTRIBUTE_REPARSE_POINT | FILE_ATTRIBUTE_DIRECTORY)) == 0;
+}
----------------
zturner wrote:
> I mentioned this elsewhere, but reparse point does not imply that the target of the reparse point is a file.  It could be a directory junction, or something else entirely.  Here's all the known reparse tags:
> 
> ```
> #define IO_REPARSE_TAG_MOUNT_POINT              (0xA0000003L)       
> #define IO_REPARSE_TAG_HSM                      (0xC0000004L)       
> #define IO_REPARSE_TAG_HSM2                     (0x80000006L)       
> #define IO_REPARSE_TAG_SIS                      (0x80000007L)       
> #define IO_REPARSE_TAG_WIM                      (0x80000008L)       
> #define IO_REPARSE_TAG_CSV                      (0x80000009L)       
> #define IO_REPARSE_TAG_DFS                      (0x8000000AL)       
> #define IO_REPARSE_TAG_SYMLINK                  (0xA000000CL)       
> #define IO_REPARSE_TAG_DFSR                     (0x80000012L)       
> #define IO_REPARSE_TAG_DEDUP                    (0x80000013L)       
> #define IO_REPARSE_TAG_NFS                      (0x80000014L)       
> #define IO_REPARSE_TAG_FILE_PLACEHOLDER         (0x80000015L)       
> #define IO_REPARSE_TAG_WOF                      (0x80000017L)       
> #define IO_REPARSE_TAG_WCI                      (0x80000018L)       
> #define IO_REPARSE_TAG_WCI_1                    (0x90001018L)       
> #define IO_REPARSE_TAG_GLOBAL_REPARSE           (0xA0000019L)       
> #define IO_REPARSE_TAG_CLOUD                    (0x9000001AL)       
> #define IO_REPARSE_TAG_CLOUD_1                  (0x9000101AL)       
> #define IO_REPARSE_TAG_CLOUD_2                  (0x9000201AL)       
> #define IO_REPARSE_TAG_CLOUD_3                  (0x9000301AL)       
> #define IO_REPARSE_TAG_CLOUD_4                  (0x9000401AL)       
> #define IO_REPARSE_TAG_CLOUD_5                  (0x9000501AL)       
> #define IO_REPARSE_TAG_CLOUD_6                  (0x9000601AL)       
> #define IO_REPARSE_TAG_CLOUD_7                  (0x9000701AL)       
> #define IO_REPARSE_TAG_CLOUD_8                  (0x9000801AL)       
> #define IO_REPARSE_TAG_CLOUD_9                  (0x9000901AL)       
> #define IO_REPARSE_TAG_CLOUD_A                  (0x9000A01AL)       
> #define IO_REPARSE_TAG_CLOUD_B                  (0x9000B01AL)       
> #define IO_REPARSE_TAG_CLOUD_C                  (0x9000C01AL)       
> #define IO_REPARSE_TAG_CLOUD_D                  (0x9000D01AL)       
> #define IO_REPARSE_TAG_CLOUD_E                  (0x9000E01AL)       
> #define IO_REPARSE_TAG_CLOUD_F                  (0x9000F01AL)       
> #define IO_REPARSE_TAG_CLOUD_MASK               (0x0000F000L)       
> #define IO_REPARSE_TAG_APPEXECLINK              (0x8000001BL)       
> #define IO_REPARSE_TAG_PROJFS                   (0x9000001CL)       
> #define IO_REPARSE_TAG_STORAGE_SYNC             (0x8000001EL)       
> #define IO_REPARSE_TAG_WCI_TOMBSTONE            (0xA000001FL)       
> #define IO_REPARSE_TAG_UNHANDLED                (0x80000020L)       
> #define IO_REPARSE_TAG_ONEDRIVE                 (0x80000021L)       
> #define IO_REPARSE_TAG_PROJFS_TOMBSTONE         (0xA0000022L)       
> #define IO_REPARSE_TAG_AF_UNIX                  (0x80000023L)       
> 
> ```
See my comment below, I think I need to do this to handle the `edge-case` example.

Separately, I'm wondering if I should also handle other attributes such as `FILE_ATTRIBUTE_DEVICE`, `FILE_ATTRIBUTE_OFFLINE`, `FILE_ATTRIBUTE_VIRTUAL` and `FILE_ATTRIBUTE_SYSTEM` since these don't seem like things I want libFuzzer to treat as files.


================
Comment at: compiler-rt/lib/fuzzer/FuzzerIOWindows.cpp:212
+        continue;
+      IterateDirRecursive(Path, DirPreCallback, DirPostCallback, FileCallback);
+    } else if (IsFile(PathAttrs) || IsLink(PathAttrs)) {
----------------
zturner wrote:
> Note that this will not follow directory symlinks or junctions.  Is that the desired behavior?
I just tested my code. I think it does handle symlinks and junctions correctly at least if these are to directories (I guess they have FILE_ATTRIBUTE_DIRECTORY set when they are directories)

Here's what I did to test.

Using an Administrator cmd.exe:
```
mkdir test-dir
cd test-dir

echo "contents" > file.txt
mkdir dir

:: symlink
mklink file-sym-link file.txt

::  hard link
mklink /H file-hardlink1 file.txt

:: hard link to symlink
mklink /H file-hardlink2 file-symlink

:: symlink to dir
mklink /D dir-symlink dir

:: junction to dir
mklink /J dir-junction dir

mklink dir\file-symlink2 file.txt

:: Breaks but I haven't found anything that handles this.
mklink edge-case dir
```

Then when I pass `test-dir` to `IterateDirRecursive` with print statements for DirPostCallback and DirPreCallback I get:
```
File: <PATH>\test-dir\dir\file-symlink2
Dir: <PATH>\test-dir\dir
File: <PATH>\test-dir\dir-junction\file-symlink2
Dir: <PATH>\test-dir\dir-junction
File: <PATH>\test-dir\dir-symlink\file-symlink2
Dir: <PATH>\test-dir\dir-symlink
File: <PATH>\test-dir\edge-case
File: <PATH>\test-dir\file-hardlink1
File: <PATH>\test-dir\file-hardlink2
File: <PATH>\test-dir\file-symlink
File: <PATH>\test-dir\file.txt
Dir: <PATH>\test-dir
```

Other than "File" printing for `edge-case` these results seem correct to me.
I assume it isn't possible to treat `edge-case` as a directory (no other windows software I've seen does).
I think I need to change what I consider a symbolic link to a file so that I don't tread `edge-case` as I file, what do you think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58513/new/

https://reviews.llvm.org/D58513