Closed Bug 1330296 Opened 3 years ago Closed 4 months ago

[Static Analysis][Uninitialized scalar variable] In function Parser<ParseHandler>::assignExpr

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox53 --- affected

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, triage-deferred, Whiteboard: CID 1398301)

Attachments

(1 file)

The Static Analysis tool Coverity detected that |lhs| is not initialised on all paths. For example:

>>    if (handler.isUnparenthesizedDestructuringPattern(lhs)) {
>>        if (kind != PNK_ASSIGN) {
>>            error(JSMSG_BAD_DESTRUCT_ASS);
>>            return null();
>>        }

I think it wouldn't hurt having it initialised right after it's declaration:

>>Node lhs = null();
Attachment #8825794 - Flags: review?(jdemooij) → review?(shu)
Comment on attachment 8825794 [details]
Bug 1330296 - initialise |lhs| with default null value.

Unsurprisingly, Coverity's over-warning here -- |lhs| is only uninitialized if |maybeAsyncArrow| is true, and that's true only if, at the end of the |if (maybeAsyncArrow)| block, the current token is TOK_ARROW.  And that causes the subsequent switch to always go into a TOK_ARROW case that never uses |lhs| and returns at the end of the block that the case labels.

This is sort of a fix, but more it's a placation.  However, my patches in bug 1319416 stand a good chance of fixing this a better way -- have the same basic control flow with |if (tt != TOK_ARROW) fail;| at the end of |if (maybeAsyncArrow)|, but then switch on |tt| directly in the subsequent switch.  Let's get those patches reviewed/landed, then see if this warning shows up still.
Attachment #8825794 - Flags: review?(shu)
Depends on: 1319416
Keywords: triage-deferred
Priority: -- → P3

Fixed by bug 1319416.

Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.