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

[Jonathan Metzman via Phabricator via llvm-commits]

metzman added a comment.

> I'm not sure this should be landed yet since IterateDirRecursive treats edge-case as a file.
> I suspect that is harmless, but ideally it wouldn't do this.

Actually I don't think there are negative consequences from treating this as a file since I haven't seen anything treat a case like this as a directory.

In D58513#1406454 <https://reviews.llvm.org/D58513#1406454>, @zturner wrote:

> In D58513#1406438 <https://reviews.llvm.org/D58513#1406438>, @metzman wrote:
>
> > In D58513#1406402 <https://reviews.llvm.org/D58513#1406402>, @zturner wrote:
> >
> > > In D58513#1406398 <https://reviews.llvm.org/D58513#1406398>, @metzman wrote:
> > >
> > > > In D58513#1406388 <https://reviews.llvm.org/D58513#1406388>, @zturner wrote:
> > > >
> > > > > Can you try `mountvol <PATH>\mount C:\` and see what you print in this case?
> > > >
> > > >
> > > > This seems to break:
> > > >
> > > >   File: <PATH>\test-dir\dir\file-symlink2
> > > >   File: <PATH>\test-dir\dir\y.txt
> > > >   Dir: <PATH>\test-dir\dir
> > > >   File: <PATH>\test-dir\dir-junction\file-symlink2
> > > >   File: <PATH>\test-dir\dir-junction\y.txt
> > > >   Dir: <PATH>\test-dir\dir-junction
> > > >   File: <PATH>\test-dir\dir-Shortcut.lnk
> > > >   File: <PATH>\test-dir\dir-symlink\file-symlink2
> > > >   File: <PATH>\test-dir\dir-symlink\y.txt
> > > >   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
> > > >   FindFirstFileA failed with error 5 for directory: <PATH>\ss64\$Recycle.Bin\blah; exiting
> > > >
> > > >
> > > > I'm not sure what the correct behavior in this case is though.
> > >
> > >
> > > Actually just use a different directory then.  Like mount D: instead of C:.  Recycle bin is protected, so you probably won't be able to get directly into it without special permissions.
> > >
> > > What I'm really wondering about is whether a volume mounted with mountvol will fall into your directory branch or your file branch.  It should go into your directory branch.
> >
> >
> > It takes the directory branch.
> >  Do we want it to take either though? Is this a common use case?
> >  This particular case has me worried because if LF used delete as its callback function for files it would be like calling `rm -rf ~/`
>
>
> It's not really that common, but is it fundamentally any different than creating a junction (or directory symlink) to your root drive?  If you follow symlinks, this is always going to be a problem, and the only *real* solution is to just not follow any directory reparse points.


Fair point.

> 
> 
>> 
>> 
>>> That said, it looks like this does find a more general bug, which is that if the directory is inaccessible you will fail and terminate the iteration.  You should probably handle this gracefully and just return from the function is `FindFirstFile` fails.
>> 
>> Done.

@zturner 
Do you think there's anything that needs fixing in IterateDirRecursive that should prevent this from landing?

(mostly for myself): I did notice one other blocking issue that should be fixed before landing. libFuzzer isn't cleaning up after itself when using fork mode if I send it ctrl-C.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58513