This patch adds a new flag vfsoverlay similar to clang’s ivfsoverlay flag. This is helpful when compiling on case sensitive file systems when cross compiling to Windows. Particularly when compiling third party code containing #pragma comment(“linker”, “/defaultlib:...”) which can’t be easily changed.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
In the LLVM Windows toolchain, we work around this by creating a directory of symlinks: https://github.com/llvm/llvm-project/blob/8527f32f0a16a77edb0b53a5ab8074e518eeff54/llvm/cmake/platforms/WinMsvc.cmake#L133-L157. A VFS overlay would be a nicer solution, and I'm excited to have this supported. I'll leave the review to @rnk and @mstorsjo though.
This seems reasonable to me, but surely this is not the complete list of places where we must use the VFS?
I added folks with cross-compilation & case (in)sensitivity experience to the review. +@ayzhao as well.
Looks alright to me.
I'll mention that you can get this working without an overlay with ciopfs (https://source.chromium.org/chromium/chromium/src/+/main:build/vs_toolchain.py;l=494?q=ciopfs), but this is an ok approach as well.
Not sure where else it would be useful. Specifically I thought this would be useful because /defaultlib can be specified in source code through pragma comment which might be harder to change than build flags. If folks have any other ideas of where it would be useful to look in the vfs I would be happy to add them.
Seems good to me in principle, I just have a few nits.
Have you checked whether this affects the performance when linking something large-ish without the vfs overlay?
lld/COFF/Driver.cpp | ||
---|---|---|
453 | Is the explicit SmallString constructor call needed, or would just path = getFilename(...) work? | |
458 | Ditto. | |
1367 | nit: I think return nullptr would be more common llvm style. | |
1371 | Shouldn't we give the user an error message? | |
1374 | Ditto if this fails. | |
lld/test/COFF/vfsoverlay.test | ||
7 | Can we have tests cases for when the /vfsoverlay file doesn't exist or is invalid? |
I did not, but can if you believe it could reasonably affect performance. My thinking is that this only adds an if statement to each findFile invocation, which for COFF seems to be very rare, and even for ld.lld which would end up searching for files much more frequently, the cost that one check per findFile doesn't seem high.
lld/COFF/Driver.cpp | ||
---|---|---|
453 | That makes the SmallString ctor fail with an assertion "Attempting to reference an element of the vector in an operation that invalidates it" |
Is the explicit SmallString constructor call needed, or would just path = getFilename(...) work?