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

[Alexandre Ganea via Phabricator via cfe-commits]

aganea marked 5 inline comments as done.
aganea added a comment.

In D80833#2071863 <https://reviews.llvm.org/D80833#2071863>, @hans wrote:

> In D80833#2071818 <https://reviews.llvm.org/D80833#2071818>, @aganea wrote:
>
> > I didn't change the PWD/CWD stored in `LF_BUILDINFO`, it has already been done by Reid a while ago: D53179 <https://reviews.llvm.org/D53179> (it already stores absolute paths)
>
>
> That's surprising to me. Nico is the real export on reproducible builds, but we generally try very hard to make builds reproducible independent of source and build dir. Maybe there should be a flag controlling this? /Brepro?


Ah, you must be using `-fdebug-compilation-dir` on Chromium I suppose, to make paths relative in .OBJs. Otherwise the default is the full path to PWD/CWD. Added a test for that.

In D80833#2071969 <https://reviews.llvm.org/D80833#2071969>, @thakis wrote:

> We don't store physical absolute paths in pdbs if you hold things right. Look for /pdbsourcepath: in http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html . rnk's change seems to store the main TU's dir as cwd, which likely either takes the dir from the main TU as passed (ie if you pass an erlative path, it stores that), or it honors /pdbsourcepath:. If that's fine as-is, that's ok. It's different from what `pwd` returns though I think. (imagine you're in c:\foo and you build bar\baz.cc. the cwd in the pdb will be c:\foo\bar but the command will be relative to c:\foo if I understand things right? I might not though.)


Without `-fdebug-compilation-dir`, the **full path **is stored in `LF_BUILDINFO` (see `CodeGenOpts::DebugCompilationDir` and `CGDebugInfo::getCurrentDirname()`)
With `-fdebug-compilation-dir .`, the PWD/CWD becomes relative ('.') and I've added code in LLD to call `pdbMakeAbsolute` on it, like for other PDB strings.
Now `/pdbsourcepath` works as well (all this is covered in lld/test/COFF/pdb-relative-source-lines.test).
However if you're in `c:\foo` and you do `clang-cl /c bar\baz.cc`, then `bar\baz.cc` will be stored as-it-is in `LF_BUILDINFO` (the PWD/CWD and the main filename are stored separately)

The only absolute paths that remain are: a. the compiler path (`D:\llvm-project\buildninjaDebMSVC\bin\clang-cl.exe` in the yaml below) and b. the `-internal-isystem` paths. However that is not an issue on our end, as we're building with `-nostdinc` + nuget packages installed in a fixed dir on all users' machines. Not sure how that works for you?



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:835
+static std::string renderCommandLine(ArrayRef<const char *> CommandLineArgs,
+                                     StringRef MainFile) {
+  std::string FlatCmdLine;
----------------
aganea wrote:
> hans wrote:
> > Don't we already have code somewhere that can do this quoting? E.g. the code that prints the cc1 args for "clang -v"?
> Yes, the code below was copy-pasted from `Command::printArg`. But that's in Clang. Should I make that code common somewhere in `llvm/lib/Support`?
Replaced by `llvm::sys::flattenWindowsCommandLine`. I can't find any equivalent for Linux, I hope just aggregating the arguments with spaces in between is fine?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833