CDash icon indicating copy to clipboard operation
CDash copied to clipboard

Parsing of errors and warnings from clang-cl

Open mcraveiro opened this issue 7 years ago • 15 comments

Hi CDash-developers,

Whilst working on a separate problem (#732) I noticed that CDash (or is it CTest?) does not seem to detect all of the errors coming out clang-cl. As an example, you can see this build with only four warnings in my CDash [1] but when you look at it on AppVeyor you can see lots of warnings and errors [2]; note that you'll need to download the log [3] to see the actual warnings and errors, but to get a flavour:

[00:01:43] In file included from ..\..\..\..\projects\dogen.utility\tests\resolver_tests.cpp:26:
[00:01:43] In file included from ..\..\..\..\projects\dogen.utility\include\dogen.utility/test/logging.hpp:28:
[00:01:43] In file included from ..\..\..\..\projects\dogen.utility\include\dogen.utility/log/logger.hpp:29:
[00:01:43] In file included from C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/sources/record_ostream.hpp:34:
[00:01:43] In file included from C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/core/record.hpp:21:
[00:01:43] In file included from C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/attributes/attribute_value_set.hpp:26:
[00:01:43] C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/attributes/attribute.hpp(69,5):  error: declaration of anonymous struct must be a definition
[00:01:43]     struct BOOST_LOG_NO_VTABLE BOOST_SYMBOL_VISIBLE impl :
[00:01:43]     ^
[00:01:43] C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/attributes/attribute.hpp(69,5):  warning: declaration does not declare anything [-Wmissing-declarations]
[00:01:43] C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/log/attributes/attribute.hpp(89,20):  error: use of undeclared identifier 'impl'; did you mean 'mpl'?
[00:01:43]     intrusive_ptr< impl > m_pImpl;
[00:01:43]                    ^~~~
[00:01:43]                    mpl
[00:01:43] C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/mpl/for_each.hpp(31,29):  note: 'mpl' declared here
[00:01:43] namespace boost { namespace mpl {
[00:01:43]                             ^

Do I need to set some flag on clang-cl to make the output more compatible with CDash? I will try to have a look at the source code to see if I can get some ideas but in the mean time, any suggestions/pointers are greatly appreciated.

Also, note that my "plain" clang builds are working just fine [1], so it appears to be a clang-cl problem only.

Many thanks for your time.

[1] https://my.cdash.org/viewBuildError.php?type=1&buildid=1543120 [2] https://ci.appveyor.com/project/mcraveiro/dogen/builds/19489775/job/xarvctfsmywvu58q [3] https://ci.appveyor.com/api/buildjobs/xarvctfsmywvu58q/log [4] https://my.cdash.org/index.php?project=MASD+Project&display=project&date=20181013

mcraveiro avatar Oct 14 '18 06:10 mcraveiro

It looks like the build expired on my.cdash.org before I got a chance to look. If this happens again, would you be able to share the Build.xml file that is generated by CTest?

zackgalbreath avatar Oct 16 '18 13:10 zackgalbreath

ARGH! I'm really sorry @zackgalbreath, I deleted the project to try to solve my other issues [1]! However, I do have a few other clang-cl builds [2], [3]. The builds ran on AppVeyor, so I can't access the Build.xml - I'll see if I can come up with some kind of hack to get it. In the mean time, is the build log from AppVeyor [4] of any use?

Many thanks for your help.

[1] https://github.com/Kitware/CDash/issues/732 [2] https://my.cdash.org/viewBuildError.php?type=1&buildid=1543930 [3] https://my.cdash.org/index.php?project=MASD+Project+-+Dogen&date=2018-10-15 (at the bottom, Experimental build group) [4] https://ci.appveyor.com/api/buildjobs/a4a962w4tckfle5u/log

mcraveiro avatar Oct 16 '18 13:10 mcraveiro

Hi @zackgalbreath,

I can now fairly reliably reproduce this problem, as my clang-cl builds are now a bit more mature. I wonder if you can take another look at this, as hopefully my.cdash.org now has the required information [1] (the Experimental Release build, with an error). As an example, I had a look at my AppVeyor log file and I see a lot of warnings [2], but when I look at it from CDash I only see a few [3].

I did try to obtain the Build.xml file from within appveyor but this is a bit of pain I'm afraid.

[1] https://my.cdash.org/index.php?project=MASD+Project+-+C%2B%2B+Reference+Implementation&date=2019-02-15 [2] https://ci.appveyor.com/api/buildjobs/sccsv0ofkfdcrw91/log [3] https://my.cdash.org/viewBuildError.php?type=1&buildid=1608636

mcraveiro avatar Feb 15 '19 14:02 mcraveiro

By the by, I just did a push that suppressed most of the warnings, so any builds after this one won't have them - but if required I can revert the fix. Thanks.

mcraveiro avatar Feb 15 '19 16:02 mcraveiro

The problem seems to be that CTest's built-in regular expressions for matching errors and warnings are failing to understand your compiler's output. If you'd like to take a closer look, here is where these regular expressions are defined.

I recommend giving CTest launchers a try. In your CTest driver script, set the following variables before calling ctest_start:

set(CTEST_USE_LAUNCHERS 1)
set(ENV{CTEST_USE_LAUNCHERS_DEFAULT} 1)

For documentation on what this actually does, search for "UseLaunchers" here: https://cmake.org/cmake/help/latest/manual/ctest.1.html#manual:ctest(1)

zackgalbreath avatar Feb 15 '19 19:02 zackgalbreath

Aha! Thanks a lot! I'll have a look at the regexes; I take it the problem starts before build.xml right? E.g. it won't help if you attach the file?

mcraveiro avatar Feb 15 '19 20:02 mcraveiro

Aha! Thanks a lot! I'll have a look at the regexes; I take it the problem starts before build.xml right? E.g. it won't help if you attach the file?

That's right, the Build.xml file contains the results of running those regexes on your build output.

zackgalbreath avatar Feb 15 '19 21:02 zackgalbreath

Cool, thanks for that. So, I've done some ad-hoc analysis on this and I think I now understand what is going on. If am I right, this is a tad painful! :-)

Clang-cl tries to emulate MSVC as much as possible, so in theory what works for one should work for the other [1]. However, I think they did not match the compiler output to a byte-wise diff, as I have spotted some differences. My testing was as follows: I have taken the regexes on the page you linked above [2] and, using a quick hand-crafted sample, I tried to figure out how they behave. The samples, taken from my build, were as follows:

MSVC:

   stereotypes_conversion_result_hash.cpp
   archetype_family_properties_hash.cpp
   c:\projects\dogen\projects\masd.dogen.coding\src\hash\helpers\node_hash.cpp(58): warning C4717: 'masd::dogen::coding::helpers::node_hasher::hash': recursive on all control paths, function will cause runtime stack overflow [C:\projects\dogen\build\output\msvc\Release\projects\masd.dogen.coding\src\masd.dogen.coding.lib.vcxproj]
   archetype_properties_hash.cpp
   artefact_hash.cpp

Clang-CL:

C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/pending/integer_log2.hpp(7,1):  warning: C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/pending/integer_log2.hpp(7): note: This header is deprecated. Use <boost/integer/integer_log2.hpp> instead. [-W#pragma-messages]
BOOST_HEADER_DEPRECATED("<boost/integer/integer_log2.hpp>");
 ^
C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/config/header_deprecated.hpp(23,37):  note: expanded from macro 'BOOST_HEADER_DEPRECATED'
 # define BOOST_HEADER_DEPRECATED(a) BOOST_PRAGMA_MESSAGE("This header is deprecated. Use " a " instead.")
                                     ^
C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/config/pragma_message.hpp(26,43):  note: expanded from macro 'BOOST_PRAGMA_MESSAGE'
# define BOOST_PRAGMA_MESSAGE(x) __pragma(message(__FILE__ "(" BOOST_STRINGIZE(__LINE__) "): note: " x))
                                           ^
 1 warning generated.

After some experimentation, the regex that catches MSVC is (at least) this one in cmCTestWarningMatches: ([^:]+): warning [3]. I then noticed that the clang-cl output is not caught by this regex because it has one extra space. Changing the regex to account for this does indeed produce a match (e.g. ([^:]+): warning) [4]. It seems to the untrained eye that cmCTestWarningMatches needs to be extended with an additional space, though one wonders if this is a scalable approach.

For good measure, I repeated the same exercise with an error:

Clang-CL

 C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/type_traits/has_trivial_move_assign.hpp(49,4):  error: no template named 'is_assignable'; did you mean 'std::is_assignable'?
    BOOST_HAS_TRIVIAL_MOVE_ASSIGN(T)
    ^
 C:\third_party\installs\vcpkg-export\installed\x64-windows-static\include\boost/type_traits/intrinsics.hpp(233,86):  note: expanded from macro 'BOOST_HAS_TRIVIAL_MOVE_ASSIGN'
 #     define BOOST_HAS_TRIVIAL_MOVE_ASSIGN(T) (__is_trivially_assignable(T&, T&&) && is_assignable<T&, T&&>::value && !::boost::is_volatile<T>::value)
                                                                                      ^
 C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.16.27023\include\type_traits(889,9):  note: 'std::is_assignable' declared here
         struct is_assignable
                ^
 In file included from ..\..\..\..\projects\masd.dogen\src\types\configuration.cpp:21:
 In file included from ..\..\..\..\projects\masd.dogen\include\masd.dogen/types/configuration.hpp:29:

In this particular case, the match was with (at least) ([^:]+): (Error:|error|undefined reference|multiply defined) in cmCTestErrorMatches - but, again, after adding an extra space [5]. Its not ideal that clang-cl decided to introduce these spurious differences, but not much can be done, I guess, since its already in the wild. What is the best way forward, do you think?

Thanks very much for your time.

[1] https://clang.llvm.org/docs/MSVCCompatibility.html [2] https://gitlab.kitware.com/cmake/cmake/blob/master/Source/CTest/cmCTestBuildHandler.cxx#L23 [3] https://regex101.com/r/8pepN4/2 [4] https://regex101.com/r/3RcU9g/1 [5] https://regex101.com/r/uYF5ao/1

mcraveiro avatar Feb 18 '19 16:02 mcraveiro

Thanks very much for your time.

You're welcome!

Like I hinted at earlier, enabling CTest launchers might be a good solution to this problem you're experiencing. For supported generators (makefiles and ninja) this works by wrapping each invocation of the compiler in an external "launcher" command. This launcher communicates with CTest via environment variables, providing more fine-grained information about what goes wrong & when.

zackgalbreath avatar Feb 18 '19 17:02 zackgalbreath

This is in contrast to the default "log scraping" approach that uses the regular expressions that you've been experimenting with.

zackgalbreath avatar Feb 18 '19 17:02 zackgalbreath

Like I hinted at earlier, enabling CTest launchers might be a good solution to this problem you're experiencing. For supported generators (makefiles and ninja) this works by wrapping each invocation of the compiler in an external "launcher" command. This launcher communicates with CTest via environment variables, providing more fine-grained information about what goes wrong & when.

But then everyone using clang-cl will also have to have a manual workaround right? At any rate, I will submit this to the clang mailing list, perhaps they are not aware of it. Seems a tad gratuitous, given they are trying to be as compatible with MSVC as possible.

mcraveiro avatar Feb 18 '19 17:02 mcraveiro

But then everyone using clang-cl will also have to have a manual workaround right?

That's right. It's an extra step and I agree if we could fix this upstream it would be great. But a lot of our projects (CMake included) uses launchers when possible, so IMO it's worth trying if you get a chance.

zackgalbreath avatar Feb 18 '19 17:02 zackgalbreath

That's right. It's an extra step and I agree if we could fix this upstream it would be great. But a lot of our projects (CMake included) uses launchers when possible, so IMO it's worth trying if you get a chance.

Understood. I will try to have a look at launchers when I get another break. Meanwhile, I asked for more info on the diffs to the clang-cl devs [1]. I'll leave it at that for now. I'll update the ticket when I hear from them.

Cheers

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-February/061326.html

mcraveiro avatar Feb 18 '19 17:02 mcraveiro

OK so the good news is, the clang-cl developers weren't aware of these whitespace differences [1]; they marked it as a bug and fixed it for the next releases [2]. However, given that there already exist a number of clang-cl releases with this behaviour, they asked if we could patch CDash to cope with it. Will you be willing to take a patch if I submit it? I'll literally just copy and paste those regexes and add a space, plus some tiny blurb explaining why they are needed.

Cheers

[1] http://lists.llvm.org/pipermail/cfe-dev/2019-February/061339.html [2] https://reviews.llvm.org/D58377

mcraveiro avatar Feb 19 '19 09:02 mcraveiro

Sounds good to me. This change will have to go in cmake/ctest. We would certainly appreciate you submitting a merge request with this fix. If that turns out to be too much trouble, you can send me a diff and I'll work on getting the change integrated into CMake master.

zackgalbreath avatar Feb 19 '19 14:02 zackgalbreath