[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked an inline comment as done.
aganea added a comment.

In D80833#2069246 <https://reviews.llvm.org/D80833#2069246>, @amccarth wrote:

> Does the full path to the tool end up only in the object file, or does this make it all the way to the final executable or library?


The `LF_BUILDINFO` records for each .OBJ are emitted as they are in the final .PDB.

> Does embedding full paths affect distributed builds or build reproducibility?

It does affect reproducibility for distributed builds, the contents of `LF_BUILDINFO` for a remotely-compiled .OBJ will be different in the final .PDB.
In our case, compilation jobs can be executed locally or remotely, depending on how they are queued.
For example,

- If a local compilation, `C:\.nuget\llvm.10.0.0.2\bin\clang-cl.exe` would be stamped in the `LF_BUILDINFO` record.
- If a remote compilation, `C:\Users\SomeUser\AppData\Local\Temp\.fbuild.tmp\worker\toolchain.130589cdf35aed3b\clang-cl.exe` would be stamped (the compiler executable is sent to the remote worker).

But you've got a good point. We would need an way to force the remote compiler to stamp the local path (which is unique for all users). We've got a manual override if the compiler path referred by `LF_BUILDINFO` can't be found locally, but this is a bit annoying, see: https://liveplusplus.tech/docs/documentation.html#FASTBuild.
It is better if the right information was there in the first place in the .OBJ. Could we do a remapping instead through VFS maybe? I'd like to avoid adding yet-a-new-flag to handle that.



================
Comment at: clang/include/clang/Basic/CodeGenOptions.h:320
+  /// Executable and command-line used to create a given CompilerInvocation.
+  const char *BuildTool = nullptr;
+  ArrayRef<const char *> CommandLineArgs;
----------------
hans wrote:
> aganea wrote:
> > hans wrote:
> > > The name BuildTool makes me think of things like Make or MSBuild rather than the compiler. And in this case we know the compiler is Clang :-) Also since this is for capturing the cc1 arguments, it shouldn't really matter if it's clang-cl, clang++ or just clang. So it seems unfortunate that it has to be piped through all the way like this..
> > > 
> > > Is there some way we could just grab the path to the current clang binary during the pdb writing?
> > There's `sys::fs::getMainExecutable(const char *argv0, void *MainExecAddr)` but that's not guaranteed to work on all platforms, you still need to provide argv[0] to fallback, see https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Unix/Path.inc#L232
> > I'm not well versed in Unix distros, can this only rarely occur?
> > Would you prefer that I used that function instead? (it is ok on Windows though)
> > 
> I think it can occur for example if the compiler is running in a chroot. And probably other cases as well.
> So maybe piping the path through is better. But perhaps we should just call it argv0?
> Or could we get away with putting just "clang" there? Does the tool consuming this info actually need the real path to the compiler or could we rely on it finding it on PATH?
Ok for argv0.
Users can have several versions of a toolchain installed locally, if they switch between branches or if working on different games at the same time.
We need to identify the right compiler through its full path, we can't rely on PATH.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833