PVS-Studio: 25 подозрительных фрагментов кода из CoreCLR

Корпорация Microsoft выложила в открытый доступ исходный код движка CoreCLR, который является ключевым элементом .NET Core. Эта новость, конечно же, не могла не привлечь наше внимание. Ведь чем больше аудитория у проекта, тем тревожнее будут выглядеть найденные подозрительные места. Несмотря на авторство Microsoft, как в любом крупном проекте, тут есть на что посмотреть и над чем задуматься.

Введение

CoreCLR является средой исполнения .NET Core, выполняя такие функции как сборку мусора или компиляции в конечный машинный код. .Net Core — это модульная реализация .Net, которая может быть использована как база для огромного количества сценариев.

Исходный код с недавнего времени доступен на GitHub и проверялся с помощью PVS-Studio 5.23. Как и я, желающие могут получить полный лог проверки с помощью Microsoft Visual Studio Community Edition, выход которой тоже был недавней новостью от Microsoft.

Опечатки

Традиционно я начинаю с мест, похожих на опечатки. В условных выражениях повторяются переменные, константы, макросы или поля структур/классов. Наличие ошибки — предмет дискуссий, тем не менее такие места были найдены и выглядят подозрительно.

V501 There are identical sub-expressions 'tree->gtOper == GT_CLS_VAR' to the left and to the right of the '||' operator. ClrJit lsra.cpp 3140

// register variable 
GTNODE(GT_REG_VAR      , "regVar"  ,0,GTK_LEAF|GTK_LOCAL)
// static data member
GTNODE(GT_CLS_VAR      , "clsVar"  ,0,GTK_LEAF)
// static data member address
GTNODE(GT_CLS_VAR_ADDR , "&clsVar" ,0,GTK_LEAF)           
....

void  LinearScan::buildRefPositionsForNode(GenTree *tree, ....)
{
  ....
  if ((tree->gtOper == GT_CLS_VAR ||
       tree->gtOper == GT_CLS_VAR) && i == 1)
  {
      registerType = TYP_PTR;
      currCandidates = allRegs(TYP_PTR);
  }
  ....
}

Хотя структура 'GenTree' имеет похожее по имени поле «tree->gtType», но оно имеет другой тип с «tree->gtOper». Думаю, здесь дело в скопированной константе. То-есть в выражении должна использоваться ещё одна константа, помимо GT_CLS_VAR.

V501 There are identical sub-expressions 'DECODE_PSP_SYM' to the left and to the right of the '|' operator. daccess 264

enum GcInfoDecoderFlags
{
    DECODE_SECURITY_OBJECT       = 0x01,
    DECODE_CODE_LENGTH           = 0x02,
    DECODE_VARARG                = 0x04,
    DECODE_INTERRUPTIBILITY      = 0x08,
    DECODE_GC_LIFETIMES          = 0x10,
    DECODE_NO_VALIDATION         = 0x20,
    DECODE_PSP_SYM               = 0x40,
    DECODE_GENERICS_INST_CONTEXT = 0x80,
    DECODE_GS_COOKIE             = 0x100,   
    DECODE_FOR_RANGES_CALLBACK   = 0x200,
    DECODE_PROLOG_LENGTH         = 0x400,
    DECODE_EDIT_AND_CONTINUE     = 0x800,
};

size_t GCDump::DumpGCTable(PTR_CBYTE table, ....)
{
  GcInfoDecoder hdrdecoder(table,
   (GcInfoDecoderFlags)(  DECODE_SECURITY_OBJECT
                        | DECODE_GS_COOKIE
                        | DECODE_CODE_LENGTH
                        | DECODE_PSP_SYM                //<==1
                        | DECODE_VARARG
                        | DECODE_PSP_SYM                //<==1
                        | DECODE_GENERICS_INST_CONTEXT  //<==2
                        | DECODE_GC_LIFETIMES
                        | DECODE_GENERICS_INST_CONTEXT  //<==2
                        | DECODE_PROLOG_LENGTH),
   0);
  ....
}

Тут даже две повторяющиеся константы, хотя в перечислении " GcInfoDecoderFlags" есть и другие константы, которые в условии не используются.

Ещё похожие места:

• V501 There are identical sub-expressions 'varLoc1.vlStk2.vls2BaseReg' to the left and to the right of the '==' operator. cee_wks util.cpp 657
• V501 There are identical sub-expressions 'varLoc1.vlStk2.vls2Offset' to the left and to the right of the '==' operator. cee_wks util.cpp 658
• V501 There are identical sub-expressions 'varLoc1.vlFPstk.vlfReg' to the left and to the right of the '==' operator. cee_wks util.cpp 661

V700 Consider inspecting the 'T foo = foo = ...' expression. It is odd that variable is initialized through itself. cee_wks zapsig.cpp 172

BOOL ZapSig::GetSignatureForTypeHandle(....)
{
  ....
  CorElementType elemType = elemType =
    TryEncodeUsingShortcut(pMT);
  ....
}

Вроде бы просто лишнее присваивание, но такие ошибки часто делают при копировании кода и забывают что-нибудь переименовать. В любом случае, в таком виде код бессмысленный.

V523 The 'then' statement is equivalent to the 'else' statement. cee_wks threadsuspend.cpp 2468

enum __MIDL___MIDL_itf_mscoree_0000_0004_0001
{
  OPR_ThreadAbort = 0,
  OPR_ThreadRudeAbortInNonCriticalRegion = .... ,
  OPR_ThreadRudeAbortInCriticalRegion = ....) ,
  OPR_AppDomainUnload = .... ,
  OPR_AppDomainRudeUnload = ( OPR_AppDomainUnload + 1 ) ,
  OPR_ProcessExit = ( OPR_AppDomainRudeUnload + 1 ) ,
  OPR_FinalizerRun = ( OPR_ProcessExit + 1 ) ,
  MaxClrOperation = ( OPR_FinalizerRun + 1 ) 
}  EClrOperation;

void Thread::SetRudeAbortEndTimeFromEEPolicy()
{
  LIMITED_METHOD_CONTRACT;
  DWORD timeout;
  if (HasLockInCurrentDomain())
  {
    timeout = GetEEPolicy()->
      GetTimeout(OPR_ThreadRudeAbortInCriticalRegion);  //<==
  }
  else
  {
    timeout = GetEEPolicy()->
      GetTimeout(OPR_ThreadRudeAbortInCriticalRegion);  //<==
  }
  ....
}

Данная диагностика находит одинаковые блоки в конструкциях if/else. И здесь тоже есть подозрение на опечатку в константе. В первом случае, по смыслу как раз подходит «OPR_ThreadRudeAbortInNonCriticalRegion».

Аналогичные места:

• V523 The 'then' statement is equivalent to the 'else' statement. ClrJit instr.cpp 3427
• V523 The 'then' statement is equivalent to the 'else' statement. ClrJit flowgraph.cpp 18815
• V523 The 'then' statement is equivalent to the 'else' statement. daccess dacdbiimpl.cpp 6374

Список инициализации конструктора

V670 The uninitialized class member 'gcInfo' is used to initialize the 'regSet' member. Remember that members are initialized in the order of their declarations inside a class. ClrJit codegencommon.cpp 92

CodeGenInterface *getCodeGenerator(Compiler *comp);

class CodeGenInterface
{
    friend class emitter;

public:
    ....
    RegSet  regSet; //<=== line 91
    ....
public:
    GCInfo  gcInfo; //<=== line 322
....
};

// CodeGen constructor
CodeGenInterface::CodeGenInterface(Compiler* theCompiler) :
    compiler(theCompiler),
    gcInfo(theCompiler),
    regSet(theCompiler, gcInfo)
{
}

Согласно стандарту, порядок инициализация членов класса в конструкторе происходит в порядке их объявления в классе. Для исправления ошибки следует перенести объявление члена класса 'gcInfo' выше объявления 'regSet'.

Ложное, но полезное предупреждение

V705 It is possible that 'else' block was forgotten or commented out, thus altering the program's operation logics. daccess daccess.cpp 2979

HRESULT Initialize()
{
  if (hdr.dwSig == sig)
  {
      m_rw = eRO;
      m_MiniMetaDataBuffSizeMax = hdr.dwTotalSize;
      hr = S_OK;
  }
  else
  // when the DAC initializes this for the case where the target is 
  // (a) a live process, or (b) a full dump, buff will point to a
  // zero initialized memory region (allocated w/ VirtualAlloc)
  if (hdr.dwSig == 0 && hdr.dwTotalSize == 0 && hdr.dwCntStreams == 0)
  {
      hr = S_OK;
  }
  // otherwise we may have some memory corruption. treat this as
  // a liveprocess/full dump
  else
  {
      hr = S_FALSE;
  }
  ....
}

Анализатор обнаружил подозрительное место в коде. Здесь видно, что код ПРОкомментирован и всё нормально. Но такого типа ошибки очень распространены, когда код после 'else' ЗАкомментирован и следующий за ним оператор становится частью условия. В данном примере нет ошибки, но её вполне можно допустить при редактировании этого места в будущем.

64-битная ошибка

V673 The '0xefefefef << 28' expression evaluates to 1080581331517177856. 60 bits are required to store the value, but the expression evaluates to the 'unsigned' type which can only hold '32' bits. cee_dac _dac object.inl 95

inline void Object::EnumMemoryRegions(void)
{
  ....
  SIZE_T size = sizeof(ObjHeader) + sizeof(Object);
  ....
  size |= 0xefefefef << 28;
  ....
}

Про термин «64-битная ошибка» можно прочесть здесь. В данном примере, после сдвига будет выполняться операция «size |= 0xf0000000» в 32-х битной программе и «size |= 0x00000000f0000000» в 64-х битной. Скорее всего, в 64-х битной программе планировали вычислять: «size |= 0x0efefefef0000000». Но где же теряется старшая часть числа?

Число «0xefefefef» имеет тип 'unsigned', так как не помещается в тип 'int'. Выполняется сдвиг 32-х битного числа и в результате мы получим 0xf0000000 беззнакового типа. Далее это беззнаковое число расширится до SIZE_T и мы получим 0x00000000f0000000.

Для корректной работы необходимо сначала выполнить явное приведение типа. Пример корректного кода:

inline void Object::EnumMemoryRegions(void)
{
  ....
  SIZE_T size = sizeof(ObjHeader) + sizeof(Object);
  ....
  size |= SIZE_T(0xefefefef) << 28;
  ....
}

Ещё такое место:

• V673 The '0xefefefef << 28' expression evaluates to 1080581331517177856. 60 bits are required to store the value, but the expression evaluates to the 'unsigned' type which can only hold '32' bits. cee_dac dynamicmethod.cpp 807

Код «в отставке»

Порой условия пишутся так, что буквально противоречат друг другу.

V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 31825, 31827. cee_wks gc.cpp 31825

void gc_heap::verify_heap (BOOL begin_gc_p)
{
  ....
  if (brick_table [curr_brick] < 0)
  {
    if (brick_table [curr_brick] == 0)
    {
      dprintf(3, ("curr_brick %Ix for object %Ix set to 0",
              curr_brick, (size_t)curr_object));
      FATAL_GC_ERROR();
    }
    ....
  }
  ....
}

Код, который никогда не получает управление, но он выглядит не таким значимым, как в следующем примере:

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 2353, 2391. utilcode util.cpp 2353

void  PutIA64Imm22(UINT64 * pBundle, UINT32 slot, INT32 imm22)
{
  if (slot == 0)
  {
    const UINT64 mask0 = UI64(0xFFFFFC000603FFFF);
    /* Clear all bits used as part of the imm22 */
    pBundle[0] &= mask0;

    UINT64 temp0;
    
    temp0  = (UINT64) (imm22 & 0x200000) << 20;     //  1 s
    temp0 |= (UINT64) (imm22 & 0x1F0000) << 11;     //  5 imm5c
    temp0 |= (UINT64) (imm22 & 0x00FF80) << 25;     //  9 imm9d
    temp0 |= (UINT64) (imm22 & 0x00007F) << 18;     //  7 imm7b
    
    /* Or in the new bits used in the imm22 */
    pBundle[0] |= temp0;
  }
  else if (slot == 1)
  {
    ....
  }
  else if (slot == 0)        //<==
  {
    const UINT64 mask1 = UI64(0xF000180FFFFFFFFF);
    /* Clear all bits used as part of the imm22 */
    pBundle[1] &= mask1;

    UINT64 temp1;
    
    temp1  = (UINT64) (imm22 & 0x200000) << 37;     //  1 s
    temp1 |= (UINT64) (imm22 & 0x1F0000) << 32;     //  5 imm5c
    temp1 |= (UINT64) (imm22 & 0x00FF80) << 43;     //  9 imm9d
    temp1 |= (UINT64) (imm22 & 0x00007F) << 36;     //  7 imm7b
    
    /* Or in the new bits used in the imm22 */
    pBundle[1] |= temp1;
  }
  FlushInstructionCache(GetCurrentProcess(),pBundle,16);
}

Возможно, очень важный код никогда не получает управление из-за ошибки в каскаде условных операторов.

Ещё подозрительные места:

• V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 2898, 2900. daccess nidump.cpp 2898
• V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 337, 339. utilcode prettyprintsig.cpp 337
• V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 774, 776. utilcode prettyprintsig.cpp 774

Неопределённое поведение

V610 Undefined behavior. Check the shift operator '<<'. The left operand '-1' is negative. bcltype metamodel.h 532

inline static mdToken decodeToken(....)
{
    //<TODO>@FUTURE: make compile-time calculation</TODO>
    ULONG32 ix = (ULONG32)(val & ~(-1 << m_cb[cTokens]));

    if (ix >= cTokens)
        return rTokens[0];
    return TokenFromRid(val >> m_cb[cTokens], rTokens[ix]);
}

Анализатор обнаружил операцию сдвига отрицательного числа, которая приводит к неопределенному поведению.

V610 Undefined behavior. Check the shift operator '<<'. The left operand '(~0)' is negative. cee_dac decodemd.cpp 456

#define bits_generation 2
#define generation_mask (~(~0 << bits_generation))

#define MASK(len) (~((~0)<<len))
#define MASK64(len) ((~((~((unsigned __int64)0))<<len)))

void Encoder::Add(unsigned value, unsigned length)
{
  ....
  value = (value & MASK(length));
  ....
}

Благодаря сообщению анализатора V610 я нашёл несколько некорректных макросов. '~0' приводится к знаковому отрицательному числу типа int, после чего выполняется сдвиг. Как в одном из макросов, необходимо выполнить явное преобразование к unsigned:

#define bits_generation 2
#define generation_mask (~(~((unsigned int)0) << bits_generation))

#define MASK(len) (~((~((unsigned int)0))<<len))
#define MASK64(len) ((~((~((unsigned __int64)0))<<len)))

Некорректный sizeof(xx)

V579 The DacReadAll function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. daccess dacimpl.h 1688

template<class T>
inline bool MisalignedRead(CORDB_ADDRESS addr, T *t)
{
  return SUCCEEDED(DacReadAll(TO_TADDR(addr), t, sizeof(t), false));
}

Вот такая маленькая функция, которая всегда берёт размер указателя. Скорее всего, тут хотели написать «sizeof(*t)», ну или «sizeof(T)».

Ещё один наглядный примерчик:

V579 The Read function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. util.cpp 4943

HRESULT GetMTOfObject(TADDR obj, TADDR *mt)
{
  if (!mt)
    return E_POINTER;

  HRESULT hr = rvCache->Read(obj, mt, sizeof(mt), NULL);
  if (SUCCEEDED(hr))
    *mt &= ~3;

  return hr;
}

Семейство функций «memFAIL»

С использованием memXXX-функций можно допустить самые разные ошибки. Для поиска таких мест в анализаторе есть несколько диагностических правил.

V512 A call of the 'memset' function will lead to underflow of the buffer 'pAddExpression'. sos strike.cpp 11973

DECLARE_API(Watch)
{
  ....
  if(addExpression.data != NULL || aExpression.data != NULL)
  {
    WCHAR pAddExpression[MAX_EXPRESSION];
    memset(pAddExpression, 0, MAX_EXPRESSION);
    swprintf_s(pAddExpression, MAX_EXPRESSION, L"%S", ....);
    Status = g_watchCmd.Add(pAddExpression);
  }
  ....
}

Распространённая ошибка, когда забывают делать поправку на размер типа:

WCHAR pAddExpression[MAX_EXPRESSION];
memset(pAddExpression, 0, sizeof(WCHAR)*MAX_EXPRESSION);

Еще несколько таких мест:

• V512 A call of the 'memset' function will lead to underflow of the buffer 'pSaveName'. sos strike.cpp 11997
• V512 A call of the 'memset' function will lead to underflow of the buffer 'pOldName'. sos strike.cpp 12013
• V512 A call of the 'memset' function will lead to underflow of the buffer 'pNewName'. sos strike.cpp 12016
• V512 A call of the 'memset' function will lead to underflow of the buffer 'pExpression'. sos strike.cpp 12024
• V512 A call of the 'memset' function will lead to underflow of the buffer 'pFilterName'. sos strike.cpp 12039

V598 The 'memcpy' function is used to copy the fields of 'GenTree' class. Virtual table pointer will be damaged by this. ClrJit compiler.hpp 1344

struct GenTree
{
  ....
  #if DEBUGGABLE_GENTREE
    virtual void DummyVirt() {}
  #endif // DEBUGGABLE_GENTREE
  ....
};

void GenTree::CopyFrom(const GenTree* src, Compiler* comp)
{
  ....
  memcpy(this, src, src->GetNodeSize());
  ....
}

Если объявлена переменная препроцессора 'DEBUGGABLE_GENTREE', то определяется виртуальная функция. Тогда класс содержит указатель на таблицу виртуальных методов и его уже нельзя вот так просто копировать.

V598 The 'memcpy' function is used to copy the fields of 'GCStatistics' class. Virtual table pointer will be damaged by this. cee_wks gc.cpp 287

struct GCStatistics
    : public StatisticsBase
{
  ....
  virtual void Initialize();
  virtual void DisplayAndUpdate();
  ....
};

GCStatistics g_LastGCStatistics;

void GCStatistics::DisplayAndUpdate()
{
  ....
  memcpy(&g_LastGCStatistics, this, sizeof(g_LastGCStatistics));
  ....
}

В этом месте некорректное копирование выполняется не только в режиме отладки.

V698 Expression 'memcmp(....) == -1' is incorrect. This function can return not only the value '-1', but any negative value. Consider using 'memcmp(....) < 0' instead. sos util.cpp 142

bool operator( )(const GUID& _Key1, const GUID& _Key2) const
  { return memcmp(&_Key1, &_Key2, sizeof(GUID)) == -1; }

Сравнивать результат функции 'memcmp' со значением 1 или -1 не корректно. Работоспособность таких конструкций зависит от библиотек, компилятора, его настроек, операционной системы, её разрядности и так далее; в таком случае необходимо проверять одно из трёх состояний: '< 0', '0' или '> 0'.

Аналогичное место:

• V698 Expression 'wcscmp(....) == -1' is incorrect. This function can return not only the value '-1', but any negative value. Consider using 'wcscmp(....) < 0' instead. sos strike.cpp 3855

Про указатели

V522 Dereferencing of the null pointer 'hp' might take place. cee_wks gc.cpp 4488

heap_segment* gc_heap::get_segment_for_loh (size_t size
#ifdef MULTIPLE_HEAPS
                                           , gc_heap* hp
#endif //MULTIPLE_HEAPS
                                           )
{
#ifndef MULTIPLE_HEAPS
    gc_heap* hp = 0;
#endif //MULTIPLE_HEAPS
    heap_segment* res = hp->get_segment (size, TRUE);
  ....
}

Если 'MULTIPLE_HEAPS' не определён, то беда. Указатель окажется равен нулю.

V595 The 'tree' pointer was utilized before it was verified against nullptr. Check lines: 6970, 6976. ClrJit gentree.cpp 6970

void Compiler::gtDispNode(GenTreePtr tree, ....)
{
  ....
  if (tree->gtOper >= GT_COUNT)
  {
    printf(" **** ILLEGAL NODE ****");
    return;
  }

  if  (tree && printFlags)
  {
    /* First print the flags associated with the node */
    switch (tree->gtOper)
    {
      ....
    }
    ....
  }
  ....
}

В исходном коде распространены места, когда валидность указателя таки проверяется, но уже после разыменования.

Весь список: CoreCLR_V595.txt.

Лишние проверки

Даже если лишний код не наносит вреда, то его наличие может просто отвлекать внимание разработки от более важных мест.

V503 This is a nonsensical comparison: pointer >= 0. cee_wks gc.cpp 21707

void gc_heap::make_free_list_in_brick (BYTE* tree,
                                       make_free_args* args)
{
  assert ((tree >= 0));
  ....
}

Вот такая проверка указателя. Ещё примеры:

• V503 This is a nonsensical comparison: pointer >= 0. cee_wks gc.cpp 23204
• V503 This is a nonsensical comparison: pointer >= 0. cee_wks gc.cpp 27683

V547 Expression 'maxCpuId >= 0' is always true. Unsigned type value is always >= 0. cee_wks codeman.cpp 1219

void EEJitManager::SetCpuInfo()
{
  ....
  unsigned char buffer[16];
  DWORD maxCpuId = getcpuid(0, buffer);
  if (maxCpuId >= 0)
  {
  ....
}

Похожий пример, только с типом DWORD.

V590 Consider inspecting the 'wzPath[0] != L'' && wzPath[0] == L'\'' expression. The expression is excessive or contains a misprint. cee_wks path.h 62

static inline bool
HasUncPrefix(LPCWSTR wzPath)
{
  _ASSERTE(!clr::str::IsNullOrEmpty(wzPath));
  return wzPath[0] != W('') && wzPath[0] == W('\')
      && wzPath[1] != W('') && wzPath[1] == W('\')
      && wzPath[2] != W('') && wzPath[2] != W('?');
}

Эту функцию можно упросить до такого варианта:

static inline bool
HasUncPrefix(LPCWSTR wzPath)
{
  _ASSERTE(!clr::str::IsNullOrEmpty(wzPath));
  return wzPath[0] == W('\')
      && wzPath[1] == W('\')
      && wzPath[2] != W('')
      && wzPath[2] != W('?');
}

Ещё такое место:

• V590 Consider inspecting this expression. The expression is excessive or contains a misprint. cee_wks path.h 72

V571 Recurring check. The 'if (moduleInfo[MSCORWKS].baseAddr == 0)' condition was already verified in line 749. sos util.cpp 751

struct ModuleInfo
{
    ULONG64 baseAddr;
    ULONG64 size;
    BOOL hasPdb;
};

HRESULT CheckEEDll()
{
  ....
  // Do we have clr.dll
    if (moduleInfo[MSCORWKS].baseAddr == 0)          //<==
    {
        if (moduleInfo[MSCORWKS].baseAddr == 0)      //<==
            g_ExtSymbols->GetModuleByModuleName (
               MAIN_CLR_MODULE_NAME_A,0,NULL,
               &moduleInfo[MSCORWKS].baseAddr);
        if (moduleInfo[MSCORWKS].baseAddr != 0 &&    //<==
            moduleInfo[MSCORWKS].hasPdb == FALSE)
        {
          ....
        }
        ....
    }
  ....
}

Во втором случае 'baseAddr' уже можно не проверять.

V704 'this == nullptr' expression should be avoided — this expression is always false on newer compilers, because 'this' pointer can never be NULL. ClrJit gentree.cpp 12731

bool FieldSeqNode::IsFirstElemFieldSeq()
{
    if (this == nullptr)
        return false;
    return m_fieldHnd == FieldSeqStore::FirstElemPseudoField;
}

Согласно стандарту С++, указатель this никогда не может быть нулевым. О возможных последствиях такого кода можно подробно прочитать в описании диагностики V704. То, что такой код может работать корректно после компиляции компилятором Visual C++, это просто везение и полагаться на него по-честному нельзя.

Весь список: CoreCLR_V704.txt.

V668 There is no sense in testing the 'newChunk' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. ClrJit stresslog.h 552

FORCEINLINE BOOL GrowChunkList ()
{
  ....
  StressLogChunk * newChunk = new StressLogChunk (....);
  if (newChunk == NULL)
  {
    return FALSE;
  }
  ....
}

Если оператор 'new' не смог выделить память, то согласно стандарту языка Си++, генерируется исключение std::bad_alloc(). Таким образом проверять указатель на равенство нулю не имеет смысла.

Лучше проверить такие места, вот полный список: CoreCLR_V668.txt.

Заключение

Недавно открытый проект CoreCLR хороший пример того, как может выглядеть софт с закрытым исходным кодом. На эту тему постоянно ведутся дискуссии и вот вам ещё один повод для размышлений и обсуждений.

Для нас же важно, что в любом большом проекте можно найти какие-то ошибки и самое лучшее применение статическому анализатору – это регулярные проверки. Не ленитесь, скачайте PVS-Studio и проверьте свой проект.

Эта статья на английском

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. PVS-Studio: 25 Suspicious Code Fragments in CoreCLR.

Автор: SvyatoslavMC

Источник