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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

unspecified
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Created attachment 332364 [details] [diff] [review]
Fix

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)
Created attachment 332365 [details] [diff] [review]
Oops, need to include the sed file changes too
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)
Created attachment 332374 [details] [diff] [review]
Gah.  Actually compiling.
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+
Created attachment 332381 [details] [diff] [review]
With those changes.

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)
Duplicate of this bug: 450273
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
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.