Closed Bug 1238917 Opened 4 years ago Closed 4 years ago

[Static Analysis][Uninitialized scalar field] In function RegExpStatics from RegExpStatics.h

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- affected
firefox48 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1347689)

Attachments

(1 file)

The Static Analysis tool Coverity added lazySticky is not initialized in the constructor of RegExpStatics.

Looking through code lazySticky  is used in several places such:

1.
>>    static size_t offsetOfLazySticky() {
>>        return offsetof(RegExpStatics, lazySticky);
>>    }

2. 
>>    RootedLinearString input(cx, matchesInput);
>>    RegExpRunStatus status = g->execute(cx, input, lazyIndex, lazySticky, &this->matches, nullptr);
>>    if (status == RegExpRunStatus_Error)
>>        return false;

I don't know for sure that the initialization of the variable that takes place here:

>>    lazySource = shared->source;
>>    lazyFlags = shared->flags;
>>    lazyIndex = lastIndex;
>>    lazySticky = sticky;
>>    pendingLazyEvaluation = 1;

is done prior of using it.  I consider that there is no harm in initializing it with false in the RegExpStatics::clear function.
Comment on attachment 8706871 [details]
MozReview Request: Bug 1238917 - initialize lazySticky in clear function. r?jorendorff@mozilla.com

https://reviewboard.mozilla.org/r/30527/#review35487

Sorry for the ridiculously slow review here.

For what it's worth, the existing code is correct: all the lazy fields are meaningless (and they won't be read) unless pendingLastEvaluation is 1.

But the constructor initializes all the other fields, so it's super weird not to be initializing this one too.
Attachment #8706871 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/5d450b4ccb14
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.