Closed
Bug 872
Opened 26 years ago
Closed 26 years ago
Bogus "property is not defined" errors
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
INVALID
People
(Reporter: rainer, Assigned: brendan)
Details
The lookup code for the JavaScript property cache is wrong. False cache hits are possible when a property is cached as PROP_NOT_FOUND. To the user, the bug manifests itself as bogus "property is not defined" errors, in perfectly valid JavaScript programs.
This is the buggy code, file jsinterp.h:
#define PROPERTY_CACHE_TEST(cache, obj, id, prop) \
PR_BEGIN_MACRO \
uintN _hashIndex = (uintN)PROPERTY_CACHE_HASH(obj, id); \
JSPropertyCacheEntry *_pce = &(cache)->table[_hashIndex]; \
(cache)->tests++; \
if (_pce->property && \
(_pce->property == PROP_NOT_FOUND ||
/* *** BUG! obj has to be compared every time! *** */
_pce->property->object == obj) && \
_pce->symbolid == id) { \
prop = _pce->property; \
} else { \
(cache)->misses++; \
prop = NULL; \
} \
PR_END_MACRO
The simplest fix is putting an extra field containing the object into the JSPropertyCacheEntry structure. That's what I did and it works. Here is the fixed section of jsinterp.h:
typedef struct JSPropertyCacheEntry {
JSObject *obj; /* weak link to object */
JSProperty *property; /* weak link to property */
jsval symbolid; /* strong link to name atom, or index jsval */
} JSPropertyCacheEntry;
typedef struct JSPropertyCache {
JSPropertyCacheEntry table[PROPERTY_CACHE_SIZE];
JSBool empty;
uint32 fills;
uint32 recycles;
uint32 tests;
uint32 misses;
uint32 flushes;
uint32 pflushes;
} JSPropertyCache;
#define PROP_NOT_FOUND ((JSProperty *)1)
#define PROP_FOUND(prop) ((prword)(prop) & (prword)-2)
#define PROPERTY_CACHE_FILL(cx, cache, obj, id, prop) \
PR_BEGIN_MACRO \
uintN _hashIndex = (uintN)PROPERTY_CACHE_HASH(obj, id); \
JSPropertyCacheEntry *_pce = &(cache)->table[_hashIndex]; \
if (_pce->property) { \
(cache)->recycles++; \
if (!JSVAL_IS_INT(_pce->symbolid)) \
js_DropAtom(cx, (JSAtom *)_pce->symbolid); \
} \
_pce->obj = obj; \
_pce->property = prop; \
_pce->symbolid = id; \
if (!JSVAL_IS_INT(id)) \
js_HoldAtom(cx, (JSAtom *)id); \
(cache)->empty = JS_FALSE; \
(cache)->fills++; \
PR_END_MACRO
#define PROPERTY_CACHE_TEST(cache, obj, id, prop) \
PR_BEGIN_MACRO \
uintN _hashIndex = (uintN)PROPERTY_CACHE_HASH(obj, id); \
JSPropertyCacheEntry *_pce = &(cache)->table[_hashIndex]; \
(cache)->tests++; \
if (_pce->obj == obj && _pce->symbolid == id) { \
prop = _pce->property; \
} else { \
(cache)->misses++; \
prop = NULL; \
} \
PR_END_MACRO
Assignee | ||
Updated•26 years ago
|
Assignee: mccabe → brendan
Updated•26 years ago
|
Status: NEW → RESOLVED
Closed: 26 years ago
Resolution: --- → INVALID
Comment 1•26 years ago
|
||
Turns out that this was based on the code from the original March drop.
Changing component to "Javascript Engine". "Javascript" component is being
retired.
You need to log in
before you can comment on or make changes to this bug.
Description
•