Bug 119338 – General GDB backtrace generation for GTK/EFL/Qt

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Zoltan Arvai]

When a crash occurs it would be helpful to get a backtrace generated by the tester bot.
The GTK guys have a nice solution for this:
https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/port/gtk.py#L138

[Gábor Ábrahám]

Created attachment 208915 [details]
draft patch

Solution based on the gtk auto backtrace generating, you should do the next things:

Change /proc/sys/kernel/core_pattern file to:
/path/to/webkit/coredumps/folder/core-pid_%p-_-process_%e

For example in my computer:
/home/abrhm/webkit/WK/coredumps/core-pid_%p-_-process_%e


set the WEBKIT_CORE_DUMPS_DIRECTORY enviromental variable to the same location:

For example in my computer:
export WEBKIT_CORE_DUMPS_DIRECTORY=/home/abrhm/webkit/WK/coredumps/

After this changes if you have debug build and a test crashes, in the results.html you should see the backtrace in the test crashlog.

Unit test needed.

[WebKit Commit Bot]

Attachment 208915 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/Scripts/webkitpy/port/qt.py']" exit_code: 1
Tools/Scripts/webkitpy/port/qt.py:196:  [QtPort._get_gdb_output] Undefined variable 'subprocess'  [pylint/E0602] [5]
Tools/Scripts/webkitpy/port/qt.py:196:  [QtPort._get_gdb_output] Undefined variable 'subprocess'  [pylint/E0602] [5]
Tools/Scripts/webkitpy/port/qt.py:196:  [QtPort._get_gdb_output] Undefined variable 'subprocess'  [pylint/E0602] [5]
Total errors found: 3 in 1 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gábor Ábrahám]

Created attachment 208916 [details]
draft patch fix

Fixed: import the subprocess.

[Gábor Ábrahám]

Created attachment 209657 [details]
draft patch v2

We had a little talk about how to do this change in a clever way. The idea is to avoid the copy-paste code so I did some refactoring, there are some things that are not ready yet. For example, the GTK copyright, changelog, or maybe some more little changes.
So I would like you to check this patch and give me some recommendation about further changes.

[Zan Dobersek]

Comment on attachment 209657 [details]
draft patch v2

Rather than creating a mix-in object from which the Port subclasses then inherit, I'd recommend creating a standalone object which the overriden Port._get_crash_log method instantiates and passes to it all the requirements (the filesystem and the path to the driver, and the required _get_crash_log parameters), returning the acquired crash log.

For instance, the class could be called GdbCrashLogGenerator, and the port which would like to use it would then just implement the _get_crash_log(...):

def _get_crash_log(self, name, pid, stdout, stderr, newer_than):
    return GdbCrashLogGenerator(...).generate()

I think the GdbCrashLogGenerator class (or whatever it will be named) is more in line with how the PulseAudioSanitizer class behaves and is used, so I wouldn't make port classes inherit from it. Plus, when implemented as a standalone object, I think it will be easier to produce a test case for it (there's already one in gtk_unittest.py, so you can take it as the base case).

Also, whatever way you choose to implement it, I think its file should be put in Tools/Scripts/webkitpy/port - the crash generator is not really specific to the layout tests.

[Csaba Osztrogonác]

*** Bug 120150 has been marked as a duplicate of this bug. ***

[Gábor Ábrahám]

Created attachment 210101 [details]
proposed patch

With this changes EFL/GTK bots could easily auto generate backtrace, just like Qt.

[Gábor Ábrahám]

Created attachment 210264 [details]
proposed patch with changelog.

I hope this is the final patch.

[Philippe Normand]

Comment on attachment 210264 [details]
proposed patch with changelog.

That patch is about 3 ports but modifies only one(qt) ?

[Gábor Ábrahám]

Created attachment 210707 [details]
add auto backtrace to efl and gtk

efl.py gtk.py and unittests fixed.
TEST NEEDED ON EFL AND GTK!

[Zan Dobersek]

Comment on attachment 210707 [details]
add auto backtrace to efl and gtk

View in context: https://bugs.webkit.org/attachment.cgi?id=210707&action=review

> Tools/Scripts/webkitpy/port/efl_unittest.py:54
> +    def test_get_crash_log(self):
> +        # This function tested in linux_get_crash_log_unittest.py
> +        pass

This doesn't test anything, so there's no much point for the method to exist.

> Tools/Scripts/webkitpy/port/gtk_unittest.py:86
> +        # This function tested in linux_get_crash_log_unittest.py
> +        pass

Ditto.

> Tools/Scripts/webkitpy/port/qt_unittest.py:130
> +    def test_get_crash_log(self):
> +        # This function tested in linux_get_crash_log_unittest.py
> +        pass

Ditto.

[Gábor Ábrahám]

Without that line Tools/Scripts/test-webkitpy fails.

[Csaba Osztrogonác]

(In reply to comment #11)
> (From update of attachment 210707 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210707&action=review
> 
> > Tools/Scripts/webkitpy/port/efl_unittest.py:54
> > +    def test_get_crash_log(self):
> > +        # This function tested in linux_get_crash_log_unittest.py
> > +        pass
> 
> This doesn't test anything, so there's no much point for the method to exist.
> 
> > Tools/Scripts/webkitpy/port/gtk_unittest.py:86
> > +        # This function tested in linux_get_crash_log_unittest.py
> > +        pass
> 
> Ditto.
> 
> > Tools/Scripts/webkitpy/port/qt_unittest.py:130
> > +    def test_get_crash_log(self):
> > +        # This function tested in linux_get_crash_log_unittest.py
> > +        pass
> 
> Ditto.

Without them efl.test_get_crash_log, gtk.test_get_crash_log and
qt.test_get_crash_log will derived from PortTestCase.test_get_crash_log 
which gives incorrect results for the Linux's GDBCrashLogGenerator.

[Csaba Osztrogonác]

Comment on attachment 210707 [details]
add auto backtrace to efl and gtk

It doesn't apply now, it conflicts with https://trac.webkit.org/changeset/155373.
Could you rebase this patch to ToT, please?

[Gábor Ábrahám]

Created attachment 211307 [details]
rebased patch

Rebased patch.

[Csaba Osztrogonác]

Comment on attachment 211307 [details]
rebased patch

r=me, I'll land it manually to make sure we don't break the GTK debug bot.

[Csaba Osztrogonác]

Comment on attachment 211307 [details]
rebased patch

Clearing flags on attachment: 211307

Committed r155614: <http://trac.webkit.org/changeset/155614>

[Csaba Osztrogonác]

All reviewed patches have been landed.  Closing bug.