Closed
Bug 1248584
Opened 9 years ago
Closed 9 years ago
[Static Analysis][Explicit null dereferenced] In function CloneOldBaselineStub
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: andi, Assigned: andi)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: CID 1345979)
Attachments
(1 file)
The Static Analysis tool Coverity added that firstMonitorStub is null pointer dereference when trying to clone in swtich code block. This can happen if the following if is false:
>> ICStub* firstMonitorStub;
>> if (fallbackStub->isMonitoredFallback()) {
>> ICMonitoredFallbackStub* monitored = fallbackStub->toMonitoredFallbackStub();
>> firstMonitorStub = monitored->fallbackMonitorStub()->firstMonitorStub();
>> } else {
>> firstMonitorStub = nullptr;
>> }
The place where it actually gets dereferenced is:
>> switch (oldStub->kind()) {
>>#define CASE_KIND(kindName) \
>> case ICStub::kindName: \
>> entry.newStub = IC##kindName::Clone(cx, stubSpace, firstMonitorStub, \
>> *oldStub->to##kindName()); \
>> break;
>> PATCHABLE_ICSTUB_KIND_LIST(CASE_KIND)
>>#undef CASE_KIND
>>
>> default:
>> MOZ_CRASH("Bad stub kind");
>> }
Looking through the ICs that are specified here and call clone all of the call func tion New<T>
>> template <typename T, typename... Args>
>> static T* New(JSContext* cx, ICStubSpace* space, JitCode* code, Args&&... args) {
>> if (!code)
>> return nullptr;
>> T* result = space->allocate<T>(code, mozilla::Forward<Args>(args)...);
>> if (!result)
>> ReportOutOfMemory(cx);
>> return result;
>> }
and as it can be seen space that is taken the value from firstMonitorStub is dereferenced this in case firstMonitorStub is nullptr leading to a nullptr dereference.
Assignee | ||
Comment 1•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/35099/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35099/
Attachment #8719766 -
Flags: review?(jdemooij)
Comment 2•9 years ago
|
||
I don't understand the problem. Do you have an error message or stack from Coverity?
(In reply to Andi-Bogdan Postelnicu from comment #0)
> and as it can be seen space that is taken the value from firstMonitorStub
> is dereferenced this in case firstMonitorStub is nullptr leading to a
> nullptr dereference.
firstMonitorStub is not passed as |space|, but as ICStub* argument right?
Assignee | ||
Comment 3•9 years ago
|
||
Coverity is just a static analysis tools so there is no stack trace. What coveirty warns is that if in case firstMonitorStub is null, a null pointer dereference will occur in function New that is called by IC##kindName::Clone :
>> template <typename T, typename... Args>
>> static T* New(JSContext* cx, ICStubSpace* space, JitCode* code, Args&&... args) {
>> if (!code)
>> return nullptr;
>> T* result = space->allocate<T>(code, mozilla::Forward<Args>(args)...);
>> if (!result)
>> ReportOutOfMemory(cx);
>> return result;
>> }
In the context of New function variable |space| is actually variable |firstMonitorStub| from CloneOldBaselineStub.
Assignee | ||
Comment 4•9 years ago
|
||
It seems i wasn't that exact in my last comments, you can see that on below lines the null pointer dereference takes place:
>>ICMonitoredStub::ICMonitoredStub(Kind kind, JitCode* stubCode, ICStub* firstMonitorStub)
>> : ICStub(kind, ICStub::Monitored, stubCode),
>> firstMonitorStub_(firstMonitorStub)
>>{
>> // If the first monitored stub is a ICTypeMonitor_Fallback stub, then
>> // double check that _its_ firstMonitorStub is the same as this one.
>> MOZ_ASSERT_IF(firstMonitorStub_->isTypeMonitor_Fallback(),
>> firstMonitorStub_->toTypeMonitor_Fallback()->firstMonitorStub() ==
>> firstMonitorStub_);
>>}
Assignee | ||
Comment 5•9 years ago
|
||
Comment on attachment 8719766 [details]
MozReview Request: Bug 1248584 - assert firstMonitorStub_ in ICMonitoredStub in order to silence Coverity. r?jandem
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35099/diff/1-2/
Attachment #8719766 -
Attachment description: MozReview Request: Bug 1248584 - if fallbackStub is not monitored exit from CloneOldBaselineStub with false. r?jandem → MozReview Request: Bug 1248584 - assert firstMonitorStub_ in ICMonitoredStub in order to silence Coverity. r?jandem
Updated•9 years ago
|
Attachment #8719766 -
Flags: review?(jdemooij) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8719766 [details]
MozReview Request: Bug 1248584 - assert firstMonitorStub_ in ICMonitoredStub in order to silence Coverity. r?jandem
https://reviewboard.mozilla.org/r/35099/#review31925
Comment 8•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•