src: print backtrace on fatal error by bnoordhuis · Pull Request #6734 · nodejs/node · GitHub

Print a C backtrace on fatal errors to make it easier to debug issues
like #6727.

CI: https://ci.nodejs.org/job/node-test-pull-request/2629/

#include "src/base/logging.h" works too, at least locally for me. Any particular reason for including the source file itself?

And CI is failing on smartos with ld: fatal: symbol 'v8::base::DumpBacktrace()' is multiply-defined: https://ci.nodejs.org/job/node-test-commit-smartos/2478/nodes=smartos14-32/console

I included the .cc so it works with shared V8 builds (think Fedora, Debian, Ubuntu, etc.,or linking against a .so.)

I'll ifdef out smartos. That user base is a rounding error compared to the aforementioned distros.

Fix-up: https://ci.nodejs.org/job/node-test-pull-request/2630/

LGTM if CI is green

Oh my gosh, +1000

Looks like plinux doesn't like it.

In file included from ../deps/v8/src/base/logging.h:12:0,
                 from ../deps/v8/src/base/logging.cc:5,
                 from ../src/backtrace.cc:7:
../deps/v8/src/base/build_config.h:102:2: error: #error Target architecture was not detected as supported by v8
 #error Target architecture was not detected as supported by v8
  ^
../deps/v8/src/base/build_config.h:140:2: error: #error Unknown target architecture pointer size
 #error Unknown target architecture pointer size
  ^
../deps/v8/src/base/build_config.h:202:2: error: #error Unknown target architecture endianness
 #error Unknown target architecture endianness
  ^
make[2]: *** [/home/iojs/build/workspace/node-test-commit-plinux/nodes/ppcbe-ubuntu1404/out/Release/obj.target/node/src/backtrace.o] Error 1

LGTM, but yea, was just about to say what @Fishrock123 did.

/cc @mhdawson re plinux error

Different approach that should work on all platforms: https://ci.nodejs.org/job/node-test-pull-request/2637/

@bnoordhuis Looks like that failed on smartos:

../src/backtrace_posix.cc
../src/backtrace_posix.cc: In function 'void node::DumpBacktrace(std::FILE*)':
../src/backtrace_posix.cc:20:47: error: invalid conversion from 'const void*' to 'void*' [-fpermissive]
     const bool have_info = dladdr(frame, &info);
                                               ^
In file included from ../src/backtrace_posix.cc:4:0:
/usr/include/dlfcn.h:115:12: error:   initializing argument 1 of 'int dladdr(void*, Dl_info*)' [-fpermissive]
 extern int dladdr(void *, Dl_info *);
            ^
node.target.mk:186: recipe for target '/home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos14-32/out/Release/obj.target/node/src/backtrace_posix.o' failed

God, smartos is like a time machine back to the '90s. Okay, loosened the const-ness. CI: https://ci.nodejs.org/job/node-test-pull-request/2638/

@bnoordhuis smartos still 🚒

Looks like something went wrong. Again: https://ci.nodejs.org/job/node-test-pull-request/2639/

Finally, green except for flaky parallel/test-debug-port-cluster on freebsd.

Works on my machine. This might be a silly question, but does the CI actually exercise this at all?

Not intentionally, I don't think. I thought about adding a test but it would be rather anti-social when core dumps are enabled.

@bnoordhuis I added a minimal test in cjihrig@3ce55fa. The abort tests aren't run anywhere (that I know of, but it would be nice to have at least something that can test it. Thoughts?

@cjihrig Thanks, added to the PR. Anyone wants to take one more quick look?

I guess the test should be skipped on Windows? Other than that still LGTM.

Eh, the RE should probably start with ^\s* and end with $, otherwise the optional parts here are pointless

Updated the test, PTAL. CI: https://ci.nodejs.org/job/node-test-pull-request/2741/

LGTM if CI is green

LGTM. Seems to be an issue with the CI though.

New CI: https://ci.nodejs.org/job/node-test-pull-request/2874/

`python tools/test.py abort` won't work without one. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

The --abort-on-uncaught-exception can terminate the process with either a SIGABRT or a SIGILL signal but the test only expected SIGABRT. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

There is no real need and it causes endless grief on Windows with some of the upcoming changes. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Print a C backtrace on fatal errors to make it easier to debug issues like #6727. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Print a C backtrace on fatal errors to make it easier to debug issues. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

This commit adds a test that validates backtraces which are printed on fatal errors. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Don't inline calls to node::DumpBacktrace() and fflush(), it makes the generated code bigger. A secondary benefit of moving it to a function is that it gives you something to put a breakpoint on. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Print a C backtrace on fatal errors to make it easier to debug issues. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

@bnoordhuis should this be backported?

Ideally, yes.

due to the AIX + compiler failures I'm going to hold off on this change for the v4.5.0 release

@bnoordhuis would you be willing to backport this PR along with #7508, #7544, and any other potential commits that need to land with this?

ping @bnoordhuis sorry about the other message. Would you be willing to backport a working version of this as mentioned above

There is no real need and it causes endless grief on Windows with some of the upcoming changes. PR-URL: nodejs#6734 Backport-PR-URL: nodejs#16550 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

There is no real need and it causes endless grief on Windows with some of the upcoming changes. PR-URL: #6734 Backport-PR-URL: #16550 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

bnoordhuis added the lib / src Issues and PRs related to general changes in the lib or src directory. label May 13, 2016

nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label May 13, 2016

addaleax reviewed May 13, 2016
View reviewed changes

Fishrock123 added the lts-watch-v4.x label May 13, 2016

bnoordhuis force-pushed the print-backtrace-on-fatal-error branch from 887c8cb to 9c63535 Compare May 13, 2016 16:55

bnoordhuis force-pushed the print-backtrace-on-fatal-error branch from 4ab11e0 to f15eb7b Compare May 19, 2016 10:14

addaleax reviewed May 19, 2016
View reviewed changes

bnoordhuis force-pushed the print-backtrace-on-fatal-error branch from f15eb7b to e52e6d2 Compare May 23, 2016 10:57

bnoordhuis mentioned this pull request May 30, 2016

Discussion: Interest in segfault-handler #7059

Closed

Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016

test: add testcfg.py to test/abort/

4b0ab5b

`python tools/test.py abort` won't work without one. PR-URL: #6734 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Fishrock123 mentioned this pull request Jul 5, 2016

Propose v6.3.0 (v2) #7550

Merged

MylesBorins mentioned this pull request Jul 14, 2016

build: enable big toc for release builds in AIX #7508

Closed

3 tasks

gibfahn mentioned this pull request Aug 2, 2016

test: remove internal headers from addons #7947

Closed

2 tasks

MylesBorins mentioned this pull request Sep 30, 2016

src: Only use the TR1 type_traits when targeting OSX<10.9 #7778

Closed

4 tasks

bnoordhuis mentioned this pull request Oct 11, 2016

src: unbreak build when compiling against uclibc #8308

Merged

gibfahn mentioned this pull request Oct 27, 2017

[v4.x backport] Use external headers in addons tests #16550

Closed

2 tasks

src: print backtrace on fatal error by bnoordhuis · Pull Request #6734 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: print backtrace on fatal error - #6734

src: print backtrace on fatal error#6734bnoordhuis merged 8 commits intonodejs:masternodejs/node:masterfrom bnoordhuis:print-backtrace-on-fatal-errorbnoordhuis/io.js:print-backtrace-on-fatal-errorCopy head branch name to clipboard

Conversation

bnoordhuis commented May 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax May 13, 2016

Choose a reason for hiding this comment

Uh oh!

addaleax May 13, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis May 13, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented May 13, 2016

Uh oh!

addaleax commented May 13, 2016

Uh oh!

Fishrock123 commented May 13, 2016

Uh oh!

Fishrock123 commented May 13, 2016

Uh oh!

cjihrig commented May 13, 2016

Uh oh!

MylesBorins commented May 13, 2016

Uh oh!

bnoordhuis commented May 13, 2016

Uh oh!

Fishrock123 commented May 13, 2016

Uh oh!

bnoordhuis commented May 13, 2016

Uh oh!

Fishrock123 commented May 13, 2016

Uh oh!

bnoordhuis commented May 13, 2016

Uh oh!

bnoordhuis commented May 13, 2016

Uh oh!

cjihrig commented May 16, 2016

Uh oh!

bnoordhuis commented May 16, 2016

Uh oh!

cjihrig commented May 16, 2016

Uh oh!

bnoordhuis commented May 19, 2016

Uh oh!

addaleax commented May 19, 2016

Uh oh!

addaleax May 19, 2016

Choose a reason for hiding this comment

Uh oh!

bnoordhuis commented May 23, 2016

Uh oh!

addaleax commented May 23, 2016

Uh oh!

cjihrig commented May 23, 2016

Uh oh!

cjihrig commented May 31, 2016

Uh oh!

MylesBorins commented Jul 11, 2016

Uh oh!

bnoordhuis commented Jul 11, 2016

Uh oh!

MylesBorins commented Jul 14, 2016

Uh oh!

MylesBorins commented Aug 30, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

src: print backtrace on fatal error#6734
bnoordhuis merged 8 commits into
nodejs:masterfrom
bnoordhuis:print-backtrace-on-fatal-error

bnoordhuis commented May 13, 2016 •
edited
Loading