[PATCH] D49366: Add support for /FUNCTIONPADMIN command-line option

[Zachary Turner via Phabricator via llvm-commits]

zturner added reviewers: rnk, ruiu, pcc.
zturner requested changes to this revision.
zturner added subscribers: ruiu, rnk.
zturner added a comment.
This revision now requires changes to proceed.

You'll definitely need to add a test here, hopefully @ruiu or @rnk can suggest the best way to write one.  But I think there should be at least 3.  One that tests `/FUNCTIONPADMIN` with x86 produces 5, one that tests `/FUNCTIONPADMIN` with x64 produces 6, and one that tests that `/FUNCTIONPADMIN:N` produces N bytes of padding.



================
Comment at: COFF/DriverUtils.cpp:271-273
+    } else {
+      error("/functionpadmin: invalid argument for this machine: " + Arg);
+    }
----------------
What about ARM64?  The documentation doesn't say what the default value is here, but it should be easy to figure out.


================
Comment at: COFF/PDB.cpp:118-122
+  Expected<const CVIndexMap &> mergeDebugT(ObjFile *File,
+                                           CVIndexMap &ObjectIndexMap);
 
-  Expected<const CVIndexMap&> maybeMergeTypeServerPDB(ObjFile *File,
-                                                      TypeServer2Record &TS);
+  Expected<const CVIndexMap &> maybeMergeTypeServerPDB(ObjFile *File,
+                                                       TypeServer2Record &TS);
----------------
It looks like you may have run clang-format against the entire source files, rather than against only the lines that you changed.  Would it be possible to revert these changes and only clang-format changed lines?  There's a script called git-clang-format in the clang source tree.  Usually what we do is add this location to `PATH`, and then run `git clang-format`  That will only format touched lines.


================
Comment at: COFF/PDB.cpp:790
+    // without any padding to mimic the behaviour of link.exe.
+    SC.Size = SecChunk->getRawSize();
+
----------------
That is quite odd.  Wouldn't this mean that the PDB has incorrect info?  I wonder how this could possibly be correct.  So either it's a bug in link.exe or there's something I don't understand about what PDB conumers use this information for.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D49366