Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

Teemperor
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@Teemperor Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 319602a to 66f4691 Compare September 1, 2017 13:02
@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Contributor Author

@phsft-bot build

@phsft-bot
Copy link
Collaborator

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
Copy link
Member

@phsft-bot build with flags -Dclingtest=On

@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

Build failed on mac1012/native.
See console output.

Failing tests:

@Teemperor Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 66f4691 to 8b434f2 Compare September 7, 2017 18:18
@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

Build failed on mac1012/native.
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator

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'

@Teemperor Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from 8b434f2 to 25bc54b Compare September 7, 2017 20:10
@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

Build failed on mac1012/native.
See console output.

Failing tests:

@Teemperor
Copy link
Contributor Author

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

@Teemperor
Copy link
Contributor Author

@phsft-bot build with flags -Dclingtest=Off

let's make sure it's not breaking master

@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

Build failed on centos7/gcc49.
See console output.

Failing tests:

@phsft-bot
Copy link
Collaborator


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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/// \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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A more common pattern would be if (!T)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points, Vassil. Thanks for explaining!

Copy link
Member

@Axel-Naumann Axel-Naumann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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

@Teemperor
Copy link
Contributor Author

Teemperor commented Sep 12, 2017

@phsft-bot build

EDIT: Jenkins is dead...

@Teemperor
Copy link
Contributor Author

@phsft-bot build

1 similar comment
@Teemperor
Copy link
Contributor Author

@phsft-bot build

@phsft-bot
Copy link
Collaborator

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
Copy link
Collaborator

@phsft-bot
Copy link
Collaborator

@phsft-bot
Copy link
Collaborator

@Teemperor Teemperor force-pushed the MakeMissingTransactionMoreVerbose branch from cec6da6 to 4d884e9 Compare September 14, 2017 12:09
@phsft-bot
Copy link
Collaborator

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants