More mistakes

V603 The object was created but it is not being used. If you wish to call constructor, 'this->StringCopyInfo::StringCopyInfo(....)' should be used. stringcopyinfo.cpp 64

void StringCopyInfo::InstantiateForceInlinedMembers()
{
 AnalysisAssert(false);
 StringCopyInfo copyInfo;
 JavascriptString *const string = nullptr;
 wchar_t *const buffer = nullptr;
 (StringCopyInfo()); // <=
 (StringCopyInfo(string, buffer)); // <=
 copyInfo.SourceString();
 copyInfo.DestinationBuffer();
}

Programmers often make mistakes, trying to explicitly call the constructor to initialize the object. In this example we see new unnamed objects of "StringCopyInfo" type that get created and then immediately destroyed. As a result, the class fields are left uninitialized.

The correct solution would be to create an initialization function and call it from the constructors in this fragment as well.

V610 Undefined behavior. Check the shift operator '<<'. The left operand '-1' is negative. constants.h 39

class Constants
{
 public:
  ....
  static const int Int31MinValue = -1 << 30;
  ....
};

According to the latest standard of the C++ language, a shift of a negative number results in undefined behavior.

V557 Array overrun is possible. The value of 'i' index could reach 8. rl.cpp 2375

enum TestInfoKind::_TIK_COUNT = 9 

const char * const TestInfoEnvLstFmt[] =
{
 " TESTFILE=\"%s\"",
 " BASELINE=\"%s\"",
 " CFLAGS=\"%s\"",
 " LFLAGS=\"%s\"",
 NULL,
 NULL,
 NULL,
 NULL // <= TestInfoEnvLstFmt[7]
};

void
 WriteEnvLst(
  Test * pDir, TestList * pTestList
 )
 {
  ....
  // print the other TIK_*
 for(int i=0;i < _TIK_COUNT; i++) {
  if (variants->testInfo.data[i] &&
               TestInfoEnvLstFmt[i]){// <=
  LstFilesOut->Add(TestInfoEnvLstFmt[i], // <=
       variants->testInfo.data[i]);
  }
  ....
 }
 ....
}

The analyzer detected that array index is out of bounds. The things is that the for() loop performs 9 iterations, but there are only 8 elements in the "TestInfoEnvLstFmt[]" array.

Perhaps, one more NULL was forgotten in the end:

const char * const TestInfoEnvLstFmt[] =
{
 " TESTFILE=\"%s\"",
 " BASELINE=\"%s\"",
 " CFLAGS=\"%s\"",
 " LFLAGS=\"%s\"",
 NULL,
 NULL,
 NULL,
 NULL // <= TestInfoEnvLstFmt[7]
 NULL // <= TestInfoEnvLstFmt[8]
};

But there is a chance that some string is missing in the middle of the array!

Dangerous Pointers

The V595 diagnostic looks for fragments where the pointer is dereferenced before it is compared with null. Usually there are several of such warnings in projects and such errors holds the record in our error base for the number of issues found (see this blog post of examples from well known software).

However, in general V595 diagnostics are too boring to be listed exhaustively for a project. Also the check and dereference of a pointer can be located quite far away from each other in the function, having dozens or even hundreds of strings between them which makes the explanation of this bug more complicated. So here we'll provide several short examples of code that most probably contain an error related to the pointer handling.

V595 The 'instrLd' pointer was utilized before it was verified against nullptr. Check lines: 1823, 1831. flowgraph.cpp 1823

IR::Instr *
FlowGraph::PeepTypedCm(IR::Instr *instr)
{
 ....
 if (instrLd &&
  !instrLd->GetSrc1()->IsEqual(instr->GetDst()))
 {
  return nullptr;
 }
 if(instrLd2 &&
  !instrLd2->GetSrc1()->IsEqual(instrLd->GetDst()))
 {
  return nullptr;
 }
 ....
}

Have a look at the pointer with the name "instrLd". In the first case we see that it is dereferenced and compared with null, in the second case a programmer forgot to do so, that's why it can cause null pointer dereference.

V595 The 'src2Val' pointer was utilized before it was verified against nullptr. Check lines: 9717, 9725. globopt.cpp 9717

bool GlobOpt::TypeSpecializeIntBinary(....)
{
 ....
 bool isIntConstMissingItem =
                  src2Val->GetValueInfo()->....
 if(isIntConstMissingItem)
 {
  isIntConstMissingItem =
           Js::SparseArraySegment<int>::....
 }
 if (!src2Val ||
      !(src2Val->GetValueInfo()->IsLikelyInt())
        ||isIntConstMissingItem)
 {
  return false;
 }
 ....
}

Pointer "Src2Val" is used at the beginning of the function, but then the developers actively began checking whether this pointer is equal to zero.

V595 The 'm_lastInstr' pointer was utilized before it was verified against nullptr. Check lines: 214, 228. irbuilderasmjs.cpp 214

void
IRBuilderAsmJs::AddInstr(IR::Instr * instr,
                               uint32 offset)
{
 m_lastInstr->InsertAfter(instr); // <=
 if (offset != Js::Constants::NoByteCodeOffset)
 {
  ....
 }
 else if (m_lastInstr) // <=
 {
  instr->SetByteCodeOffset(
      m_lastInstr->GetByteCodeOffset());
 }
 m_lastInstr = instr;
 ....
}

One more example of careless use of a pointer that can potentially be a null pointer.

A list of similar fragments: 

• V595 The 'arrayData' pointer was utilized before it was verified against nullptr. Check lines: 868, 870. immutablelist.h 868

• V595 The 'pMembersList' pointer was utilized before it was verified against nullptr. Check lines: 2012, 2015. diagobjectmodel.cpp 2012

• V595 The 'walkerRef' pointer was utilized before it was verified against nullptr. Check lines: 3191, 3193. diagobjectmodel.cpp 3191

• V595 The 'block->loop' pointer was utilized before it was verified against nullptr. Check lines: 981, 1002. globopt.cpp 981

• V595 The 'src2Val' pointer was utilized before it was verified against nullptr. Check lines: 12528, 12536. globopt.cpp 12528

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 1966, 1967. irbuilderasmjs.cpp 1966

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 2010, 2011. irbuilderasmjs.cpp 2010

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 2076, 2077. irbuilderasmjs.cpp 2076

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 3591, 3592. irbuilderasmjs.cpp 3591

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 4113, 4114. irbuilderasmjs.cpp 4113

• V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 4510, 4511. irbuilderasmjs.cpp 4510

• V595 The 'm_lastInstr' pointer was utilized before it was verified against nullptr. Check lines: 1102, 1116. irbuilder.cpp 1102 

This list shows some of the simplest and clearest examples. To examine all such fragments, developers should have a look at the analysis result themselves.