Closed Bug 298730 Opened 20 years ago Closed 20 years ago

ASSERTION: unexpected target property value: '!JSVAL_IS_PRIMITIVE(*vp)

Categories

(Core :: JavaScript Engine, defect)

1.7 Branch
x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bc, Assigned: brendan)

Details

(Keywords: fixed-aviary1.0.5, fixed1.7.9, regression)

Attachments

(1 file)

I first started seeing this assertion on 6/22 on the 1.0.5 branch only in windows. ###!!! ASSERTION: unexpected target property value: '!JSVAL_IS_PRIMITIVE(*vp)', file ./mozilla/caps/src/nsScriptSecurityManager.cpp, line 458 nsScriptSecurityManager::CheckObjectAccess(JSContext * 0x00f0eec8, JSObject * 0x02d4abc8, long 47490132, JSAccessMode JSACC_WATCH, long * 0x0012f144) line 458 + 46 bytes js_CheckAccess(JSContext * 0x00f0eec8, JSObject * 0x02d4abc8, long 16345400, int 3, long * 0x0012f144, unsigned int * 0x0012f140) line 3413 + 50 bytes js_Interpret(JSContext * 0x00f0eec8, long * 0x0012f260) line 3954 + 48 bytes js_Execute(JSContext * 0x00f0eec8, JSObject * 0x02d4a220, JSScript * 0x00fbc8a8, JSStackFrame * 0x00000000, unsigned int 0, long * 0x0012f38c) line 1173 + 13 bytes JS_ExecuteScript(JSContext * 0x00f0eec8, JSObject * 0x02d4a220, JSScript * 0x00fbc8a8, long * 0x0012f38c) line 3540 + 25 bytes mozJSComponentLoader::GlobalForLocation(const char * 0x00fb0d98, nsIFile * 0x02c9ada8) line 1120 + 27 bytes mozJSComponentLoader::ModuleForLocation(const char * 0x00fb0d98, nsIFile * 0x02c9ada8) line 913 + 19 bytes mozJSComponentLoader::AttemptRegistration(nsIFile * 0x02c9ada8, int 0) line 749 + 24 bytes mozJSComponentLoader::AutoRegisterComponent(mozJSComponentLoader * const 0x00fb0e70, int 0, nsIFile * 0x02c9ada8, int * 0x0012f680) line 675 + 14 bytes mozJSComponentLoader::RegisterComponentsInDir(int 0, nsIFile * 0x003e8380) line 583 + 24 bytes mozJSComponentLoader::AutoRegisterComponents(mozJSComponentLoader * const 0x00fb0e70, int 0, nsIFile * 0x003e8380) line 540 nsComponentManagerImpl::AutoRegisterNonNativeComponents(nsIFile * 0x003e8380) line 3280 + 47 bytes nsComponentManagerImpl::AutoRegisterImpl(int 0, nsIFile * 0x00000000, int 1) line 3250 + 20 bytes nsComponentManagerImpl::AutoRegister(nsComponentManagerImpl * const 0x003eeca8, nsIFile * 0x00000000) line 3422 + 19 bytes NS_InitXPCOM2(nsIServiceManager * * 0x0012fb04, nsIFile * 0x003e7740, nsIDirectoryServiceProvider * 0x0012ff40) line 603 ScopedXPCOMStartup::Initialize() line 795 + 29 bytes xre_main(int 1, char * * 0x003e7678, const nsXREAppData * 0x0041e01c kAppData) line 1783 + 11 bytes main(int 1, char * * 0x003e7678) line 58 + 18 bytes id 16345400 (0x00F96938) is val 47490132 (0x02D4A454) = string (0x02D4A450) "operation" Johnny says that js_CheckAccess should only be called for a limited number of properties like __proto__ etc and not for things like this.
Flags: blocking-aviary1.0.5?
To reproduce, just start the browser.
likely a regression from bug 296397
Brendan says set XPCOM_DEBUG_BREAK=warn, we aren't going to fix this for 1.0.5
Flags: blocking-aviary1.0.5? → blocking-aviary1.0.5-
How many times does the assertion fire? If it's firing a lot then it might indicate we're checking a lot more than we intend to--if so the patch for bug 296397 might not be doing the right thing. Otherwise we can just fix an innocuous warning assertion on the trunk. Basically it comes down to the question "In the future 'assertions are fatal' world, would this one be left as an assertion or changed to a warning?" If it's just a warning we don't need to rush a fix for the stable branch.
Flags: blocking-aviary1.0.5- → blocking-aviary1.0.5?
Flags: blocking-aviary1.0.5? → blocking-aviary1.0.5-
Re: comment 2: Actually this is not a regression from bug 296397 -- jst just removed the assertion on the trunk. I can do the same on the branch if someone wants to approve that. /be
Brendan, I thought we fixed the JS engine also to always get the value of the property we're checking for before doing the check. That should make us not hit this assertion, or was that done only for __parent__ and __proto__ or something?
(In reply to comment #4) > How many times does the assertion fire? 24 times on startup and for the occasional page. You can see the most recent log at <http://test.bclary.com/results/2005-06-30-firefox-1.0.x-dbg-01.log>
Flags: blocking-aviary1.0.5- → blocking-aviary1.0.5?
(In reply to comment #6) > Brendan, I thought we fixed the JS engine also to always get the value of the > property we're checking for before doing the check. No, we didn't need or want that cost -- getters are not idempotent, or even necessarily cheap, etc. In this case, some JS object initialisers are using get and set syntax to declare getters and setters, so there is no such property before the first (usually getter) is declared. > That should make us not hit > this assertion, or was that done only for __parent__ and __proto__ or something? Only for proto and parent, based on JSACC_PROTO/PARENT, and using the directly addressed object, not the prototype object in which the property was found, cuz __proto__ and __parent__ are magic. Again, you (how soon we forget ;-) removed the bogo-assertion in trunk rev 1.261 of nsScriptSecurityManager.cpp. We could put that change on the branch. All I need is the word from jay. /be
Here's the diff. /be
Attachment #187901 - Flags: superreview+
Attachment #187901 - Flags: review+
Attachment #187901 - Flags: approval1.7.9?
Attachment #187901 - Flags: approval-aviary1.0.5?
Comment on attachment 187901 [details] [diff] [review] jst's trunk patch to rid us of this irritation Yes, let's get that patch in. a=jay
Attachment #187901 - Flags: approval1.7.9?
Attachment #187901 - Flags: approval1.7.9+
Attachment #187901 - Flags: approval-aviary1.0.5?
Attachment #187901 - Flags: approval-aviary1.0.5+
Fixed on branches. /be
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Flags: blocking-aviary1.0.5? → blocking-aviary1.0.5+
Removing keywords until we know this made it onto the suite. Brendan: I don't see a checkin for this on the 1.7 branch, can you take a look and get that in asap? If it's already in, please return the fixed keyword.
Flags: blocking1.7.9+
Keywords: fixed1.7.9
Sorry, forgot to hit the MOZILLA_1_7_BRANCH -- done. /be
Keywords: fixed1.7.9
Flags: testcase-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: