WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
76935
[Windows] NRWT doesn't save crash logs on Apple's Windows port
https://bugs.webkit.org/show_bug.cgi?id=76935
Summary
[Windows] NRWT doesn't save crash logs on Apple's Windows port
Adam Roben (:aroben)
Reported
2012-01-24 13:00:04 PST
NRWT doesn't save crash logs on Windows like ORWT does. This is an important thing to support before we switch our bots over to NRWT.
Attachments
Patch
(7.09 KB, patch)
2013-05-30 12:00 PDT
,
Brent Fulgham
no flags
Details
Formatted Diff
Diff
Patch
(7.33 KB, patch)
2013-05-30 17:49 PDT
,
Brent Fulgham
rniwa
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2012-01-24 13:00:20 PST
<
rdar://problem/10747585
>
Brent Fulgham
Comment 2
2013-05-24 17:03:49 PDT
See 'setUpWindowsCrashLogSaving' in the ORWT Perl script to see what needs to be done here. Note: Windows 8 takes long strides to make this even more impossible. We might need to require that test bots have the registry key permissions changes to permit regular users to change this particular value.
Brent Fulgham
Comment 3
2013-05-30 12:00:55 PDT
Created
attachment 203370
[details]
Patch
Brent Fulgham
Comment 4
2013-05-30 12:09:29 PDT
To encourage Windows 7 and Windows 8 to allow the registry keys to be modified, you need to do the following: 1. Since we are running in 32-bit mode, the actual key to be modified is "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows NT\CurrentVersion\AeDebug" This is the location we are touching using the "regtool --wow32" command. 2. Adjust the security settings as follows: (a) Launch regedit as an Administrator. (b) Navigate to "HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows NT\CurrentVersion\AeDebug". (c) Right-click on this node, and select 'Permissions' (d) Make sure that the "Users" group is given Full Control over this key. This permission adjustment is needed to make sure that Windows will allow the script to flip the desired debugger from the ntsd.exe program back to whatever the user wanted originally.
Ryosuke Niwa
Comment 5
2013-05-30 12:21:26 PDT
Comment on
attachment 203370
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=203370&action=review
Looks okay but needs to use more webkitpy abstractions to be consistent.
> Tools/Scripts/webkitpy/port/win.py:141 > + possible_paths = [self._filesystem.join(os.environ['PROGRAMFILES'], "Windows Kits", "8.0", "Debuggers", "x64", "ntsd.exe"), > + self._filesystem.join(os.environ['PROGRAMFILES'], "Windows Kits", "8.0", "Debuggers", "x86", "ntsd.exe"), > + self._filesystem.join(os.environ['PROGRAMFILES'], "Debugging Tools for Windows (x86)", "ntsd.exe"), > + self._filesystem.join(os.environ['ProgramW6432'], "Debugging Tools for Windows (x64)", "ntsd.exe"), > + self._filesystem.join(os.environ['SYSTEMROOT'], "system32", "ntsd.exe")]
Nit: wrong indentation. self. should appear exactly 4 spaces to the right of possible.
> Tools/Scripts/webkitpy/port/win.py:157 > + with open(command_file, "w") as text_file: > + text_file.write(".logopen /t \"%s\\CrashLog%s.txt\"\n" % (cygpath(self.results_directory()), self.CRASH_LOG_PREFIX)) > + text_file.write(".srcpath \"%s\"\n" % cygpath(os.environ['WEBKIT_SOURCE'])) > + text_file.write("!analyze -vv\n") > + text_file.write("~*kpn\n") > + text_file.write("q\n")
Please use self._filesystem.write_text_file. cygpath(os.environ['WEBKIT_SOURCE']) should be read off of WebKitFinder.webkit_base.
> Tools/Scripts/webkitpy/port/win.py:162 > + value = subprocess.Popen(["regtool", "--wow32", "get", registry_key], bufsize=1024, stdout=subprocess.PIPE).communicate()[0]
Use webkitpy.common.system.executive.
> Tools/Scripts/webkitpy/port/win.py:174 > + rc = subprocess.call(["regtool", "--wow32", "set", "-s", str(registry_key), str(value)]) > + if rc == 2: > + rc = subprocess.call(["regtool", "--wow32", "add", "-s", str(registry_key)]) > + if rc == 0: > + rc = subprocess.call(["regtool", "--wow32", "set", "-s", str(registry_key), str(value)]) > + if rc: > + _log.warn("Error setting key: %s to value %s. Error=%ld." % (key, value, rc)) > + return False
Ditto.
> Tools/Scripts/webkitpy/port/win.py:190 > + ntsd_path = self._ntsd_location() > + if None == ntsd_path:
Do: not ntsd_path.
> Tools/Scripts/webkitpy/port/win.py:199 > + command_file = self.create_debugger_command_file() > + if None == command_file:
Ditto.
> Tools/Scripts/webkitpy/port/win.py:201 > + debugger_options = "\"" + cygpath(ntsd_path) + "\" -p %ld -e %ld -g -lines -cf \"" + cygpath(command_file) + "\""
Why not '"%s" -p %ld -e %ld -g -lines -cf "%s"' % (cygpath(ntsd_path), cygpath(command_file))?
Brent Fulgham
Comment 6
2013-05-30 17:47:43 PDT
Comment on
attachment 203370
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=203370&action=review
>> Tools/Scripts/webkitpy/port/win.py:141 >> + self._filesystem.join(os.environ['SYSTEMROOT'], "system32", "ntsd.exe")] > > Nit: wrong indentation. self. should appear exactly 4 spaces to the right of possible.
Done.
>> Tools/Scripts/webkitpy/port/win.py:157 >> + text_file.write("q\n") > > Please use self._filesystem.write_text_file. cygpath(os.environ['WEBKIT_SOURCE']) should be read off of WebKitFinder.webkit_base.
Great! I didn't know about WebKitFinder. That's very useful.
>> Tools/Scripts/webkitpy/port/win.py:162 >> + value = subprocess.Popen(["regtool", "--wow32", "get", registry_key], bufsize=1024, stdout=subprocess.PIPE).communicate()[0] > > Use webkitpy.common.system.executive.
Done
>> Tools/Scripts/webkitpy/port/win.py:174 >> + return False > > Ditto.
Done
>> Tools/Scripts/webkitpy/port/win.py:190 >> + if None == ntsd_path: > > Do: not ntsd_path.
Done
>> Tools/Scripts/webkitpy/port/win.py:199 >> + if None == command_file: > > Ditto.
Done
>> Tools/Scripts/webkitpy/port/win.py:201 >> + debugger_options = "\"" + cygpath(ntsd_path) + "\" -p %ld -e %ld -g -lines -cf \"" + cygpath(command_file) + "\"" > > Why not '"%s" -p %ld -e %ld -g -lines -cf "%s"' % (cygpath(ntsd_path), cygpath(command_file))?
Yes, that's much cleaner. I had to use the new string format syntax to avoid having to escape all of the other format specifiers (e.g., the '%ld' tokens) that are meant to be passed to the external utility.
Brent Fulgham
Comment 7
2013-05-30 17:49:13 PDT
Created
attachment 203403
[details]
Patch
Ryosuke Niwa
Comment 8
2013-05-30 18:45:15 PDT
Comment on
attachment 203403
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=203403&action=review
> Tools/Scripts/webkitpy/port/win.py:33 > +import subprocess
We shouldn't be importing this.
> Tools/Scripts/webkitpy/port/win.py:35 > +import tempfile
We shouldn't include this.
> Tools/Scripts/webkitpy/port/win.py:150 > + debugger_temp_directory = tempfile.mkdtemp()
Use filesystem.mkdtemp
Brent Fulgham
Comment 9
2013-05-30 20:31:31 PDT
Committed
r151004
: <
http://trac.webkit.org/changeset/151004
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug