Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Visual Studio LLVM toolchain clang-cl --coverage option and linker errors #39846

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR40877
Status NEW
Importance P normal
Reported by Alexander Neumann (neumann@imt.uni-luebeck.de)
Reported on 2019-02-27 01:45:54 -0800
Last modified on 2019-09-25 09:03:14 -0700
Version unspecified
Hardware PC Windows NT
CC hans@chromium.org, htmldeveloper@gmail.com, llvm-bugs@lists.llvm.org, mcastelluccio@mozilla.com, rnk@google.com, yurybura@gmail.com, zturner@google.com, zufuliu@163.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also PR40948
So I played with the --coverage option for clang-cl and found that my test
projects where not linking due to missing symbols.

After a bit of search for the symbols I found out that I need to link against
"clang_rt.profile-x86_64.lib". After doing that linking works fine. The problem
however is that linking to the file requires a full path. In my CMake file the
command looks like:

> target_link_libraries(TestObjLib INTERFACE
"\$(LLVMInstallDir)\\lib\\clang\\${CMAKE_CXX_COMPILER_VERSION}\\lib\\windows\\clang_rt.profile-x86_64.lib")

Personally I would expect one of the following behaviors

a) --coverage automatically adds "clang_rt.profile-x86_64.lib" to the required
dependencies

or

b) I have to manually add "clang_rt.profile-x86_64.lib" as a dependency but
only that and not the full path.
> target_link_libraries(TestObjLib INTERFACE clang_rt.profile-x86_64.lib)
should be enough
This obviously requires the LLVM toolset in VS to add
\$(LLVMInstallDir)\\lib\\clang\\<CLANG_VERSION_PLACEHOLDER>\\lib\\windows\\ to
the library paths.

Other minor details:
clang-cl -help does not list the --coverage option
Quuxplusone commented 5 years ago
+mcastelluccio who exposed the -coverage flag in clang-cl in r317709 and
+rnk who reviewed it
+zturner who maintains the LLVM VS toolset

> Personally I would expect one of the following behaviors
>
> a) --coverage automatically adds "clang_rt.profile-x86_64.lib" to the
required dependencies

It works automatically if you also use clang-cl to link.

But I see you're using the LLVM toolset, so I suppose it's invoking link.exe
directly.

Maybe Zach knows if we could do something in the toolset to make this work
better.

> Other minor details:
> clang-cl -help does not list the --coverage option

This is because the option doesn't have any help text.

Marco, how was your experience using clang-cl with -coverage? Would you like to
help improve the documentation, including help text for the flag?
Quuxplusone commented 5 years ago
Detail: LLVM toolset in VS defaults to ldd-link.exe

the LLVM Toolset would require a registry key for the installed version.
Currently only the Path is in the registry and read out by the toolset via:

><LLVMInstallDir Condition="'$(LLVMInstallDir)' ==
''">$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM\LLVM)</LLVMInstallDir>

Something like
><LLVMVersion Condition="'$(LLVMVersion)' ==
''">$(Registry:HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM\Version)</LLVMVersion>
Would be necessary to add the dir
>\$(LLVMInstallDir)\\lib\\clang\\$(LLVMVersion)\\lib\\windows\\

to the property LibraryPath within the LLVM.Common.props file

The question thus is: Who writes the registry key:
HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM\LLVM
and could add
HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\LLVM\Version
to it

(but this would be way b) and not way a) )

Way a could be resolved by the above and adding all *.lib within
\$(LLVMInstallDir)\\lib\\clang\\$(LLVMVersion)\\lib\\windows\\ to the
additional dependencies in the props file
Quuxplusone commented 5 years ago
(In reply to Hans Wennborg from comment #1)
> > Other minor details:
> > clang-cl -help does not list the --coverage option
>
> This is because the option doesn't have any help text.
>
> Marco, how was your experience using clang-cl with -coverage? Would you like
> to help improve the documentation, including help text for the flag?

Yes, we've been using it for a while now. I'll file a separate bug for adding
documentation, but I'm not sure when I'll get to it.

Between the two proposed options, I definitely prefer (a). It'd be nice if the -
-coverage option worked by itself, without the user having to manually link to
the profile lib.
BTW, the same problem applies to PGO.
Quuxplusone commented 5 years ago

If clang can find its own resource directory, can't we make LLD also be able to find this as a builtin library path?

This would only work when the linker is set to LLD of course.

Quuxplusone commented 5 years ago
I think it's better for clang to works without explicit linker flags just like
using -fsanitize=undefined:
https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#usage
> Use clang++ to compile and link your program with -fsanitize=undefined flag.
Make sure to use clang++ (not ld) as a linker, so that your executable is
linked with proper UBSan runtime libraries.
Quuxplusone commented 5 years ago
In the long run, I'd like things to work the way I (apparently!) described for
asan back in 2014:
https://reviews.llvm.org/D4428#56536

> What do you think of embedding just the basename into .drectve, and adding
> clang's resource dir to the LIB environment variable in the build system?
> That's a far less invasive change than making clang do the link step instead
> of link.exe, and should solve the problem.

Which, I guess, is midway between what we do for --coverage and ubsan today.