[cxxmodules] Print stacktrace before aborting on a missing exception. #921

Teemperor · 2017-09-01T11:37:12Z

We will probably see an increasing amount of these failures with
C++ modules as we now deserialize all declarations instead of just
the PCH ones. To safe us a lot of debugging time on where to push
the needed transaction, let's directly print the stack trace here
in the rare case that we crash here.

phsft-bot · 2017-09-01T11:37:26Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-09-01T11:44:57Z

Build failed on centos7/gcc49.
See console output.

Errors:

/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:42: error: ‘Transaction’ does not name a type
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.h:57:18: error: invalid use of non-static data member ‘cling::DeclCollector::m_CurTransaction’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:80:28: error: from this location
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:114:42: error: cannot convert ‘cling::Transaction* const’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:196:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:235:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:244:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:255:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:267:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:285:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’

And 2 more

phsft-bot · 2017-09-01T11:48:08Z

Build failed on slc6/gcc49.
See console output.

Errors:

/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:42: error: ‘Transaction’ does not name a type
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.h:57:18: error: invalid use of non-static data member ‘cling::DeclCollector::m_CurTransaction’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:80:28: error: from this location
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:114:42: error: cannot convert ‘cling::Transaction* const’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:196:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:235:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:244:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:255:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:267:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:285:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’

And 2 more

phsft-bot · 2017-09-01T11:50:53Z

Build failed on slc6/gcc62.
See console output.

Errors:

/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:42: error: ‘Transaction’ does not name a type
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:80:28: error: invalid use of non-static data member ‘cling::DeclCollector::m_CurTransaction’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:114:42: error: cannot convert ‘cling::Transaction* const’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:196:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:235:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:244:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:255:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:267:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:285:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:303:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’

And 1 more

phsft-bot · 2017-09-01T12:36:18Z

Build failed on mac1012/native.
See console output.

Errors:

/Volumes/MacintoshHD/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:42: error: unknown type name 'Transaction'; did you mean 'cling::Transaction'?
/Volumes/MacintoshHD/build/jenkins/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:80:28: error: use of non-static data member 'm_CurTransaction' of 'DeclCollector' from nested type 'PPAdapter'

phsft-bot · 2017-09-01T12:57:36Z

Build failed on ubuntu14/native.
See console output.

Errors:

/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:42: error: ‘Transaction’ does not name a type
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:64:55: error: ISO C++ forbids declaration of ‘transaction’ with no type [-fpermissive]
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.h:57:18: error: invalid use of non-static data member ‘cling::DeclCollector::m_CurTransaction’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:80:28: error: from this location
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:114:42: error: cannot convert ‘cling::Transaction* const’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:196:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:235:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:244:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:255:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’
/mnt/vdb/lsf/workspace/root-pullrequests-build/root/interpreter/cling/lib/Interpreter/DeclCollector.cpp:267:42: error: cannot convert ‘cling::Transaction*’ to ‘const int*’ for argument ‘1’ to ‘void {anonymous}::assertHasTransaction(const int*)’

And 3 more

phsft-bot · 2017-09-01T13:02:36Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-09-01T13:58:07Z

Build failed on slc6/gcc49.
See console output.

Errors:

ERROR: Timeout after 10 minutes
ERROR: Error fetching remote repo 'origin'
stderr: error: RPC failed; result=18, HTTP code = 200
ERROR: Error fetching remote repo 'origin'

phsft-bot · 2017-09-01T14:03:43Z

Build failed on slc6/gcc62.
See console output.

Errors:

ERROR: Timeout after 10 minutes
ERROR: Error fetching remote repo 'origin'
stderr: error: RPC failed; result=18, HTTP code = 200
ERROR: Error fetching remote repo 'origin'

Teemperor · 2017-09-01T15:48:14Z

@phsft-bot build

phsft-bot · 2017-09-01T15:48:26Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

vgvassilev · 2017-09-01T17:56:17Z

@phsft-bot build with flags -Dclingtest=On

phsft-bot · 2017-09-01T17:56:28Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

phsft-bot · 2017-09-01T19:09:30Z

Build failed on mac1012/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_countIncludePaths

phsft-bot · 2017-09-07T18:19:12Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

phsft-bot · 2017-09-07T19:32:00Z

Build failed on mac1012/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_countIncludePaths

phsft-bot · 2017-09-07T19:46:21Z

Build failed on slc6/gcc49.
See console output.

Errors:

ERROR: Timeout after 10 minutes
ERROR: Error fetching remote repo 'origin'
stderr: error: RPC failed; result=18, HTTP code = 200
ERROR: Error fetching remote repo 'origin'

phsft-bot · 2017-09-07T20:10:48Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=On
How to customize builds

phsft-bot · 2017-09-07T21:23:00Z

Build failed on mac1012/native.
See console output.

Failing tests:

projectroot.roottest.root.meta.roottest_root_meta_countIncludePaths

Teemperor · 2017-09-08T08:52:41Z

The failing count include test is also failing in master, so this is not a regression: #962 (comment)

Teemperor · 2017-09-08T08:53:14Z

@phsft-bot build with flags -Dclingtest=Off

let's make sure it's not breaking master

phsft-bot · 2017-09-08T08:53:29Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON -Dclingtest=Off
How to customize builds

phsft-bot · 2017-09-08T09:32:31Z

Build failed on centos7/gcc49.
See console output.

Failing tests:

projectroot.tree.treeplayer.test.gtest_tree_treeplayer_test_dataframe_simple

phsft-bot · 2017-09-08T09:38:52Z

Build failed on slc6/gcc62.
See console output.

Failing tests:

projectroot.tree.treeplayer.test.gtest_tree_treeplayer_test_dataframe_simple

vgvassilev · 2017-09-11T08:27:35Z

interpreter/cling/lib/Interpreter/DeclCollector.cpp

+
+  /// \brief Asserts that the given transaction is not null, otherwise prints a
+  /// stack trace to stderr.
+  static void assertHasTransaction(const cling::Transaction* transaction) {


Using just T as a variable name is a common notion for variable names of type cling::Transaction.

vgvassilev · 2017-09-11T08:28:16Z

interpreter/cling/lib/Interpreter/DeclCollector.cpp

+  /// \brief Asserts that the given transaction is not null, otherwise prints a
+  /// stack trace to stderr.
+  static void assertHasTransaction(const cling::Transaction* transaction) {
+    if (transaction == nullptr) {


A more common pattern would be if (!T)

vgvassilev · 2017-09-11T08:29:26Z

interpreter/cling/lib/Interpreter/DeclCollector.cpp

+  static void assertHasTransaction(const cling::Transaction* transaction) {
+    if (transaction == nullptr) {
+      llvm::sys::PrintStackTrace(llvm::errs());
+      assert(false && "Missing transaction during deserialization!");


Shouldn't we use llvm_unreachable here. If so we will need to rename the routine.

The original code was using assert. For me this looks like a poster child of assert: validating a precondition is met.

According to some rules llvm_unreachable is the preferred primitive. I do not have a strong opinion here, but the closer to the llvm coding guidelines we are in cling, the better.

Good points, Vassil. Thanks for explaining!

Axel-Naumann

LGTM!

Axel-Naumann · 2017-09-11T10:50:58Z

interpreter/cling/lib/Interpreter/DeclCollector.cpp

+  static void assertHasTransaction(const cling::Transaction* transaction) {
+    if (transaction == nullptr) {
+      llvm::sys::PrintStackTrace(llvm::errs());
+      assert(false && "Missing transaction during deserialization!");


The original code was using assert. For me this looks like a poster child of assert: validating a precondition is met.

Teemperor · 2017-09-12T09:33:03Z

Added do not merge as Jenkins is restating so this only looks green.

Teemperor · 2017-09-12T10:57:46Z

@phsft-bot build

EDIT: Jenkins is dead...

Teemperor · 2017-09-12T16:47:22Z

@phsft-bot build

Teemperor · 2017-09-13T09:44:00Z

@phsft-bot build

phsft-bot · 2017-09-13T09:44:09Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

phsft-bot · 2017-09-13T11:02:51Z

Build failed on mac1012/native.
See console output.

Failing tests:

phsft-bot · 2017-09-13T11:17:10Z

Build failed on slc6/gcc49.
See console output.

Failing tests:

phsft-bot · 2017-09-13T11:18:38Z

Build failed on slc6/gcc62.
See console output.

Failing tests:

phsft-bot · 2017-09-14T12:09:23Z

Starting build on centos7/gcc49, mac1012/native, slc6/gcc49, slc6/gcc62, ubuntu14/native with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

We will probably see an increasing amount of these failures with C++ modules as we now deserialize all declarations instead of just the PCH ones. To safe us a lot of debugging time on where to push the needed transaction, let's directly print the stack trace here in the rare case that we crash here.

Teemperor requested a review from Axel-Naumann as a code owner September 1, 2017 11:37

Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 319602a to 66f4691 Compare September 1, 2017 13:02

Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 66f4691 to 8b434f2 Compare September 7, 2017 18:18

Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 8b434f2 to 25bc54b Compare September 7, 2017 20:10

Teemperor assigned vgvassilev Sep 7, 2017

vgvassilev reviewed Sep 11, 2017

View reviewed changes

Axel-Naumann approved these changes Sep 11, 2017

View reviewed changes

Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch 2 times, most recently from 964acb8 to cec6da6 Compare September 12, 2017 07:52

Teemperor added the do not merge label Sep 12, 2017

Teemperor removed the do not merge label Sep 14, 2017

Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from cec6da6 to 4d884e9 Compare September 14, 2017 12:09

vgvassilev merged commit 5841454 into root-project:master Sep 20, 2017

Teemperor deleted the MakeMissingTransactionMoreVerbose branch December 19, 2017 08:39

phsft-bot mentioned this pull request Oct 2, 2018

[cxxmodules] Remove workarounds in test. #2733

Merged

phsft-bot mentioned this pull request Jul 4, 2023

[RF] Add methods to create owning RooFit proxies via std::unique_ptr #12924

Merged

[cxxmodules] Print stacktrace before aborting on a missing exception. #921

[cxxmodules] Print stacktrace before aborting on a missing exception. #921

Uh oh!

Conversation

Teemperor commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

phsft-bot commented Sep 1, 2017

Errors:

Uh oh!

Teemperor commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Uh oh!

vgvassilev commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Uh oh!

phsft-bot commented Sep 1, 2017

Failing tests:

Uh oh!

phsft-bot commented Sep 7, 2017

Uh oh!

phsft-bot commented Sep 7, 2017

Failing tests:

Uh oh!

phsft-bot commented Sep 7, 2017

Errors:

Uh oh!

phsft-bot commented Sep 7, 2017

Uh oh!

phsft-bot commented Sep 7, 2017

Failing tests:

Uh oh!

Teemperor commented Sep 8, 2017

Uh oh!

Teemperor commented Sep 8, 2017

Uh oh!

phsft-bot commented Sep 8, 2017

Uh oh!

phsft-bot commented Sep 8, 2017

Failing tests:

Uh oh!

phsft-bot commented Sep 8, 2017

Failing tests:

Uh oh!

vgvassilev Sep 11, 2017

Choose a reason for hiding this comment

Uh oh!

vgvassilev Sep 11, 2017

Choose a reason for hiding this comment

Uh oh!

vgvassilev Sep 11, 2017

Choose a reason for hiding this comment

Uh oh!

Axel-Naumann Sep 11, 2017

Choose a reason for hiding this comment

Uh oh!

vgvassilev Sep 11, 2017

Teemperor commented Sep 12, 2017 •

edited

Loading