Closed Bug 449233 Opened 16 years ago Closed 16 years ago

[FIX]Assert when decrementing jsdhash/pldhash recursion level past 0

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Details

Attachments

(1 file, 3 obsolete files)

Attached patch Fix (obsolete) — Splinter Review
I've found this useful in debugging some things in the past.  Most of the pldhash changes are just bug 357016, which never regenerated pldhash.  I had to add the single-line thing because of the way JS_ASSERT gets formatted multiline by plify_jsdhash.sed
Attachment #332364 - Flags: superreview?(dbaron)
Attachment #332364 - Flags: review?(dbaron)
Attachment #332364 - Attachment is obsolete: true
Attachment #332365 - Flags: superreview?(dbaron)
Attachment #332365 - Flags: review?(dbaron)
Attachment #332364 - Flags: superreview?(dbaron)
Attachment #332364 - Flags: review?(dbaron)
Attached patch Gah. Actually compiling. (obsolete) — Splinter Review
Attachment #332365 - Attachment is obsolete: true
Attachment #332374 - Flags: superreview?(dbaron)
Attachment #332374 - Flags: review?(dbaron)
Attachment #332365 - Flags: superreview?(dbaron)
Attachment #332365 - Flags: review?(dbaron)
Comment on attachment 332374 [details] [diff] [review]
Gah.  Actually compiling.

Could you make both INCREMENT_RECURSION_LEVEL and DECREMENT_RECURSION_LEVEL into statements rather than expressions, then?  (i.e., put both within JS_BEGIN_MACRO and JS_END_MACRO, and drop the extra pair of parentheses around the outside of the -- and ++ expressions?  (Though it seems I switched to that approach and then Brendan got me to switch back -- see bug 334180 comment 21, etc.)

(I'm not sure if I officially count as a reviewer for JS.)
Attachment #332374 - Flags: superreview?(dbaron)
Attachment #332374 - Flags: superreview+
Attachment #332374 - Flags: review?(dbaron)
Attachment #332374 - Flags: review+
I guess I should get brendan to take a look too...

The other option is to use a comma expression, if we really want to be able to get a value out of INCREMENT and DECREMENT.
Attachment #332374 - Attachment is obsolete: true
Attachment #332381 - Flags: review?(brendan)
Comment on attachment 332381 [details] [diff] [review]
With those changes.

>diff --git a/js/src/jsdhash.cpp b/js/src/jsdhash.cpp
>--- a/js/src/jsdhash.cpp
>+++ b/js/src/jsdhash.cpp
>@@ -62,29 +62,37 @@
>  * table->ops or to an enumerator do not cause re-entry into a call that
>  * can mutate the table.  The recursion level is stored in additional
>  * space allocated at the end of the entry store to avoid changing
>  * JSDHashTable, which could cause issues when mixing DEBUG and
>  * non-DEBUG components.
>  */
> #ifdef DEBUG
> 
>+#define JSDHASH_SINGLE_LINE_ASSERTION JS_ASSERT

Argh, maybe just JSDHASH_INLINE_ASSERT or ...ONELINE_...?

r=me with that, and thanks for using and maintaining plify-jsdhash.sed!

/be
Attachment #332381 - Flags: review?(brendan) → review+
Pushed add694324c7e with that change.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: