ChakraCore:Microsoft EdgeのJavaScriptエンジンを確認する

2015年12月、JSConf USカンファレンスで、開発者はMicrosoft Edgeで実行されているChakra JavaScriptエンジンの主要コンポーネントのソースコードを開く予定であることを発表しました。 最近、MITライセンスに基づくChackraCoreのソースコードがGitHubの対応するリポジトリに公開されました。 この記事では、PVS-Studio静的アナライザーを使用して、プロジェクトで何か面白いものを見つけたことがわかります。



はじめに



ChakraCoreは、HTML / CSS / JSで記述されたMicrosoft EdgeおよびWindowsアプリケーションを実行する高性能JavaScriptエンジンであるChakraのコアコンポーネントです。 ChakraCoreは、x86 / x64 / ARMのJavaScript JITコンパイル、ガベージコレクション、および最新のJavaScript機能を幅広くサポートしています。



PVS-Studioは、C、C ++、C#で記述されたプログラムのソースコードのエラーを検出するための静的アナライザーです。 PVS-Studioツールは、最新のアプリケーションの開発者向けであり、Visual Studio 2010-2015環境に統合されます。



次の開いているプロジェクトのチェックに関する記事を準備する際に、静的アナライザーが提供するすべての警告に関する情報を決して提供しません。 したがって、プロジェクトの作成者が独自に分析を実行し、アナライザーによって発行されたすべてのメッセージを調査することをお勧めします。 しばらくの間、オープンソースプロジェクトの開発者への鍵を提供します。



その他のエラー











V501 「|-」の左側と右側に同一のサブ式「this-> propId == Js :: PropertyIds :: _ superReferenceSymbol」があります 演算子。 diagobjectmodel.cpp 123



IDiagObjectModelDisplay * ResolvedObject::CreateDisplay() { .... if (this->isConst || this->propId == Js::PropertyIds::_superReferenceSymbol || this->propId == Js::PropertyIds::_superReferenceSymbol) { pOMDisplay->SetDefaultTypeAttribute(....); } .... }
      
      





条件には2つの同一のチェックがあります。 おそらく、コードを記述するときに、たとえば、「Js :: PropertyIds :: _superCtorReferenceSymbol」ではなく、IntelliSenseメニューで同じ定数が誤って選択されました。



V501 「==」演算子の左右には、同一のサブ式「GetVarSymID(srcIndexOpnd-> GetStackSym())」があります。 globopt.cpp 20795



 void GlobOpt::EmitMemop(....) { .... IR::RegOpnd *srcBaseOpnd = nullptr; IR::RegOpnd *srcIndexOpnd = nullptr; IRType srcType; GetMemOpSrcInfo(...., srcBaseOpnd, srcIndexOpnd, srcType); Assert(GetVarSymID(srcIndexOpnd->GetStackSym()) == // <= GetVarSymID(srcIndexOpnd->GetStackSym())); // <= .... }
      
      





さらに2つの同じ比較。 おそらく、彼らは「srcIndexOpnd-> GetStackSym()」と「srcBaseOpnd-> GetStackSym()」を比較したいと考えていました。



V517 「if(A){...} else if(A){...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 行を確認してください:3220、3231。lower.cpp 3220



 bool Lowerer::GenerateFastBrSrEq(...., IR::RegOpnd * srcReg1, IR::RegOpnd * srcReg2, ....) { if (srcReg2 && IsConstRegOpnd(srcReg2)) { .... } else if (srcReg1 && IsConstRegOpnd(srcReg1)) { .... } else if (srcReg2 && (srcReg2->m_sym->m_isStrConst)) { .... } else if (srcReg1 && (srcReg1->m_sym->m_isStrConst)) // <= { .... } else if (srcReg2 && (srcReg2->m_sym->m_isStrEmpty)) { .... } else if (srcReg1 && (srcReg1->m_sym->m_isStrConst)) // <= { .... } return false; }
      
      





条件ステートメントのカスケードで2つの同一のチェックが見つかりました。その結果、最後の条件のコードブロックは制御されません。 提示された例の完全なコードは非常に大きく、タイプミスに気付くことは困難です。 これは、プログラマーがすぐに疲れて注意を失ってしまう同じタイプのコードを操作するときに、静的アナライザーがどのように役立つかを示す良い例です。



おそらく、彼らは次のような最後の2つの条件を書きたいと思っていました。



 .... else if (srcReg2 && (srcReg2->m_sym->m_isStrEmpty)) { .... } else if (srcReg1 && (srcReg1->m_sym-> m_isStrEmpty)) // <= { .... }
      
      





V713ポインターscriptContextは、同じ論理式のnullptrに対して検証される前に、論理式で使用されました。 diaghelpermethodwrapper.cpp 214



 template <bool doCheckParentInterpreterFrame> void HandleHelperOrLibraryMethodWrapperException(....) { .... if (!exceptionObject->IsDebuggerSkip() || exceptionObject == scriptContext->GetThreadContext()->.... || exceptionObject == scriptContext->GetThreadContext()->.... || !scriptContext) // <= { throw exceptionObject->CloneIfStaticExceptionObject(....); } .... }
      
      





scriptContextポインターの逆参照は、その有効性がチェックされる前に行われます。 運のおかげで、そのような間違いはまだ現れておらず、気づかれていません。 このようなエラーは、コード内で非常に長い間存続し、まれな非定型的な状況で現れます。



V570 「this-> isInlined」変数はそれ自体に割り当てられます。 functioncodegenjittimedata.h 625



 void SetupRecursiveInlineeChain( Recycler *const recycler, const ProfileId profiledCallSiteId) { if (!inlinees) { inlinees = RecyclerNewArrayZ(....); } inlinees[profiledCallSiteId] = this; inlineeCount++; this->isInlined = isInlined; // <= }
      
      





ブール変数「isInlined」に同じ値が設定されていることは非常に疑わしいです。 おそらく彼らは何か他のものを書きたかった。



変数を自分自身に割り当てる別の場所:





V590 'sub [i]!='-'&& sub [i] ==' / ''式の検査を検討してください。 表現が過剰であるか、誤植が含まれています。 rl.cpp 1388



 const char * stristr ( const char * str, const char * sub ) { .... for (i = 0; i < len; i++) { if (tolower(str[i]) != tolower(sub[i])) { if ((str[i] != '/' && str[i] != '-') || (sub[i] != '-' && sub[i] == '/')) { / <= // if the mismatch is not between '/' and '-' break; } } } .... }
      
      





アナライザーは、条件式(sub [i]!= '-')の一部がテストの結果に影響しないことを検出しました。 これを検証するために、真理値表を作成します。 おそらく何らかのタイプミスがありますが、正しいコードはどうあるべきか、答えるのは難しいと思います。











V603オブジェクトは作成されましたが、使用されていません。 コンストラクタを呼び出す場合は、「this-> StringCopyInfo :: StringCopyInfo(....)」を使用する必要があります。 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(); }
      
      





プログラマーは、コンストラクターを明示的に呼び出してオブジェクトを初期化しようとすると、多くの場合間違いを犯します。 この例では、タイプ「StringCopyInfo」の新しい名前のないオブジェクトが作成され、その後破棄されます。 その結果、クラスフィールドは初期化されません。



正しいオプションは、初期化関数を作成し、コンストラクターからこの場所で呼び出すことです。



V610未定義の動作。 シフト演算子「<<」を確認してください。 左のオペランド '-1'は負です。 constants.h 39



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





現在のC ++言語標準によれば、負の数をシフトすると未定義の動作が発生します。



V557配列のオーバーランが可能です。 「i」インデックスの値は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]); } .... } .... }
      
      





アナライザーは、配列外のインデックス出口を検出しました。 実際、for()ループは9回の反復を実行し、「TestInfoEnvLstFmt []」配列には8つの要素しかありません。



ほとんどの場合、最後に別のNULLを追加するのを忘れていました。



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





しかし、おそらく彼らはアレイの真ん中のいくつかの行を逃しました!



危険なポインター











診断V595は、ゼロがチェックされる前にポインターが逆参照されるコードのセクションを検出します。 通常、チェック対象のプロジェクトには常にこのような警告が多数あります。 エラーのデータベースでは、発見された欠点の数で最初になります( 例を参照)。 しかし、実際には、V595診断はプロジェクトから多くの例を書くのが少し退屈です。 また、関数コードでは、ポインターのチェックと間接参照が非常に遠くなる可能性があります。互いに数十行または数百行もあるため、記事のエラーの説明が複雑になります。



次に、ポインターを操作するときにエラーが発生する可能性が最も高い、最も短くて最も説明的なコード例をいくつか説明します。



V595 nullptrに対して検証される前に、「instrLd」ポインターが使用されました。 行をチェック: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; } .... }
      
      





「instrLd」という名前のポインターに注意してください。 最初の条件では、このポインターの逆参照はゼロのチェックとペアになりますが、2番目の条件ではこれを行うのを忘れたため、nullポインターの逆参照が発生したときに状況が発生する可能性があります。



V595 nullsrcに対して検証される前に、「src2Val」ポインターが使用されました。 行を確認: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; } .... }
      
      





src2Valポインターは関数の先頭で使用されますが、開発者はポインターがゼロかどうかを積極的にチェックし始めました。



V595 nullptrに対して検証される前に、「m_lastInstr」ポインターが使用されました。 行をチェック: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; .... }
      
      





ポインターの別の不正確な使用。潜在的にnullになる可能性があります。



同様の場所のリスト:





リストには、最も単純で明確な例が含まれています。 そのような場所をすべて調べるには、開発者はアナライザレポートを自分で調べる必要があります。



V522 nullポインター「tempNumberTracker」の参照が行われる場合があります。 backwardpass.cpp 578



 void BackwardPass::MergeSuccBlocksInfo(BasicBlock * block) { TempNumberTracker * tempNumberTracker = nullptr; // <= line 346 .... if (!block->isDead) { .... if(!IsCollectionPass()) { .... if (this->DoMarkTempNumbers()) { tempNumberTracker = JitAnew(....); // <= line 413 } .... .... if (blockSucc->tempNumberTracker != nullptr) { .... tempNumberTracker->MergeData(....); // <= line 578 if (deleteData) { blockSucc->tempNumberTracker = nullptr; } } .... }
      
      





別の診断の例だけでなく、ポインターについて。 ここに関数MergeSuccBlocksInfo()のフラグメントがあります。これは非常に長く、707行です。 しかし、静的解析の助けを借りて、「tempNumberTracker」ポインターを見つけることができました。このポインターの初期化は、多くの条件が原因で失敗する可能性があります。 その結果、状況が失敗した場合、nullポインターは逆参照されます。



やめて Assertをチェックしてください!











プログラムに配置されたアサートは、開発者が、正常に動作しているプログラムに対して何らかの式が真であると仮定することを示します。 しかし、そのような「成功した」チェックは信頼できますか?



V547式 'srcIndex-src-> left> = 0'は常にtrueです。 符号なしの型の値は常に> = 0です。sparsearraysegment.inl355



 class SparseArraySegmentBase { public: static const uint32 MaxLength; .... uint32 size; .... } template<typename T> SparseArraySegment<T>* SparseArraySegment<T>::CopySegment(...., uint32 srcIndex, ....) { .... AssertMsg(srcIndex - src->left >= 0, // <= "src->left > srcIndex resulting in \ negative indexing of src->elements"); js_memcpy_s(dst->elements + dstIndex - dst->left, sizeof(T) * inputLen, src->elements + srcIndex - src->left, sizeof(T) * inputLen); return dst; }
      
      





「srcIndex-src-> left> = 0」という比較に注意してください。 2つの符号なし数値の差は、常にゼロ以上になります。 さらに、この式は、メモリを操作する関数で使用されます。 結果は、プログラマが期待したものとは異なる場合があります。



V547式は常に真です。 ここでは、おそらく「&&」演算子を使用する必要があります。 bytecodegenerator.cpp 805



 void ByteCodeGenerator::AssignRegister(Symbol *sym) { AssertMsg(sym->GetDecl() == nullptr || sym->GetDecl()->nop != knopConstDecl || // <= sym->GetDecl()->nop != knopLetDecl, "...."); // <= if (sym->GetLocation() == Js::Constants::NoRegister) { sym->SetLocation(NextVarRegister()); } }
      
      





このアサートでは、一部の値が部分的にテストされます。 「sym-> GetDecl()== nullptr」という仮定がfalseの場合、次の条件は常にtrueです。 これを確認するには、真理値表を作成します。











V547式「callSiteId> = 0」は常にtrueです。 符号なしの型の値は常に> = 0です。inline.cpp 1181



 typedef uint16 ProfileId; Func * Inline::BuildInlinee(Js::FunctionBody* funcBody, ....) { .... Js::ProfileId callSiteId = static_cast<Js::ProfileId>(....); Assert(callSiteId >= 0); .... }
      
      





この場所と他の2つの場所で、アナライザーは符号なしの数値とゼロの誤った比較を見つけました。





おわりに



Microsoftは、無料ライセンスの下でプロジェクトを開く傾向があります。 私たちにとって、これは新しいプロジェクトでのアナライザーの追加テストであり、このような有名なソフトウェアメーカーのプロジェクトでの静的分析の有用性と有効性を実証する機会でもあります。



.NET CoreCLR、.NET CoreFX、Microsoft Code Contractsなど、Microsoftの他のプロジェクトを含む、 実績のあるすべてのプロジェクトのリストに興味があるかもしれません。







英語を話す聴衆とこの記事を共有したい場合は、翻訳へのリンクを使用してください:Svyatoslav Razmyslov。 ChakraCore:Microsoft Edge用のJavaScriptエンジンの分析



記事を読んで質問がありますか?
多くの場合、記事には同じ質問が寄せられます。 ここで回答を集めました: PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。




All Articles