[PDB] Don't include input files in the 'cmd' entry of S_ENVBLOCK

[saudi]

Summary

The S_ENVBLOCK is used for the Linker information inside of the pdb file contains an entry "cmd" for the command line arguments.
This patch aligns this entry to what MSVC generates, stripping the input files arguments.

The issue we encountered was in the case of a large project where we link many .obj files, and trying to use the Live++ tool for edit-and-continue.
Since the linker's command line reached 100KB, it gets truncated to match the 64 KB limit of the S_ENVBLOCK of the PDB file.
Live++ uses it to detect whether the appropriate flags (like /FUNCTIONPADMIN) were used for proper hotpatching.
It may miss truncated arguments and give false warnings to the user.

In our case, with this patch, the 100KB command line (which tends to get bigger as the project evolves) gets reduced to about 12KB, and evolves much slower.

Pros:

• align on MSVC's behavior
• keep S_ENVBLOCK size under the 64KB limit

Cons:

• the command line found in the pdb can't be used as-is to link the project the way it was.

[saudi]

Added release note

[thakis]

lld/COFF/PDB.cpp

1357 ↗

(On Diff #474292)

What if I'm cross-linking on Linux and passing in /path/to/foo.o?

[saudi]

lld/COFF/PDB.cpp

1357 ↗

(On Diff #474292)

Indeed, I can't make the assumption that any argument starting with / is an input file.
I'll investigate more to get a posix-friendly version.

[saudi]

Perform the argument list filtering in the driver, to use the result of parsed arguments.
Updated pdb-linker-module.test, checking that the object file is indeed discarded.

Since the object file is provided as an absolute path, this checks that '/path/to/file.o' is not mistaken for a command line argument.

[thakis]

Works for me, thanks!

lld/COFF/DriverUtils.cpp

859

Instead of this line, maybe config->argv.push_back(argv[0]);? Maybe a bit more direct.

[saudi]

Simplified init of config->argv

[saudi]

lld/COFF/DriverUtils.cpp

859

replacing with = {argv[0]} to keep the previous behavior concerning re-initialization of config->argv

[thieta]

LGTM!

lld/COFF/DriverUtils.cpp

opt::InputArgList ArgParser::parse(ArrayRef<const char *> argv) {
   if (args.hasArg(OPT_verbose) && argv.size() != expandedArgv.size()) {
     std::string msg = "Command line:";
     for (const char *s : expandedArgv)
       msg += " " + std::string(s);
     message(msg);
   }
 
   // Save the command line after response file expansion so we can write it to
-  // the PDB if necessary.
-  config->argv = {expandedArgv.begin(), expandedArgv.end()};
+  // the PDB if necessary. Mimic MSVC, which skips input files.
+  config->argv = {argv[0]};
+  for (opt::Arg *arg : args) {
+    if (arg->getOption().getKind() != opt::Option::InputClass) {
+      config->argv.push_back(args.getArgString(arg->getIndex()));
+    }
+  }
 
   // Handle /WX early since it converts missing argument warnings to errors.
   errorHandler().fatalWarnings = args.hasFlag(OPT_WX, OPT_WX_no, false);
 
   if (missingCount)
     fatal(Twine(args.getArgString(missingIndex)) + ": missing argument");
 
   handleColorDiagnostics(args);

lld/docs/ReleaseNotes.rst

* ``--no-warnings``/``-w`` is now available to suppress warnings.
   (`D136569 <https://reviews.llvm.org/D136569>`_)
 
 Breaking changes
 ----------------
 
 COFF Improvements
 -----------------
 
-* ...
+* The linker command line entry in ``S_ENVBLOCK`` of the PDB is now stripped
+  from input files, to align with MSVC behavior.
+  (`D137723 <https://reviews.llvm.org/D137723>`_)
 
 MinGW Improvements
 ------------------
 
 * The lld-specific options ``--guard-cf``, ``--no-guard-cf``,
   ``--guard-longjmp`` and ``--no-guard-longjmp`` has been added to allow
   enabling Control Flow Guard and long jump hardening. These options are
   disabled by default, but enabling ``--guard-cf`` will also enable

lld/test/COFF/pdb-linker-module.test

 SYMS-NEXT: 64 | S_ENVBLOCK
 SYMS-NEXT: - cwd
 SYMS-NEXT: -
 SYMS-NEXT: - exe
 SYMS-NEXT: - {{.*}}lld-link
 SYMS-NEXT: - pdb
 SYMS-NEXT: - {{.*}}pdb-linker-module{{.*}}pdb
 SYMS-NEXT: - cmd
-SYMS-NEXT: - /debug /pdb:{{.*}}pdb-linker-module{{.*}}pdb /nodefaultlib "/entry:1 ""hello"" 2" "/manifestuac:level='asInvoker' uiAccess='false'" {{.*}}pdb-diff.obj
+SYMS-NEXT: - /debug /pdb:{{.*}}pdb-linker-module{{.*}}pdb /nodefaultlib "/entry:1 ""hello"" 2" "/manifestuac:level='asInvoker' uiAccess='false'" /force

lld/test/COFF/pdb-relative-source-lines.test

 CHECK-NEXT:       Entries:     
 CHECK-NEXT:         - cwd
 CHECK-NEXT:         - 'c:\src'
 CHECK-NEXT:         - exe
 CHECK-NEXT:         - 'c:\src\lld-link'
 CHECK-NEXT:         - pdb 
 CHECK-NEXT:         - 'c:\src\out.pdb'
 CHECK-NEXT:         - cmd
-CHECK-NEXT:         - '-debug -pdbsourcepath:c:\src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj'
+CHECK-NEXT:         - '-debug -pdbsourcepath:c:\src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb'
 
 CHECK-LABEL: IpiStream:
 
 CHECK:    - Kind:            LF_STRING_ID
 CHECK-NEXT:      StringId:
 CHECK-NEXT:        Id:              0
 CHECK-NEXT:        String:          .
 CHECK-NEXT:     - Kind:            LF_STRING_ID

 POSIX-NEXT:       Entries:     
 POSIX-NEXT:         - cwd
 POSIX-NEXT:         - '/usr/src'
 POSIX-NEXT:         - exe
 POSIX-NEXT:         - '/usr/src/lld-link'
 POSIX-NEXT:         - pdb 
 POSIX-NEXT:         - '/usr/src/out.pdb'
 POSIX-NEXT:         - cmd
-POSIX-NEXT:         - '-debug -pdbsourcepath:/usr/src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb pdb_lines_1_relative.obj pdb_lines_2_relative.obj'
+POSIX-NEXT:         - '-debug -pdbsourcepath:/usr/src -entry:main -nodefaultlib -out:out.exe -pdb:out.pdb'
 
 ABSOLUTE-LABEL: StringTable:
 ABSOLUTE-NOT: {{/|\\}}.{{/|\\}}pdb_lines_1.c
 
 POSIX-LABEL: IpiStream:
 POSIX:    - Kind:            LF_STRING_ID
 POSIX-NEXT:      StringId:
 POSIX-NEXT:        Id:              0