aganea added a comment.
In D80833#2073458 <https://reviews.llvm.org/D80833#2073458>, @hans wrote:
> a. The compiler path is only absolute because it was absolute when put into
> argv[0] right? I don't see anything in the code that makes it absolute? I
> imagine most build systems will invoke the compiler using an absolute path so
> you'll get the desired result.
Understood, I wasn't using `-no-canonical-prefixes` that's why I was seeing
full paths for argv[0].
================
Comment at: clang/test/CodeGen/debug-info-codeview-buildinfo.c:1
+// RUN: %clang_cl /c /Z7 %s /Fo%t.obj
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
----------------
hans wrote:
> The %s arg needs to come after "--", otherwise it can be interpreted as a
> command line flag, e.g. on Mac files are often under /Users which will be
> interpreted as a /U flag (see other tests using %clang_cl for examples).
Fixed, Reid already mentionned that a while ago, I'll remember next time!
================
Comment at: clang/tools/driver/cc1_main.cpp:184
-int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
ensureSufficientStack();
----------------
hans wrote:
> Maybe I'm missing something, but why is this changing? The current code with
> Argv0 and the rest of the args as separate parameters seems like it would fit
> in will with the rest of the patch?
Reverted.
================
Comment at: lld/COFF/PDB.cpp:1041
+ // Remap the contents of the LF_BUILDINFO record.
+ remapBuildInfo(tMerger.getIDTable());
+
----------------
hans wrote:
> Naive question because I'm not familiar with the PDB writing, but would it be
> possible to remap the LF_BUILDINFO entries earlier, e.g. when they're read in
> from the object files? It seems like a lot of code is now added to do the
> remapping "after the fact".
The whole type merging machinery isn't designed for changing records
on-the-fly. We only modify TypeIndex values inside the record, but for that we
have hardcoded offsets (all the `discoverTypeIndices` functions in
https://github.com/llvm/llvm-project/blob/cda7ff9ddcefe0051d173e7c126c679063d29fbb/llvm/lib/DebugInfo/CodeView/TypeIndexDiscovery.cpp).
The rest of the record is emitted as-it-is in the output TPI or IPI streams.
It's certainly feaseable to modify records in `TypeStreamMerger` in the same
way we examine the `LF_ENDPRECOMP` record. However that requires a lot more
piping, and I figured it wasn't worth it (unless we have other record types to
modify in the future).
================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:840
+ erase_if(Vec, [&](StringRef Arg) {
+ return Arg.data() == nullptr || Arg == MainFilename;
+ });
----------------
hans wrote:
> Does Arg.data() == nullptr really happen here?
Yes if passing `clang -cc1` directly. Both the `CC1Command` and `InitLLVM` are
pushing a `nullptr` terminator as the last arg :-( This seems quite widespead
behavior across the codebase.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:846
+#else
+ std::string FlatCmdLine = llvm::join(Vec, " ");
+#endif
----------------
hans wrote:
> I suppose this won't work with filenames that contain spaces.
>
> I was hoping we could do whatever "clang -v" does when it prints arguments.
> It seems to do some basic quoting and escaping. Does it have some function we
> could call from here?
The code for printing "clang -v" is/was entirely in Clang:
https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/Job.cpp#L103
I've moved `Command::printArg` to `llvm::sys::printArg` and using that now.
This creates a slight difference from MSVC, in the sense that with Clang all
arguments are quoted, no matter what. Whereas MSVC adds quotes when it's
necessary (that's LLVM's `sys::flattenWindowsCommandLine` behavior). But I
think that shouldn't matter much.
Please @stefan_reinalter @lantictac let me know if you think otherwise.
```
D:\llvm-project> buildninjaRel\bin\clang-cl /c b.cpp /Z7
-fdebug-compilation-dir . -no-canonical-prefixes
D:\llvm-project> buildninjaRel\bin\llvm-pdbutil.exe dump -all b.obj | grep
LF_BUILDINFO -A 5
0x1007 | LF_BUILDINFO [size = 28]
0x1003: `.`
0x1005: `buildninjaRel\bin\clang-cl.exe`
0x1004: `b.cpp`
<no type>: ``
0x1006: `"-cc1" "-triple" "x86_64-pc-windows-msvc19.26.28806"
"-emit-obj" "-mrelax-all" "-mincremental-linker-compatible" "-disable-free"
"-main-file-name" "b.cpp" "-mrelocation-model" "pic" "-pic-level" "2"
"-mthread-model" "posix" "-mframe-pointer=none" "-relaxed-aliasing"
"-fmath-errno" "-fno-rounding-math" "-mconstructor-aliases" "-munwind-tables"
"-target-cpu" "x86-64" "-mllvm" "-x86-asm-syntax=intel" "-D_MT"
"-flto-visibility-public-std" "--dependent-lib=libcmt"
"--dependent-lib=oldnames" "-stack-protector" "2" "-fms-volatile"
"-fdiagnostics-format" "msvc" "-gcodeview" "-debug-info-kind=limited"
"-resource-dir" "buildninjaRel\\lib\\clang\\11.0.0" "-internal-isystem"
"buildninjaRel\\lib\\clang\\11.0.0\\include" "-internal-isystem" "C:\\Program
Files (x86)\\Microsoft Visual
Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\ATLMFC\\include"
"-internal-isystem" "C:\\Program Files (x86)\\Microsoft Visual
Studio\\2019\\Professional\\VC\\Tools\\MSVC\\14.26.28801\\include"
"-internal-isystem" "C:\\Program Files (x86)\\Windows
Kits\\NETFXSDK\\4.8\\include\\um" "-internal-isystem" "C:\\Program Files
(x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" "-internal-isystem"
"C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared"
"-internal-isystem" "C:\\Program Files (x86)\\Windows
Kits\\10\\include\\10.0.18362.0\\um" "-internal-isystem" "C:\\Program Files
(x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" "-internal-isystem"
"C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\cppwinrt"
"-fdeprecated-macro" "-fdebug-compilation-dir" "." "-ferror-limit" "19"
"-fmessage-length=146" "-fno-use-cxa-atexit" "-fms-extensions"
"-fms-compatibility" "-fms-compatibility-version=19.26.28806" "-std=c++14"
"-fdelayed-template-parsing" "-fcolor-diagnostics" "-faddrsig" "-x" "c++"`
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80833/new/
https://reviews.llvm.org/D80833
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
