Open Bug 620241 Opened 9 years ago Updated 2 years ago

Decompile leaks argv in JSOP_NEW.. when LOCAL_ASSERT fails

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

ASSIGNED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, memory-leak)

Attachments

(1 file)

1297 #define LOCAL_ASSERT_CUSTOM(expr, BAD_EXIT)                                   \
1298     JS_BEGIN_MACRO                                                            \
1299         JS_ASSERT(expr);                                                      \
1300         if (!(expr)) { BAD_EXIT; }                                            \
1301     JS_END_MACRO
1302 
1303 #define LOCAL_ASSERT_RV(expr, rv)                                             \
1304     LOCAL_ASSERT_CUSTOM(expr, return (rv))

1825 Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)

1869 #define LOCAL_ASSERT(expr)    LOCAL_ASSERT_RV(expr, NULL)

3584               case JSOP_NEW:
3585               case JSOP_CALL:
3586               case JSOP_EVAL:
3587               case JSOP_FUNCALL:
3588               case JSOP_FUNAPPLY:
3589                 argc = GET_ARGC(pc);
3590                 argv = (char **)
3591                     cx->malloc((size_t)(argc + 1) * sizeof *argv);
3592                 if (!argv)
3593                     return NULL;
3594 
3595                 op = JSOP_SETNAME;
3596                 ok = JS_TRUE;
3597                 for (i = argc; i > 0; i--)
3598                     argv[i] = JS_strdup(cx, POP_STR());
3599 
3600                 /* Skip the JSOP_PUSHOBJ-created empty string. */
3601                 LOCAL_ASSERT(ss->top >= 2);
Attached patch patchSplinter Review
Assignee: general → timeless
Status: NEW → ASSIGNED
Attachment #498618 - Flags: review?(jorendorff)
Comment on attachment 498618 [details] [diff] [review]
patch

Review of attachment 498618 [details] [diff] [review]:

To me, the leak looks worse than that; all the elements of argv are leaked as well. The cleanup code is on line 3674:

>                for (i = 0; i <= argc; i++)
>                    cx->free_(argv[i]);
>                cx->free_(argv);

Needs a `goto cleanup` sort of fix.
Attachment #498618 - Flags: review?(jorendorff)
The relevant code appears to have been substantially rewritten. Can this be closed?
You need to log in before you can comment on or make changes to this bug.