75387 – Get GDB to display a proper backtrace through JITted functions

UNCONFIRMED75387

Get GDB to display a proper backtrace through JITted functions

https://bugs.webkit.org/show_bug.cgi?id=75387

Summary Get GDB to display a proper backtrace through JITted functions

Sanjoy Das

Reported 2011-12-30 08:10:53 PST

Currently GDB cannot unwind through call frames of JS functions JIT compiled by JSCore. One new way to fix this is to supply GDB with a plugin that knows how to unwind through such call frames. The forthcoming patch adds such a plugin (in `Tools/gdb'), JSCPlugin.so and modifies JSCore to work with the plugin. The patches should work with GDB 7.4 and above. The plugin can be loaded using the `jit-reader-load', once it is copied to `$(installdir)/lib/gdb'. ENABLE_GDB_JIT_INTEGRATION in JIT.h needs to be set to 1.

Attachments
Proposed patch (24.23 KB, patch) 2011-12-30 12:50 PST, Sanjoy Das	webkit-ews: commit-queue-	Details Formatted Diff Diff
Add a webkit style checker exception for GDBInterface.cpp (1.84 KB, patch) 2012-01-12 00:21 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Add a check-webkit-style exception for GDBInterface.cpp (1.84 KB, patch) 2012-01-12 00:41 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Add a check-webkit-style exception for GDBInterface.cpp (1.84 KB, patch) 2012-01-12 00:52 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Proposed patch (26.89 KB, patch) 2012-01-12 09:35 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Proposed patch (26.97 KB, patch) 2012-01-14 03:08 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Proposed patch (26.88 KB, patch) 2012-01-24 10:23 PST, Sanjoy Das	fpizlo: review- webkit-ews: commit-queue-	Details Formatted Diff Diff
Proposed patch (27.55 KB, patch) 2012-01-28 01:43 PST, Sanjoy Das	no flags	Details Formatted Diff Diff
Proposed patch (24.16 KB, patch) 2012-03-27 20:00 PDT, Sanjoy Das	buildbot: commit-queue-	Details Formatted Diff Diff
Proposed patch (24.12 KB, patch) 2012-03-30 01:25 PDT, Sanjoy Das	buildbot: commit-queue-	Details Formatted Diff Diff
Proposed patch (24.20 KB, patch) 2012-03-30 19:13 PDT, Sanjoy Das	fpizlo: review- fpizlo: commit-queue-	Details Formatted Diff Diff
Show Obsolete (10) View All Add attachment proposed patch, testcase, etc.

Sanjoy Das

Comment 1 2011-12-30 12:50:17 PST

Created attachment 120814 [details] Proposed patch

WebKit Review Bot

Comment 2 2012-01-08 09:02:10 PST

Attachment 120814 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/jit/GDBInterface.cpp:64: __attribute__ is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/JavaScriptCore/jit/GDBInterface.cpp:66: __jit_debug_descriptor is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 2 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.

Early Warning System Bot

Comment 3 2012-01-08 09:16:21 PST

Comment on attachment 120814 [details] Proposed patch Attachment 120814 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11113203

Gyuyoung Kim

Comment 4 2012-01-08 09:17:41 PST

Comment on attachment 120814 [details] Proposed patch Attachment 120814 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11166233

Sanjoy Das

Comment 5 2012-01-08 09:55:54 PST

GDB needs to see both these symbols as they are so they can't be renamed. Is there some way I can tell check-webkit-style to not check a particular file (or a particular region in a file)?

Philippe Normand

Comment 6 2012-01-09 01:28:20 PST

(In reply to comment #5) > GDB needs to see both these symbols as they are so they can't be renamed. > > Is there some way I can tell check-webkit-style to not check a particular file (or a particular region in a file)? I'd suggest to add a new rule in the _BASE_FILTER_RULES variable of Tools/Scripts/webkitpy/style/checker.py. Can you do that in a separate patch please? Also don't forget to set the r? and cq? flags on the patches :)

Sanjoy Das

Comment 7 2012-01-12 00:21:06 PST

Created attachment 122176 [details] Add a webkit style checker exception for GDBInterface.cpp I modified _PATH_RULES_SPECIFIER and added an exception for GDBInterface.cpp

WebKit Review Bot

Comment 8 2012-01-12 00:24:32 PST

Attachment 122176 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/style/checker.py:232: whitespace before ']' [pep8/E202] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sanjoy Das

Comment 9 2012-01-12 00:41:52 PST

Created attachment 122181 [details] Add a check-webkit-style exception for GDBInterface.cpp Fix the pep8/E202 style error in the previous patch.

WebKit Review Bot

Comment 10 2012-01-12 00:44:55 PST

Attachment 122181 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/style/checker.py:232: whitespace before ']' [pep8/E202] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sanjoy Das

Comment 11 2012-01-12 00:52:08 PST

Created attachment 122185 [details] Add a check-webkit-style exception for GDBInterface.cpp

Sanjoy Das

Comment 12 2012-01-12 09:35:26 PST

Created attachment 122261 [details] Proposed patch The style errors this patch has are fixed by https://bugs.webkit.org/show_bug.cgi?id=76187. This version adds the new files to all the build systems, so there should hopefully be no more build errors.

Raphael Kubo da Costa (:rakuco)

Comment 13 2012-01-12 19:35:49 PST

Comment on attachment 122261 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=122261&action=review > Source/JavaScriptCore/CMakeLists.txt:102 > + jit/GDBInterface.cpp Please keep this list alphabetically sorted (that may apply to the other buildsystems touched by this patch as well).

Sanjoy Das

Comment 14 2012-01-14 03:08:23 PST

Created attachment 122538 [details] Proposed patch The new files added to the build scripts are now in sorted order.

Sanjoy Das

Comment 15 2012-01-24 10:23:43 PST

Created attachment 123762 [details] Proposed patch Older patch with edits to make sure it applies cleanly and minor cosmetic changes.

Early Warning System Bot

Comment 16 2012-01-24 11:45:28 PST

Comment on attachment 123762 [details] Proposed patch Attachment 123762 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11342020

Filip Pizlo

Comment 17 2012-01-24 12:14:34 PST

Comment on attachment 123762 [details] Proposed patch I like this a lot. Two issues: 1) Fix the build on Qt and Windows. Seems like Qt doesn't like your use of void*, which should be easy to fix with some static_cast's: ../../../../Source/JavaScriptCore/jit/GDBInterface.cpp:79: error: pointer of type 'void *' used in arithmetic And Windows is just complaining because the code using gcc/clang specific stuff; probably best to put some #if !OS(WINDOWS) or similar around the relevant code. 2) Would it be possible for you to make the JSCPlugin code use something other than a makefile? For example, that makefile would not work on Mac. But perhaps autotools would. Marking r- because of the Qt and Windows build. Otherwise I think it's probably good to go.

Sanjoy Das

Comment 18 2012-01-28 01:43:35 PST

Created attachment 124434 [details] Proposed patch The void * error has been fixed by adding proper casts. Changed the __attribute__((noinline)) to NEVER_INLINE, so this should now work on Windows. jscplugin now uses scons, so it should build successfully on OSX. I don't have access to an OSX installation so I could not test this completely, but it builds and works as expected on Debian. I've used LoadableModule instead of SharedLibrary, which I think is the correct method on OSX.

Sanjoy Das

Comment 19 2012-02-08 08:09:04 PST

Does the current patch look good?

Filip Pizlo

Comment 20 2012-02-14 10:10:38 PST

(In reply to comment #19) > Does the current patch look good? Apologies for not looking at this; I've been out of town and mostly AFK. I will look.

Andy Wingo

Comment 21 2012-03-09 09:51:36 PST

The patch does not currently apply. Sanjoy, would you mind rebasing it? Thanks!

Sanjoy Das

Comment 22 2012-03-27 20:00:33 PDT

Created attachment 134207 [details] Proposed patch Sorry for the delay, have attached a patch that applies correctly.

WebKit Review Bot

Comment 23 2012-03-27 20:07:02 PDT

Attachment 134207 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/CMakeLists.txt', u'S..." exit_code: 1 Source/JavaScriptCore/runtime/Executable.cpp:32: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 15 files If any of these errors are false positives, please file a bug against check-webkit-style.

Build Bot

Comment 24 2012-03-27 20:46:44 PDT

Comment on attachment 134207 [details] Proposed patch Attachment 134207 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12148778

Philippe Normand

Comment 25 2012-03-28 00:07:16 PDT

In bug 81659 I'm trying to add gdb support to run-webkit-tests --gtk. It'd be great if at some point I can enable this plugin as well for the crash logs reporting.

Sanjoy Das

Comment 26 2012-03-30 01:25:36 PDT

Created attachment 134742 [details] Proposed patch

Build Bot

Comment 27 2012-03-30 01:41:56 PDT

Comment on attachment 134742 [details] Proposed patch Attachment 134742 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12288106

Sanjoy Das

Comment 28 2012-03-30 19:13:48 PDT

Created attachment 134931 [details] Proposed patch

Filip Pizlo

Comment 29 2013-10-31 11:37:46 PDT

Comment on attachment 134931 [details] Proposed patch We're moving onto the C stack, so this looks like it's no longer relevant.

Note You need to log in before you can comment on or make changes to this bug.

Status UNCONFIRMED

Resolution

Priority P3

Severity Enhancement

Classification Unclassified

Version 528+ (Nightly build)

Hardware PC

OS Linux

Product WebKit

Component JavaScriptCore

Assignee

Nobody

Reported

2011-12-30 08:10 PST

Modified

2017-07-18 08:29 PDT History

CC List

14 users Show

URL

Keywords

Depends on

Blocks