[PATCH] D36265: Support lib changes to allow VFS implementation handling working dir locally.

[Ilya Biryukov via Phabricator via llvm-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D36265#830856, @zturner wrote:

> Also, even without using the `Nt` functions, you could get the working directory via the process's `HANDLE`, then stitch together a new working directory and build an absolute path out of it.  It's not entirely atomic the way `openat` is because someone could change the working directory of the process after you query its working directory, but I suspect that wouldn't matter?


Right, that does not matter. I actually made a patch that does exactly that in clang's `vfs::FileSystem`, it doesn't require any changes in the Support library and all LLVM tests seem to pass if you do that: https://reviews.llvm.org/D36226?vs=on&id=109362&whitespace=ignore-most#toc.
But there's a major flaw in that implementation: if your working dir is symlinked and it changes during the lifetime of your program, you will be looking into different directories on each file lookup.
Unfortunately, that's what would break us at Google, since we to access the code via a FUSE filesystem that has a symlink that will change its destination on each request, we really need to read its file descriptor only once.

In https://reviews.llvm.org/D36265#830854, @zturner wrote:

> It's not just Windows, but not even all unixes support `openat`.  Minimum required version of various Unixes are:
>
> DragonFly 2.3.
>  FreeBSD 8.0.
>  Linux 2.6.16
>  NetBSD 7.0.
>  OpenBSD 5.0.
>  OS X 10.10.
>
> I don't know how old some of these are, but you should check first to make sure we don't support any versions of these unix variants with versions older than specified above.  As for Windows, If we're going to use `Nt` functions, I would rather they be entirely separate functions.  i.e. not done via an optional parameter to an existing function, but a completely new function.


Thanks, we definitely don't want to make LLVM incompatible with those platforms. I imagine we still need to keep LLVM running on those versions, they aren't too old and probably still widely used. 
Now it seems we'll have to put this as a separate implementation under a configurable macro, so that it can be turned off on systems that don't support `openat`.
Maybe there's a list of minimal versions that LLVM strives to support?

Technically, creating separate functions should not be too hard. 
There are two concerns that I have with having both APIs:

1. We must be very careful to not mix `Win32` and `Nt` APIs (i.e. file opened via `NtCreateFile` should be read via `NtReadFile`, not `ReadFile`).
2. Having both `Win32` and `Nt` functions  is that `Nt` variants may stay untested if most of LLVM and Clang will be using `Win32`.

What problems do you foresee if we switch to `Nt` API everywhere?


https://reviews.llvm.org/D36265