991014 - CID 1136682, CID 1136683: Uninitialized pointer field in jit/LiveRangeAllocator.h and jit/LinearScan.h as found by Coverity

Status: RESOLVED INVALID

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

+++ This bug was initially created as a clone of Bug #991012 +++

Coverity analysis of source code in js/src has found uninitialized pointer fields in jit/LiveRangeAllocator.h and jit/LinearScan.h issues, that probably happened in the past month.

________________________________________________________________________________________________________
*** CID 1136682:  Uninitialized pointer field  (UNINIT_CTOR)
/js/src/jit/LiveRangeAllocator.h: 412 in js::jit::VirtualRegister::VirtualRegister(js::jit::TempAllocator &)()
406         void operator=(const VirtualRegister &) MOZ_DELETE;
407         VirtualRegister(const VirtualRegister &) MOZ_DELETE;
408     
409       protected:
410         explicit VirtualRegister(TempAllocator &alloc)
411           : intervals_(alloc)
>>>     CID 1136682:  Uninitialized pointer field  (UNINIT_CTOR)
>>>     Non-static class member "isTemp_" is not initialized in this constructor nor in any functions that it calls.
412         {}
413     
414       public:
415         bool init(TempAllocator &alloc, LBlock *block, LInstruction *ins, LDefinition *def,
416                   bool isTemp)
417         {

________________________________________________________________________________________________________
*** CID 1136683:  Uninitialized pointer field  (UNINIT_CTOR)
/js/src/jit/LinearScan.h: 31 in js::jit::LinearScanVirtualRegister::LinearScanVirtualRegister(js::jit::TempAllocator &)()
25         // processed by freeAllocation().
26         bool finished_ : 1;
27     
28       public:
29         LinearScanVirtualRegister(TempAllocator &alloc)
30           : VirtualRegister(alloc)
>>>     CID 1136683:  Uninitialized pointer field  (UNINIT_CTOR)
>>>     Non-static class member "finished_" is not initialized in this constructor nor in any functions that it calls.
31         {}
32         void setCanonicalSpill(LAllocation *alloc) {
33             canonicalSpill_ = alloc;
34         }
35         LAllocation *canonicalSpill() const {
36             return canonicalSpill_;


jandem, any thoughts on how to move forward here?

Comment 1

[Jan de Mooij [:jandem]]

This looks like a false-positive. VirtualRegisterMap::init initializes all VirtualRegisters to zero, but Coverity's static analysis is not smart enough to reason about that.

Anyway, initializing these in the constructor is trivial, will post a patch later today.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Jan de Mooij [:jandem] from comment #1)
> Anyway, initializing these in the constructor is trivial, will post a patch
> later today.

Hm there's actually a bunch of other fields that have to be initialized this way. Considering there can be many VirtualRegisters for large scripts, I'm not too happy about doing that just to silence this Coverity warning.

As I explained in comment 1, the memory is explicitly initialized to zero with memset. Gary, ok if we close this bug?

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Sure.