781628 - Crashreporter tests broken on symlink builds because JS __LOCATION__ gets dereferenced symbolic link

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[:Irving Reid (No longer working on Firefox)]

+++ This bug was initially created as a clone of Bug #754894 +++

The fix to bug 670514 caused several of the tests in toolkit/crashreporter/test to start failing on symlink builds:

Irvings-MacBook-Pro% make SOLO_FILE="test_crash_purevirtual.js" -C toolkit/crashreporter/test check-one
/usr/bin/python2.7 -u /Users/ireid/tbird/comm-central/mozilla/config/pythonpath.py \
	  -I/Users/ireid/tbird/comm-central/mozilla/build -I../../../_tests/mozbase/mozinfo \
	  /Users/ireid/tbird/comm-central/mozilla/testing/xpcshell/runxpcshelltests.py \
	  --symbols-path=../../../dist/crashreporter-symbols \
	  --build-info-json=../../../mozinfo.json \
	  --test-path=test_crash_purevirtual.js \
	  --profile-name=firefox \
	  --verbose \
	   \
	  /Users/ireid/tbird/firefox-objdir/dist/bin/xpcshell \
	  ../../../_tests/xpcshell/toolkit/crashreporter/test/unit ../../../_tests/xpcshell/toolkit/crashreporter/test/unit_ipc
TEST-INFO | profile dir is /var/folders/13/5v1sbktd1cl05mvkntw3b4gc0000gn/T/firefox/xpcshellprofile
TEST-INFO | /Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js | running test ...
TEST-INFO | /Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js | full command: ['/Users/ireid/tbird/firefox-objdir/dist/bin/xpcshell', '-g', '/Users/ireid/tbird/firefox-objdir/dist/bin', '-a', '/Users/ireid/tbird/firefox-objdir/dist/bin', '-r', '/Users/ireid/tbird/firefox-objdir/dist/bin/components/httpd.manifest', '-m', '-n', '-s', '-e', 'const _HTTPD_JS_PATH = "/Users/ireid/tbird/firefox-objdir/dist/bin/components/httpd.js";', '-e', 'const _HEAD_JS_PATH = "/Users/ireid/tbird/comm-central/mozilla/testing/xpcshell/head.js";', '-f', '/Users/ireid/tbird/comm-central/mozilla/testing/xpcshell/head.js', '-e', 'const _SERVER_ADDR = "localhost"', '-e', 'const _HEAD_FILES = ["/Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/head_crashreporter.js"];', '-e', 'const _TAIL_FILES = [];', '-e', 'const _TEST_FILE = ["/Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js"];', '-e', '_execute_test(); quit(0);']
TEST-INFO | /Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js | current directory: '/Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit'
TEST-INFO | /Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js | environment: ['XPCOM_DEBUG_BREAK=stack-and-abort', 'NS_TRACE_MALLOC_DISABLE_STACKS=1', 'XPCOM_MEM_LEAK_LOG=/var/folders/13/5v1sbktd1cl05mvkntw3b4gc0000gn/T/firefox/xpcshellprofile/runxpcshelltests_leaks.log', 'XPCSHELL_TEST_PROFILE_DIR=/var/folders/13/5v1sbktd1cl05mvkntw3b4gc0000gn/T/firefox/xpcshellprofile', 'MOZ_CRASHREPORTER_NO_REPORT=1', 'DYLD_LIBRARY_PATH=/Users/ireid/tbird/firefox-objdir/dist/bin']
TEST-UNEXPECTED-FAIL | /Users/ireid/tbird/firefox-objdir/_tests/xpcshell/toolkit/crashreporter/test/unit/test_crash_purevirtual.js | test failed (with xpcshell return code: 3), see following log:
>>>>>>>
### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /var/folders/13/5v1sbktd1cl05mvkntw3b4gc0000gn/T/firefox/xpcshellprofile/runxpcshelltests_leaks.log
resource://test/CrashTestUtils.jsm:28: Error: couldn't open library
1967868256[111210140]: WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Users/ireid/tbird/comm-central/mozilla/xpcom/base/nsExceptionService.cpp, line 199
WARNING: nsExceptionService ignoring thread destruction after shutdown: file /Users/ireid/tbird/comm-central/mozilla/xpcom/base/nsExceptionService.cpp, line 199
1967868256[111210140]: WARNING: OOPDeinit() without successful OOPInit(): file /Users/ireid/tbird/comm-central/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp, line 1998
WARNING: OOPDeinit() without successful OOPInit(): file /Users/ireid/tbird/comm-central/mozilla/toolkit/crashreporter/nsExceptionHandler.cpp, line 1998
nsStringStats
 => mAllocCount:           1962
 => mReallocCount:          338
 => mFreeCount:            1962
 => mShareCount:           7591
 => mAdoptCount:             57
 => mAdoptFreeCount:         57
<<<<<<<
INFO | Result summary:
INFO | Passed: 0
INFO | Failed: 1
INFO | Todo: 0
make: *** [check-one] Error 1


I'll attach my improvement to the error handling in CrashTestUtils.jsm that helped me debug the code, but the short answer is that the behaviour of the JavaScript __LOCATION__ property changed so that it now contains the original location of the file instead of the symlink path, and the test case relies on finding a compiled dynamic library in the directory containing the symlink.

Comment 1

[:Irving Reid (No longer working on Firefox)]

Attachment: Improve error handling to display the full path of the library that failed to load

Catch the jsctypes library load failure and add the library file path to the exception.

It might be better to put the path in the exception thrown by the jsctypes library rather than doing it here.

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 650744 [details] [diff] [review]
Improve error handling to display the full path of the library that failed to load

In and of itself this patch is fine, but it's not actually going to fix the test, right?

This behavior change is kind of surprising. The module gets loaded via a resource: URI which I suppose turns into a file: URI under the hood? Dereferencing the symlink there doesn't seem like the right behavior (but I don't have time to read and understand the subtleties of that bug).

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Irving Reid (:irving) from comment #1)
> It might be better to put the path in the exception thrown by the jsctypes
> library rather than doing it here.

This sounds useful, you should file it under js-ctypes if it's not already filed.

Comment 4

[:Irving Reid (No longer working on Firefox)]

(In reply to Ted Mielczarek [:ted] from comment #3)
> (In reply to Irving Reid (:irving) from comment #1)
> > It might be better to put the path in the exception thrown by the jsctypes
> > library rather than doing it here.
> 
> This sounds useful, you should file it under js-ctypes if it's not already
> filed.

Bug 782471 with patch filed against js-ctypes

Comment 5

[:Irving Reid (No longer working on Firefox)]

(In reply to Ted Mielczarek [:ted] from comment #2)
> In and of itself this patch is fine, but it's not actually going to fix the
> test, right?

Right, just wanted to make someone aware that the change had caused a problem with your module

> This behavior change is kind of surprising. The module gets loaded via a
> resource: URI which I suppose turns into a file: URI under the hood?
> Dereferencing the symlink there doesn't seem like the right behavior (but I
> don't have time to read and understand the subtleties of that bug).

Not sure if the JS interpreter's __LOCATION__ attribute has the same kind of security issues as URL references, so I'll leave it up to the experts.

Comment 6

[(not currently active) Ted Mielczarek]

I'm going to fix this in a patch on bug 733501. Thanks for the bug!