Bug 57847

Summary: Crash logs not generated in 64-bit Windows 7
Product: WebKit Reporter: Pere Martir <pere.martir4>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: ap, aroben, commit-queue, sfalken
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows 7   
Attachments:
Description Flags
Proposed patch
aroben: review-
Proposed patch with ChangeLog none

Description Pere Martir 2011-04-05 07:40:58 PDT
Created attachment 88229 [details]
Proposed patch

run-webkit-tests doesn't find the correct location of ntsd.exe (NT Symbolic Debugger) in 64-bit Windows.

To generate the crash logs, "Debugging Tools for Windows 32-bit Version" must be installed as indicated in http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs

Debugging Tools now comes with Windows SDK and the default installation directory (not likely configurable) in 64-bit Windows is:

  C:\Program Files\Debugging Tools for Windows (x64)

Tools/Scripts/old-run-webkit-tests instead looks for:

  C:\Program Files (x86)/Debugging Tools for Windows (x86)

Note that "the 64-bit versions of Debugging Tools for Windows allow you to debug both 32-bit and 64-bit user-mode applications running on 64-bit processors."  64-bit version of Debugging Tools is also the one installed by Windows SDK.

I've modified old-run-webkit-tests and successfully generated the crash logs.
Comment 1 Alexey Proskuryakov 2011-04-05 10:55:56 PDT
This sounds very interesting!

> http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs

That data should probably be in <http://www.webkit.org/quality/crashlogs.html>. Since WebKit site is in svn, one can just make a patch for it.
Comment 2 Adam Roben (:aroben) 2011-04-05 11:00:32 PDT
Comment on attachment 88229 [details]
Proposed patch

The code change here looks fine. Thanks for fixing the bug!

However, your patch needs a ChangeLog. <http://www.webkit.org/coding/contributing.html> explains how to write one.
Comment 3 Adam Roben (:aroben) 2011-04-05 11:01:17 PDT
(In reply to comment #1)
> This sounds very interesting!
> 
> > http://trac.webkit.org/wiki/BuildingOnWindows#GettingCrashLogs
> 
> That data should probably be in <http://www.webkit.org/quality/crashlogs.html>. Since WebKit site is in svn, one can just make a patch for it.

The wiki page is specifically about getting run-webkit-tests to save crash logs automatically. The webkit.org page isn't specifically about run-webkit-tests. But maybe the info could be added there anyway.
Comment 4 Alexey Proskuryakov 2011-04-05 11:15:59 PDT
What I meant was that the webkit.org page didn't have any information about getting crash logs under Windows 7 at all.
Comment 5 Adam Roben (:aroben) 2011-04-05 11:24:46 PDT
Oh, I see. The webkit.org page should be changed to say "Windows Vista or Windows 7", since the same technique works on both.
Comment 6 Pere Martir 2011-04-06 07:25:57 PDT
Created attachment 88412 [details]
Proposed patch with ChangeLog

Some other points should also be modified in the trac Wiki.

* I wondered if the test script also works in 64-bit since the wiki says very explicitly - installing "32-bit" Debugging Tools.

* The wiki page doesn't mention that the Registry value "ForceQueue" must be set
to 1 so that the crash logs are stored automatically, otherwise some
GUI dialogs will pop up and pause the test script. This is actually mentioned in http://www.webkit.org/quality/crashlogs.html
Comment 7 Adam Roben (:aroben) 2011-04-06 08:02:33 PDT
(In reply to comment #6)
> Some other points should also be modified in the trac Wiki.

Since it's a wiki, feel free to make the modifications yourself.

> * I wondered if the test script also works in 64-bit since the wiki says very explicitly - installing "32-bit" Debugging Tools.

I think we were just mistaken before that the 64-bit tools wouldn't work. (The 32-bit tools are still a good idea to have installed, though, since AFAIK you need the 32-bit tools for gflags to work, e.g.)

> * The wiki page doesn't mention that the Registry value "ForceQueue" must be set
> to 1 so that the crash logs are stored automatically, otherwise some
> GUI dialogs will pop up and pause the test script. This is actually mentioned in http://www.webkit.org/quality/crashlogs.html

The wiki page is all about getting run-webkit-tests's automatic crash log saving to work. Is ForceQueue really needed for that? run-webkit-tests installs a post-mortem debugger, which should prevent the WER dialog from appearing at all.
Comment 8 Adam Roben (:aroben) 2011-04-08 08:47:09 PDT
Pere, if you want your patch to be committed you should mark it cq?.
Comment 9 WebKit Review Bot 2011-04-08 09:04:44 PDT
Comment on attachment 88412 [details]
Proposed patch with ChangeLog

Rejecting attachment 88412 [details] from commit-queue.

pere.martir4@gmail.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 10 Adam Roben (:aroben) 2011-04-08 09:08:25 PDT
Comment on attachment 88412 [details]
Proposed patch with ChangeLog

Non-committers should mark patches cq?. Then a committer can mark it cq+.
Comment 11 WebKit Commit Bot 2011-04-08 10:43:33 PDT
Comment on attachment 88412 [details]
Proposed patch with ChangeLog

Clearing flags on attachment: 88412

Committed r83310: <http://trac.webkit.org/changeset/83310>
Comment 12 WebKit Commit Bot 2011-04-08 10:43:37 PDT
All reviewed patches have been landed.  Closing bug.