diff --git a/src/InstrumentationEngine.Lib/DataContainer.cpp b/src/InstrumentationEngine.Lib/DataContainer.cpp index 6032c6fe..2ae62540 100644 --- a/src/InstrumentationEngine.Lib/DataContainer.cpp +++ b/src/InstrumentationEngine.Lib/DataContainer.cpp @@ -4,7 +4,7 @@ #include "stdafx.h" #include "DataContainer.h" -MicrosoftInstrumentationEngine::CDataContainer::CDataContainer() +MicrosoftInstrumentationEngine::CDataContainer::CDataContainer() : m_dataContainerMap(1) { DEFINE_REFCOUNT_NAME(CDataContainer); @@ -32,27 +32,51 @@ MicrosoftInstrumentationEngine::CDataContainer::~CDataContainer() HRESULT MicrosoftInstrumentationEngine::CDataContainer::SetDataItem( _In_ const GUID* pComponentId, _In_ const GUID* pObjectGuid, - _In_ IUnknown* pDataItem + _In_opt_ IUnknown* pDataItem ) { HRESULT hr = S_OK; CCriticalSectionHolder lock(&m_cs); - CAtlMap>* pMap; + CAtlMap>* pMap = nullptr; if (m_dataContainerMap.Lookup(*pComponentId, pMap) == false) { - pMap = new CAtlMap>; - if (!pMap) + // Only create the new map if we aren't setting the + // data item to null. + if (pDataItem != nullptr) { - return E_OUTOFMEMORY; - } + pMap = new CAtlMap>(1); + if (!pMap) + { + return E_OUTOFMEMORY; + } - m_dataContainerMap.SetAt(*pComponentId, pMap); + m_dataContainerMap.SetAt(*pComponentId, pMap); + } } - pMap->SetAt(*pObjectGuid, CComPtr(pDataItem)); + if (pMap != nullptr) + { + // if the data item is null, free memory. + if (pDataItem == nullptr) + { + pMap->RemoveKey(*pObjectGuid); + + if (pMap->GetCount() == 0) + { + m_dataContainerMap.RemoveKey(*pComponentId); + + // need to manually delete the map. + delete pMap; + } + } + else + { + pMap->SetAt(*pObjectGuid, CComPtr(pDataItem)); + } + } return hr; } diff --git a/src/InstrumentationEngine.Lib/DataContainer.h b/src/InstrumentationEngine.Lib/DataContainer.h index a26f0fef..6772c08c 100644 --- a/src/InstrumentationEngine.Lib/DataContainer.h +++ b/src/InstrumentationEngine.Lib/DataContainer.h @@ -23,7 +23,7 @@ namespace MicrosoftInstrumentationEngine virtual HRESULT __stdcall SetDataItem( _In_ const GUID* componentId, _In_ const GUID* objectGuid, - _In_ IUnknown* pDataItem + _In_opt_ IUnknown* pDataItem ); virtual HRESULT __stdcall GetDataItem( diff --git a/src/InstrumentationEngine/BranchTargetInfo.cpp b/src/InstrumentationEngine/BranchTargetInfo.cpp new file mode 100644 index 00000000..791df0b9 --- /dev/null +++ b/src/InstrumentationEngine/BranchTargetInfo.cpp @@ -0,0 +1,148 @@ +#include "stdafx.h" +#include "refcount.h" +#include "Instruction.h" +#include "BranchTargetInfo.h" + +namespace MicrosoftInstrumentationEngine +{ + // Note: we construct m_branches with only one initial bucket to save memory and because + // most instructions will not be targeted by that many branches. + CBranchTargetInfo::CBranchTargetInfo(_In_ CInstruction* pInstruction) : m_pInstruction(pInstruction), m_branches(1) + { + } + + HRESULT CBranchTargetInfo::GetInstance(_In_ CInstruction* pInstruction, _Outptr_ CBranchTargetInfo** ppResult) + { + IfNullRet(pInstruction); + IfNullRet(ppResult); + + *ppResult = nullptr; + + const GUID* uuid = &__uuidof(CBranchTargetInfo); + CComPtr pUnknown; + HRESULT hr; + // Don't assert, it is common for there to not be target info. + IfFailRetNoLog(pInstruction->GetDataItem(uuid, uuid, &pUnknown)); + CComPtr pTargetInfo; + IfFailRet(pUnknown.QueryInterface(&pTargetInfo)); + + IfFalseRet(pTargetInfo != nullptr, E_UNEXPECTED); + *ppResult = pTargetInfo.Detach(); + + return S_OK; + } + + HRESULT CBranchTargetInfo::GetOrCreateInstance(_In_ CInstruction* pInstruction, _Out_ CBranchTargetInfo** ppResult) + { + IfNullRet(pInstruction); + IfNullRet(ppResult); + if (!SUCCEEDED(GetInstance(pInstruction, ppResult))) + { + CComPtr pResult; + pResult.Attach(new CBranchTargetInfo(pInstruction)); + const GUID* uuid = &__uuidof(CBranchTargetInfo); + pInstruction->SetDataItem(uuid, uuid, pResult); + *ppResult = pResult.Detach(); + } + + return S_OK; + } + + HRESULT CBranchTargetInfo::SetBranchTarget(_In_ CInstruction* pBranch, _In_opt_ CInstruction* pTarget, _In_opt_ CInstruction* pOldTarget) + { + IfNullRet(pBranch); + + HRESULT hr; + + if (pOldTarget != nullptr) + { + CComPtr pOldInfo; + + // There may not be an instance of CBranchTargetInfo if we are in the + // middle of retargetting a branch. That is OK, just ignore it. + if (SUCCEEDED(CBranchTargetInfo::GetInstance(pOldTarget, &pOldInfo))) + { + pOldInfo->Remove(pBranch); + } + } + + if (pTarget != nullptr) + { + CComPtr pInfo; + IfFailRet(CBranchTargetInfo::GetOrCreateInstance(pTarget, &pInfo)); + pInfo.p->m_branches.emplace(pBranch); + } + + return S_OK; + } + + HRESULT CBranchTargetInfo::RetargetBranches(_In_ CInstruction* pOriginalInstruction, _In_ CInstruction* pNewInstruction) + { + IfNullRet(pOriginalInstruction); + IfNullRet(pNewInstruction); + + CComPtr pTargetInfo; + if (SUCCEEDED(CBranchTargetInfo::GetInstance(pOriginalInstruction, &pTargetInfo))) + { + return pTargetInfo->Retarget(pNewInstruction); + } + return S_OK; + } + + HRESULT CBranchTargetInfo::Retarget(_In_ CInstruction* pNewInstruction) + { + HRESULT hr = S_OK; + + // Remove this CBranchTargetInfo from the data container. If a new one + // is needed, it will be regenerated when branch targets are set. + Disconnect(); + for (CInstruction* pInstr : m_branches) + { + if (pInstr->GetIsBranchInternal()) + { + CComPtr pBranch; + IfFailRet(pInstr->QueryInterface(__uuidof(CBranchInstruction), (void**)&pBranch)); + + // This indicates that the new instruction targets this instruction. + // If we were to retarget it, we would cause an infinite loop. + if (pBranch != pNewInstruction) + { + IfFailRet(pBranch->SetBranchTarget(pNewInstruction)); + } + else + { + // reset the branch target, since the disconnection + // above will have removed it. + IfFailRet(pBranch->SetBranchTarget(m_pInstruction)); + } + } + else if (pInstr->GetIsSwitchInternal()) + { + CComPtr pSwitch; + IfFailRet(pInstr->QueryInterface(__uuidof(ISwitchInstruction), (void**)&pSwitch)); + IfFalseRet(pSwitch != nullptr, E_UNEXPECTED); + IfFailRet(pSwitch->ReplaceBranchTarget(m_pInstruction, pNewInstruction)); + } + } + + return S_OK; + } + + void CBranchTargetInfo::Remove(_In_ CInstruction* pOldBranch) + { + m_branches.erase(pOldBranch); + if (m_branches.empty()) + { + Disconnect(); + } + } + + void CBranchTargetInfo::Disconnect() + { + const GUID* uuid = &__uuidof(CBranchTargetInfo); + + // break cycles. + m_pInstruction->SetDataItem(uuid, uuid, nullptr); + } +} + diff --git a/src/InstrumentationEngine/BranchTargetInfo.h b/src/InstrumentationEngine/BranchTargetInfo.h new file mode 100644 index 00000000..60039812 --- /dev/null +++ b/src/InstrumentationEngine/BranchTargetInfo.h @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#pragma once + + +namespace MicrosoftInstrumentationEngine +{ + class __declspec(uuid("0E23A44A-6700-4810-889C-8067638C512E")) + CBranchTargetInfo : public IUnknown, virtual CModuleRefCount + { + private: + + // This is a raw pointer to avoid circular references. + CInstruction* m_pInstruction; + + // Using raw pointers for speed. The graph should keep these pointers + // alive. + std::unordered_set m_branches; + + public: + DEFINE_DELEGATED_REFCOUNT_ADDREF(CBranchTargetInfo); + DEFINE_DELEGATED_REFCOUNT_RELEASE(CBranchTargetInfo); + STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void** ppvObject) override + { + return ImplQueryInterface( + static_cast(this), + this, + riid, + ppvObject + ); + } + + public: + static HRESULT GetInstance(_In_ CInstruction* pInstruction, _Outptr_ CBranchTargetInfo** ppResult); + static HRESULT GetOrCreateInstance(_In_ CInstruction* pInstruction, _Out_ CBranchTargetInfo** ppResult); + static HRESULT SetBranchTarget(_In_ CInstruction* pBranch, _In_opt_ CInstruction* pNewTarget, _In_opt_ CInstruction* pOldTarget); + static HRESULT RetargetBranches(_In_ CInstruction* pOriginalInstruction, _In_ CInstruction* pNewInstruction); + void Disconnect(); + + virtual ~CBranchTargetInfo() = default; + + CBranchTargetInfo(_In_ CInstruction* pInstruction); + HRESULT Retarget(_In_ CInstruction* pNewInstruction); + void Remove(_In_ CInstruction* pOldBranch); + }; +} \ No newline at end of file diff --git a/src/InstrumentationEngine/CMakeLists.txt b/src/InstrumentationEngine/CMakeLists.txt index d13f489c..2e7709e1 100644 --- a/src/InstrumentationEngine/CMakeLists.txt +++ b/src/InstrumentationEngine/CMakeLists.txt @@ -20,6 +20,7 @@ set(src_files ./AssemblyInfo.cpp #./AssemblyInjector.cpp #./AtlModule.cpp + ./BranchTargetInfo.cpp ./CompositeType.cpp ./CorHeaders.cpp ./CorMethodMalloc.cpp diff --git a/src/InstrumentationEngine/ExceptionClause.cpp b/src/InstrumentationEngine/ExceptionClause.cpp index cf2f3737..c65c62de 100644 --- a/src/InstrumentationEngine/ExceptionClause.cpp +++ b/src/InstrumentationEngine/ExceptionClause.cpp @@ -24,11 +24,11 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::InitializeFromSmall( m_flags = static_cast(pSmallClause->Flags); #endif - IfFailRet(pInstructionGraph->GetInstructionAtOffset(pSmallClause->TryOffset, &m_pTryFirstInstruction)); + IfFailRet(pInstructionGraph->GetInstructionAtOffsetInternal(pSmallClause->TryOffset, &m_pTryFirstInstruction)); IfFailRet(pInstructionGraph->GetInstructionAtEndOffset(pSmallClause->TryOffset + pSmallClause->TryLength, &m_pTryLastInstruction)); // Is it legal to not have a handler for a filter? Not doing if fail ret here. - pInstructionGraph->GetInstructionAtOffset(pSmallClause->HandlerOffset, &m_pHandlerFirstInstruction); + pInstructionGraph->GetInstructionAtOffsetInternal(pSmallClause->HandlerOffset, &m_pHandlerFirstInstruction); pInstructionGraph->GetInstructionAtEndOffset(pSmallClause->HandlerOffset + pSmallClause->HandlerLength, &m_pHandlerLastInstruction); if (m_flags == COR_ILEXCEPTION_CLAUSE_NONE) @@ -39,7 +39,7 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::InitializeFromSmall( else if (m_flags == COR_ILEXCEPTION_CLAUSE_FILTER) { //filter block - get the filter first instruction - IfFailRet(pInstructionGraph->GetInstructionAtOffset(pSmallClause->FilterOffset, &m_pFilterFirstInstruction)); + IfFailRet(pInstructionGraph->GetInstructionAtOffsetInternal(pSmallClause->FilterOffset, &m_pFilterFirstInstruction)); } CLogging::LogMessage(_T("End CExceptionClause::InitializeFromSmall")); @@ -57,11 +57,11 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::InitializeFromFat( m_flags = pFatClause->Flags; - IfFailRet(pInstructionGraph->GetInstructionAtOffset(pFatClause->TryOffset, &m_pTryFirstInstruction)); + IfFailRet(pInstructionGraph->GetInstructionAtOffsetInternal(pFatClause->TryOffset, &m_pTryFirstInstruction)); IfFailRet(pInstructionGraph->GetInstructionAtEndOffset(pFatClause->TryOffset + pFatClause->TryLength, &m_pTryLastInstruction)); // Is it legal to not have a handler for a filter? Not doing if fail ret here. - pInstructionGraph->GetInstructionAtOffset(pFatClause->HandlerOffset, &m_pHandlerFirstInstruction); + pInstructionGraph->GetInstructionAtOffsetInternal(pFatClause->HandlerOffset, &m_pHandlerFirstInstruction); pInstructionGraph->GetInstructionAtEndOffset(pFatClause->HandlerOffset + pFatClause->HandlerLength, &m_pHandlerLastInstruction); if (m_flags == COR_ILEXCEPTION_CLAUSE_NONE) @@ -72,7 +72,7 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::InitializeFromFat( else if (m_flags == COR_ILEXCEPTION_CLAUSE_FILTER) { //filter block - get the filter first instruction - IfFailRet(pInstructionGraph->GetInstructionAtOffset(pFatClause->FilterOffset, &m_pFilterFirstInstruction)); + IfFailRet(pInstructionGraph->GetInstructionAtOffsetInternal(pFatClause->FilterOffset, &m_pFilterFirstInstruction)); } CLogging::LogMessage(_T("End CExceptionClause::InitializeFromSmall")); @@ -95,18 +95,15 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::RenderExceptionClause( DWORD firstTryOffset = 0; DWORD lastTryOffset = 0; - DWORD lastTrySize = 0; + DWORD lastTrySize = m_pTryLastInstruction->GetInstructionSize();; IfFailRet(m_pTryFirstInstruction->GetOffset(&firstTryOffset)); IfFailRet(m_pTryLastInstruction->GetOffset(&lastTryOffset)); - IfFailRet(CInstruction::GetInstructionSize(m_pTryLastInstruction, &lastTrySize)); DWORD firstHandlerOffset = 0; DWORD lastHandlerOffset = 0; - DWORD lastHandlerSize = 0; + DWORD lastHandlerSize = m_pHandlerLastInstruction->GetInstructionSize(); IfFailRet(m_pHandlerFirstInstruction->GetOffset(&firstHandlerOffset)); IfFailRet(m_pHandlerLastInstruction->GetOffset(&lastHandlerOffset)); - IfFailRet(CInstruction::GetInstructionSize(m_pHandlerLastInstruction, &lastHandlerSize)); - pEHClause->TryOffset = firstTryOffset; pEHClause->TryLength = (lastTryOffset + lastTrySize) - firstTryOffset; @@ -308,7 +305,14 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::SetTryFirstInstruction HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CExceptionClause::SetTryFirstInstruction")); - m_pTryFirstInstruction = pInstruction; + if (pInstruction != nullptr) + { + IfFailRet(pInstruction->QueryInterface(__uuidof(CInstruction), (void**)&m_pTryFirstInstruction)); + } + else + { + m_pTryFirstInstruction = nullptr; + } CLogging::LogMessage(_T("End CExceptionClause::SetTryFirstInstruction")); return hr; @@ -319,7 +323,14 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::SetTryLastInstruction( HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CExceptionClause::SetTryLastInstruction")); - m_pTryLastInstruction = pInstruction; + if (pInstruction != nullptr) + { + IfFailRet(pInstruction->QueryInterface(__uuidof(CInstruction), (void**)&m_pTryLastInstruction)); + } + else + { + m_pTryLastInstruction = nullptr; + } CLogging::LogMessage(_T("End CExceptionClause::SetTryLastInstruction")); return hr; @@ -330,7 +341,14 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::SetHandlerFirstInstruc HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CExceptionClause::SetHandlerFirstInstruction")); - m_pHandlerFirstInstruction = pInstruction; + if (pInstruction != nullptr) + { + IfFailRet(pInstruction->QueryInterface(__uuidof(CInstruction), (void**)&m_pHandlerFirstInstruction)); + } + else + { + m_pHandlerFirstInstruction = nullptr; + } CLogging::LogMessage(_T("End CExceptionClause::SetHandlerFirstInstruction")); return hr; @@ -341,7 +359,14 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::SetHandlerLastInstruct HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CExceptionClause::SetHandlerLastInstruction")); - m_pHandlerLastInstruction = pInstruction; + if (pInstruction != nullptr) + { + IfFailRet(pInstruction->QueryInterface(__uuidof(CInstruction), (void**)&m_pHandlerLastInstruction)); + } + else + { + m_pHandlerLastInstruction = nullptr; + } CLogging::LogMessage(_T("End CExceptionClause::SetHandlerLastInstruction")); return hr; @@ -352,7 +377,14 @@ HRESULT MicrosoftInstrumentationEngine::CExceptionClause::SetFilterFirstInstruct HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CExceptionClause::SetFilterFirstInstruction")); - m_pFilterFirstInstruction = pInstruction; + if (pInstruction != nullptr) + { + IfFailRet(pInstruction->QueryInterface(__uuidof(CInstruction), (void**)&m_pFilterFirstInstruction)); + } + else + { + m_pFilterFirstInstruction = nullptr; + } CLogging::LogMessage(_T("End CExceptionClause::SetFilterFirstInstruction")); return hr; diff --git a/src/InstrumentationEngine/ExceptionClause.h b/src/InstrumentationEngine/ExceptionClause.h index 3cb263ed..dbe4c4b2 100644 --- a/src/InstrumentationEngine/ExceptionClause.h +++ b/src/InstrumentationEngine/ExceptionClause.h @@ -18,11 +18,11 @@ namespace MicrosoftInstrumentationEngine { private: CorExceptionFlag m_flags; - CComPtr m_pTryFirstInstruction; - CComPtr m_pTryLastInstruction; - CComPtr m_pHandlerFirstInstruction; - CComPtr m_pHandlerLastInstruction; - CComPtr m_pFilterFirstInstruction; + CComPtr m_pTryFirstInstruction; + CComPtr m_pTryLastInstruction; + CComPtr m_pHandlerFirstInstruction; + CComPtr m_pHandlerLastInstruction; + CComPtr m_pFilterFirstInstruction; mdToken m_ExceptionHandlerType; diff --git a/src/InstrumentationEngine/Instruction.cpp b/src/InstrumentationEngine/Instruction.cpp index 5258275f..ef9093e6 100644 --- a/src/InstrumentationEngine/Instruction.cpp +++ b/src/InstrumentationEngine/Instruction.cpp @@ -3,6 +3,7 @@ #include "stdafx.h" #include "Instruction.h" +#include "BranchTargetInfo.h" HRESULT MicrosoftInstrumentationEngine::CInstruction::LogInstruction(bool ignoreTest) { @@ -111,9 +112,6 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::InstructionFromBytes( { HRESULT hr = S_OK; *ppInstruction = NULL; - - CLogging::LogMessage(_T("Starting CInstruction::InstructionFromBytes")); - ILOrdinalOpcode opcode; CInstruction::OrdinalOpcodeFromBytes(pCode, pEndOfCode, &opcode); @@ -193,15 +191,11 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::InstructionFromBytes( default: { - CLogging::LogMessage(_T("CInstruction::InstructionFromBytes - Incorrect operand type")); return E_FAIL; } } } } - - CLogging::LogMessage(_T("End CInstruction::InstructionFromBytes")); - return S_OK; } @@ -362,56 +356,37 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::SetIsRemoved() HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOffset(_Out_ DWORD* pdwOffset) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOffset")); - IfNullRetPointer(pdwOffset); IfFailRet(EnsureGraphUpdated()); *pdwOffset = m_offset; - - CLogging::LogMessage(_T("End CInstruction::GetOffset")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOriginalOffset(_Out_ DWORD* pdwOffset) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOriginalOffset")); - IfNullRetPointer(pdwOffset); IfFailRet(EnsureGraphUpdated()); *pdwOffset = m_origOffset; - - CLogging::LogMessage(_T("End CInstruction::GetOriginalOffset")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOpCodeName(_Out_ BSTR* pbstrName) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOpCodeName")); - IfNullRetPointer(pbstrName); CComBSTR bstrOpCodeName = s_ilOpcodeInfo[m_opcode].m_name; *pbstrName = bstrOpCodeName.Detach(); - - CLogging::LogMessage(_T("End CInstruction::GetOpCodeName")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOpCode(_Out_ ILOrdinalOpcode* pOpCode) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOpCode")); IfNullRetPointer(pOpCode); *pOpCode = m_opcode; - - CLogging::LogMessage(_T("End CInstruction::GetOpCode")); - return S_OK; } @@ -419,78 +394,54 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOpCodeName(_Out_ BSTR* HRESULT MicrosoftInstrumentationEngine::CInstruction::GetAlternateOrdinalOpcode(_Out_ ILOrdinalOpcode* pAlternative) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetAlternateOrdinalOpcode")); IfNullRetPointer(pAlternative); *pAlternative = s_ilOpcodeInfo[m_opcode].m_alternate; - - CLogging::LogMessage(_T("End CInstruction::GetAlternateOrdinalOpcode")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOpcodeFlags(_Out_ ILOpcodeFlags* pFlags) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetAlternateOrdinalOpcode")); IfNullRetPointer(pFlags); *pFlags = s_ilOpcodeInfo[m_opcode].m_flags; - - CLogging::LogMessage(_T("End CInstruction::GetAlternateOrdinalOpcode")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetInstructionLength(_Out_ DWORD* pdwLength) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetInstructionLength ")); IfNullRetPointer(pdwLength); *pdwLength = s_ilOpcodeInfo[m_opcode].m_opcodeLength + s_ilOpcodeInfo[m_opcode].m_operandLength; - - CLogging::LogMessage(_T("End CInstruction::GetInstructionLength ")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOpcodeLength(_Out_ DWORD* pdwLength) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOpcodeLength")); IfNullRetPointer(pdwLength); *pdwLength = s_ilOpcodeInfo[m_opcode].m_opcodeLength; - - CLogging::LogMessage(_T("End CInstruction::GetOpcodeLength")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOperandType(_Out_ ILOperandType* pType) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOpcodeLength")); IfNullRetPointer(pType); *pType = s_ilOpcodeInfo[m_opcode].m_type; - - CLogging::LogMessage(_T("End CInstruction::GetOpcodeLength")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOperandLength(_Out_ DWORD* pdwLength) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOpcodeLength")); IfNullRetPointer(pdwLength); *pdwLength = s_ilOpcodeInfo[m_opcode].m_operandLength; - - CLogging::LogMessage(_T("End CInstruction::GetOpcodeLength")); - return S_OK; } @@ -498,13 +449,9 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOperandLength(_Out_ DWO HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsNew(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetIsNew")); IfNullRetPointer(pbValue); *pbValue = m_instructionGeneration == InstructionGeneration::Generation_New ? TRUE : FALSE; - - CLogging::LogMessage(_T("End CInstruction::GetIsNew")); - return S_OK; } @@ -512,77 +459,44 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsNew(_Out_ BOOL* pbVal HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsRemoved(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetIsRemoved")); IfNullRetPointer(pbValue); *pbValue = m_bIsRemoved; - - CLogging::LogMessage(_T("End CInstruction::GetIsRemoved")); - - return S_OK; -} - -HRESULT MicrosoftInstrumentationEngine::CInstruction::GetInstructionSize(_In_ IInstruction* pInstruction, _Out_ DWORD* pdwSize) -{ - HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetInstructionSize")); - IfNullRetPointer(pInstruction); - IfNullRetPointer(pdwSize); - - DWORD dwOpCodeLength = 0; - IfFailRet(pInstruction->GetOpcodeLength(&dwOpCodeLength)); - - DWORD dwOperandLength = 0; - IfFailRet(pInstruction->GetOperandLength(&dwOperandLength)); - - *pdwSize = dwOpCodeLength + dwOperandLength; - - CLogging::LogMessage(_T("End CInstruction::GetInstructionSize")); - return S_OK; } -HRESULT MicrosoftInstrumentationEngine::CInstruction::GetInstructionSize(_Out_ DWORD* pdwSize) +DWORD MicrosoftInstrumentationEngine::CInstruction::GetInstructionSize() { - return GetInstructionSize(this, pdwSize); + // Optimization. GetInstructionSize() is not implemented as a combination of GetOpCodeLength() + GetOperandLength() because + // it is called many times inside tight loops. Removing the additional virtual function call reduces overhead, and + // allows better optimization on return values. + return s_ilOpcodeInfo[m_opcode].m_opcodeLength + s_ilOpcodeInfo[m_opcode].m_operandLength; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsBranch(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetIsBranch")); IfNullRetPointer(pbValue); *pbValue = GetIsBranchInternal(); - - CLogging::LogMessage(_T("End CInstruction::GetIsBranch")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsSwitch(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetIsSwitch")); IfNullRetPointer(pbValue); *pbValue = GetIsSwitchInternal(); - - CLogging::LogMessage(_T("End CInstruction::GetIsSwitch")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsCallInstruction(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetIsCallInstruction")); IfNullRetPointer(pbValue); *pbValue = (m_opcode == Cee_Call || m_opcode == Cee_Calli || m_opcode == Cee_Callvirt); - - CLogging::LogMessage(_T("End CInstruction::GetIsCallInstruction")); - return S_OK; } @@ -674,42 +588,32 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::GetIsFallThrough(_Out_ BOO HRESULT MicrosoftInstrumentationEngine::CInstruction::GetNextInstruction(_Out_ IInstruction** ppNextInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetNextInstruction")); IfNullRetPointer(ppNextInstruction); *ppNextInstruction = NULL; if (m_pNextInstruction == NULL) { - CLogging::LogMessage(_T("CInstruction::GetNextInstruction - no next instruction")); return E_FAIL; } *ppNextInstruction = (IInstruction*)(m_pNextInstruction.p); (*ppNextInstruction)->AddRef(); - - CLogging::LogMessage(_T("End CInstruction::GetNextInstruction")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetPreviousInstruction(_Out_ IInstruction** ppPrevInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetPreviousInstruction")); IfNullRetPointer(ppPrevInstruction); *ppPrevInstruction = NULL; if (m_pPreviousInstruction == NULL) { - CLogging::LogMessage(_T("CInstruction::GetPreviousInstruction - no previous instruction")); return E_FAIL; } *ppPrevInstruction = (IInstruction*)(m_pPreviousInstruction.p); (*ppPrevInstruction)->AddRef(); - - CLogging::LogMessage(_T("End CInstruction::GetPreviousInstruction")); - return S_OK; } @@ -717,112 +621,77 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::GetPreviousInstruction(_Ou HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOriginalNextInstruction(_Out_ IInstruction** ppNextInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOriginalNextInstruction")); IfNullRetPointer(ppNextInstruction); *ppNextInstruction = NULL; if (m_pOriginalNextInstruction == NULL) { - CLogging::LogMessage(_T("CInstruction::GetOriginalNextInstruction - no original next instruction")); return E_FAIL; } *ppNextInstruction = (IInstruction*)(m_pOriginalNextInstruction.p); (*ppNextInstruction)->AddRef(); - - CLogging::LogMessage(_T("End CInstruction::GetOriginalNextInstruction")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::GetOriginalPreviousInstruction(_Out_ IInstruction** ppPrevInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::GetOriginalPreviousInstruction")); IfNullRetPointer(ppPrevInstruction); *ppPrevInstruction = NULL; if (m_pOriginalPreviousInstruction == NULL) { - CLogging::LogMessage(_T("CInstruction::GetOriginalPreviousInstruction - no original previous instruction")); return E_FAIL; } *ppPrevInstruction = (IInstruction*)(m_pOriginalPreviousInstruction.p); (*ppPrevInstruction)->AddRef(); - - CLogging::LogMessage(_T("End CInstruction::GetOriginalPreviousInstruction")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::SetNextInstruction(_In_opt_ CInstruction* pInstruction, _In_ bool setOrig) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::SetNextInstruction")); - m_pNextInstruction = pInstruction; if (m_pOriginalNextInstruction == NULL && setOrig) { m_pOriginalNextInstruction = m_pNextInstruction; } - - CLogging::LogMessage(_T("End CInstruction::SetNextInstruction")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::SetPreviousInstruction(_In_opt_ CInstruction* pInstruction, _In_ bool setOrig) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::SetPreviousInstruction")); - m_pPreviousInstruction = pInstruction; if (m_pOriginalPreviousInstruction == NULL && setOrig) { m_pOriginalPreviousInstruction = m_pPreviousInstruction; } - - CLogging::LogMessage(_T("End CInstruction::SetPreviousInstruction")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::SetOriginalOffset(_In_ ULONG offset) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::SetOriginalOffset offset is %04x"), offset); - m_origOffset = offset; - - CLogging::LogMessage(_T("End CInstruction::SetOriginalOffset")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::SetOffset(_In_ ULONG offset) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::SetOffset offset is %04x"), offset); - m_offset = offset; - - CLogging::LogMessage(_T("End CInstruction::SetOffset")); - return S_OK; } HRESULT MicrosoftInstrumentationEngine::CInstruction::SetInstructionGeneration(_In_ InstructionGeneration instructionGeneration) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstruction::SetInstructionGeneration")); - m_instructionGeneration = instructionGeneration; - - CLogging::LogMessage(_T("End CInstruction::SetInstructionGeneration")); - return S_OK; } @@ -918,14 +787,9 @@ HRESULT MicrosoftInstrumentationEngine::COperandInstruction::InitializeFromBytes HRESULT MicrosoftInstrumentationEngine::COperandInstruction::GetOperandType(_Out_ enum ILOperandType* pType) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting COperandInstruction::GetOperandType")); IfNullRetPointer(pType); *pType = s_ilOpcodeInfo[m_opcode].m_type; - - CLogging::LogMessage(_T("End COperandInstruction::GetOperandType")); - return S_OK; } @@ -936,8 +800,6 @@ HRESULT MicrosoftInstrumentationEngine::COperandInstruction::GetOperandValue( ) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting COperandInstruction::GetOperandValue")); IfNullRetPointer(pBytes); if (dwSize < s_ilOpcodeInfo[m_opcode].m_operandLength) @@ -975,9 +837,6 @@ HRESULT MicrosoftInstrumentationEngine::COperandInstruction::GetOperandValue( return E_FAIL; } } - - CLogging::LogMessage(_T("End COperandInstruction::GetOperandValue")); - return hr; } @@ -987,8 +846,6 @@ HRESULT MicrosoftInstrumentationEngine::COperandInstruction::SetOperandValue( ) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting COperandInstruction::SetOperandValue")); IfNullRetPointer(pBytes); if (dwSize != s_ilOpcodeInfo[m_opcode].m_operandLength) @@ -1026,9 +883,6 @@ HRESULT MicrosoftInstrumentationEngine::COperandInstruction::SetOperandValue( return E_FAIL; } } - - CLogging::LogMessage(_T("End COperandInstruction::SetOperandValue")); - return S_OK; } @@ -1092,13 +946,9 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::InitializeFromBytes( HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::IsShortBranch(_Out_ BOOL* pbValue) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CBranchInstruction::IsShortBranch")); IfNullRetPointer(pbValue); *pbValue = (m_opcode < Cee_Br || m_opcode == Cee_Leave_S); - - CLogging::LogMessage(_T("End CBranchInstruction::IsShortBranch")); - return hr; } @@ -1107,8 +957,6 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::IsShortBranch(_Out_ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::ExpandBranch() { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CBranchInstruction::ExpandBranch")); - if (CInstruction::s_ilOpcodeInfo[m_opcode].m_operandLength == 1) { //Expand the branch to the long form @@ -1122,9 +970,6 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::ExpandBranch() m_opcode = (ILOrdinalOpcode) (m_opcode + (Cee_Br - Cee_Br_S)); } } - - CLogging::LogMessage(_T("End CBranchInstruction::ExpandBranch")); - return S_OK; } @@ -1132,23 +977,16 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::ExpandBranch() HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::GetBranchTarget(_Out_ IInstruction** ppTarget) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting CBranchInstruction::GetTargetOffset")); IfNullRetPointer(ppTarget); *ppTarget = (IInstruction*)m_pBranchTarget; (*ppTarget)->AddRef(); - - CLogging::LogMessage(_T("End CBranchInstruction::GetTargetOffset")); - return hr; } HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::GetTargetOffset(_Out_ DWORD* pOffset) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting CBranchInstruction::GetTargetOffset")); IfNullRetPointer(pOffset); *pOffset = 0; @@ -1160,20 +998,18 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::GetTargetOffset(_Out { *pOffset = m_decodedTargetOffset; } - - CLogging::LogMessage(_T("End CBranchInstruction::GetTargetOffset")); - return hr; } HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::SetBranchTarget(_In_ IInstruction* pInstruction) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting CBranchInstruction::SetBranchTarget")); IfNullRetPointer(pInstruction); - m_pBranchTarget = pInstruction; + CComPtr pOldInstruction = m_pBranchTarget; + m_pBranchTarget.Release(); + IfFailRet(CInstruction::CastTo(pInstruction, &m_pBranchTarget)); + IfFailRet(CBranchTargetInfo::SetBranchTarget(this, m_pBranchTarget, pOldInstruction)); if (m_pBranchTarget == nullptr) { @@ -1185,36 +1021,17 @@ HRESULT MicrosoftInstrumentationEngine::CBranchInstruction::SetBranchTarget(_In_ { m_pOrigBranchTarget = m_pBranchTarget; } - - CLogging::LogMessage(_T("End CBranchInstruction::SetBranchTarget")); - return hr; } -MicrosoftInstrumentationEngine::CSwitchInstruction::CSwitchInstruction( - _In_ ILOrdinalOpcode opcode, - _In_ bool isNew - ) : CInstruction(opcode, isNew) -{ - -} - MicrosoftInstrumentationEngine::CSwitchInstruction::CSwitchInstruction( _In_ ILOrdinalOpcode opcode, _In_ bool isNew, - _In_ DWORD cBranchTargets, - _In_reads_(cBranchTargets) IInstruction** ppBranchTargets - ) : CInstruction(opcode, isNew) + _In_ DWORD initialCount + ) : CInstruction(opcode, isNew), m_branchTargets(initialCount) { - m_branchTargets.reserve(cBranchTargets); - - for (DWORD i = 0; i < cBranchTargets; i++) - { - CComPtr pTarget = ppBranchTargets[i]; - m_branchTargets[i] = pTarget; - } } HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::InitializeFromBytes( @@ -1223,9 +1040,6 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::InitializeFromBytes( ) { HRESULT hr = S_OK; - - CLogging::LogMessage(_T("Starting CSwitchInstruction::Initialize")); - const ULONG* p = (ULONG*)(pCode + CInstruction::s_ilOpcodeInfo[m_opcode].m_opcodeLength); ULONG count = *p; // First U4 integer is the count of branch deltas p++; @@ -1241,9 +1055,6 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::InitializeFromBytes( m_origBranchTargetOffsets.push_back(*p); p++; } - - CLogging::LogMessage(_T("End CSwitchInstruction::Initialize")); - return hr; } @@ -1257,19 +1068,22 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::GetOperandLength(_Ou return hr; } +DWORD MicrosoftInstrumentationEngine::CSwitchInstruction::GetInstructionSize() +{ + // Optimization. GetInstructionSize() is not implemented as a combination of GetOpCodeLength() + GetOperandLength() because + // it is called many times inside tight loops. Removing the additional virtual function call reduces overhead, and + // allows better optimization on return values. + return s_ilOpcodeInfo[m_opcode].m_opcodeLength + (DWORD)((m_branchTargets.size() + 1) * sizeof(DWORD)); +} + HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::GetBranchTarget(_In_ DWORD index, _Out_ IInstruction** ppTarget) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::GetBranchTarget")); - *ppTarget = GetBranchTargetInternal(index); if (*ppTarget != NULL) { (*ppTarget)->AddRef(); } - - CLogging::LogMessage(_T("End CSwitchInstruction::GetBranchTarget")); - return hr; } @@ -1281,7 +1095,7 @@ IInstruction* MicrosoftInstrumentationEngine::CSwitchInstruction::GetBranchTarge return NULL; } - const CComPtr& pInstruction = m_branchTargets[index]; + const CComPtr& pInstruction = m_branchTargets[index]; if (pInstruction == NULL) { CLogging::LogError(_T("CSwitchInstruction::GetBranchTarget - branch target at index is null")); @@ -1297,9 +1111,12 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::SetBranchTarget(_In_ HRESULT hr = S_OK; CLogging::LogMessage(_T("Starting CSwitchInstruction::SetBranchTarget")); - m_branchTargets[index] = pTarget; + CComPtr oldTarget = m_branchTargets[index]; + CComPtr pNewTarget; + IfFailRet(CInstruction::CastTo(pTarget, &pNewTarget)); - CLogging::LogMessage(_T("End CSwitchInstruction::SetBranchTarget")); + m_branchTargets[index] = pNewTarget; + IfFailRet(CBranchTargetInfo::SetBranchTarget(this, pNewTarget, oldTarget)); return hr; } @@ -1307,21 +1124,14 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::SetBranchTarget(_In_ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::RemoveBranchTargetAt(_In_ DWORD index) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::RemoveBranchTargetAt")); - m_branchTargets.erase(m_branchTargets.begin() + index); - - CLogging::LogMessage(_T("End CSwitchInstruction::RemoveBranchTargetAt")); - return hr; } HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::RemoveBranchTarget(_In_ IInstruction* pTarget) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::RemoveBranchTarget")); - - for(std::vector>::iterator iter = m_branchTargets.begin(); iter != m_branchTargets.end(); ++iter) + for(std::vector>::iterator iter = m_branchTargets.begin(); iter != m_branchTargets.end(); ++iter) { if(*iter == pTarget) { @@ -1329,9 +1139,6 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::RemoveBranchTarget(_ } } - CLogging::LogMessage(_T("End CSwitchInstruction::RemoveBranchTarget")); - - return hr; } @@ -1340,19 +1147,14 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::RemoveBranchTarget(_ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::ReplaceBranchTarget(_In_ IInstruction* pOriginal, _In_ IInstruction *pNew) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::SetBranchTarget")); - for(ULONG i = 0; i < m_branchTargets.size(); i++) { if(m_branchTargets[i] == pOriginal) { - SetBranchTarget(i, pNew); + IfFailRet(SetBranchTarget(i, pNew)); //Need to continue as several branches of the switch might point to the same location } } - - CLogging::LogMessage(_T("End CSwitchInstruction::SetBranchTarget")); - return hr; } @@ -1360,24 +1162,17 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::ReplaceBranchTarget( HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::GetBranchCount(_Out_ DWORD* pBranchCount) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::GetBranchCount")); - IfNullRetPointer(pBranchCount); *pBranchCount = (DWORD)(m_branchTargets.size()); - - CLogging::LogMessage(_T("End CSwitchInstruction::GetBranchCount")); - return hr; } HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::GetBranchOffset(_In_ DWORD index, _In_ DWORD* pdwOffset) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CSwitchInstruction::GetBranchCount")); - IfNullRetPointer(pdwOffset); - CComPtr pInstruction = m_branchTargets[index]; + CComPtr pInstruction = m_branchTargets[index]; if (pInstruction != NULL) { IfFailRet(pInstruction->GetOffset(pdwOffset)); @@ -1387,16 +1182,11 @@ HRESULT MicrosoftInstrumentationEngine::CSwitchInstruction::GetBranchOffset(_In_ // The offsets encoded in the instruction stream are relative to this instruction *pdwOffset = m_origBranchTargetOffsets[index]; } - - CLogging::LogMessage(_T("End CSwitchInstruction::GetBranchCount")); - return hr; } MicrosoftInstrumentationEngine::CLoadConstInstruction::CLoadConstInstruction(_In_ int value) : COperandInstruction(Cee_Ldc_I4, TRUE) { - CLogging::LogMessage(_T("Starting CLoadConstInstruction::CLoadConstInstruction")); - // Check if the instruction can can be made a one byte // where the value of the constant is implied by the opcode if (value >= -1 && value <= 8) @@ -1413,16 +1203,12 @@ MicrosoftInstrumentationEngine::CLoadConstInstruction::CLoadConstInstruction(_In { m_value.i = value; } - - CLogging::LogMessage(_T("End CLoadConstInstruction::CLoadConstInstruction")); } MicrosoftInstrumentationEngine::CLoadLocalInstruction::CLoadLocalInstruction( _In_ USHORT index ) : COperandInstruction(Cee_Ldloc, TRUE) { - CLogging::LogMessage(_T("Starting CLoadLocalInstruction::CLoadLocalInstruction")); - // Check if the instruction can can be made a one byte if (index < 4) { @@ -1439,8 +1225,6 @@ MicrosoftInstrumentationEngine::CLoadLocalInstruction::CLoadLocalInstruction( m_opcode = Cee_Ldloc; m_value.i = index; } - - CLogging::LogMessage(_T("End CLoadLocalInstruction::CLoadLocalInstruction")); } @@ -1449,8 +1233,6 @@ MicrosoftInstrumentationEngine::CLoadLocalAddrInstruction::CLoadLocalAddrInstruc _In_ USHORT index ) : COperandInstruction(Cee_Ldloca, TRUE) { - CLogging::LogMessage(_T("Starting CLoadLocalAddrInstruction::CLoadLocalAddrInstruction")); - // Check if the instruction can can be made a one byte if (index <= 0xff) { @@ -1462,16 +1244,12 @@ MicrosoftInstrumentationEngine::CLoadLocalAddrInstruction::CLoadLocalAddrInstruc m_opcode = Cee_Ldloca; m_value.i = index; } - - CLogging::LogMessage(_T("End CLoadLocalAddrInstruction::CLoadLocalAddrInstruction")); } MicrosoftInstrumentationEngine::CStoreLocalInstruction::CStoreLocalInstruction( _In_ USHORT index ) : COperandInstruction(Cee_Stloc, TRUE) { - CLogging::LogMessage(_T("Starting CLoadLocalAddrInstruction::CStoreLocalInstruction")); - // Check if the instruction can can be made a one byte if (index < 4) { @@ -1488,8 +1266,6 @@ MicrosoftInstrumentationEngine::CStoreLocalInstruction::CStoreLocalInstruction( m_opcode = Cee_Stloc; m_value.i = index; } - - CLogging::LogMessage(_T("End CLoadLocalAddrInstruction::CStoreLocalInstruction")); } @@ -1497,8 +1273,6 @@ MicrosoftInstrumentationEngine::CLoadArgInstruction::CLoadArgInstruction( _In_ USHORT index ) : COperandInstruction(Cee_Ldarg, TRUE) { - CLogging::LogMessage(_T("Starting CLoadLocalAddrInstruction::CLoadArgInstruction")); - // Check if the instruction can can be made a one byte if (index < 4) { @@ -1515,16 +1289,12 @@ MicrosoftInstrumentationEngine::CLoadArgInstruction::CLoadArgInstruction( m_opcode = Cee_Ldarg; m_value.i = (BYTE) index; } - - CLogging::LogMessage(_T("End CLoadLocalAddrInstruction::CLoadArgInstruction")); } MicrosoftInstrumentationEngine::CLoadArgAddrInstruction::CLoadArgAddrInstruction( _In_ USHORT index ) : COperandInstruction(Cee_Ldarga, TRUE) { - CLogging::LogMessage(_T("Starting CLoadLocalAddrInstruction::CLoadArgAddrInstruction")); - // Check if the instruction can can be 2 bytes if (index <= 0xff) { @@ -1536,8 +1306,6 @@ MicrosoftInstrumentationEngine::CLoadArgAddrInstruction::CLoadArgAddrInstruction m_opcode = Cee_Ldarga; m_value.i = (BYTE) index; } - - CLogging::LogMessage(_T("End CLoadLocalAddrInstruction::CLoadArgAddrInstruction")); } // Return the instruction's impact on the execution stack. @@ -1796,6 +1564,13 @@ HRESULT MicrosoftInstrumentationEngine::CInstruction::Disconnect() { m_pOriginalNextInstruction.Release(); } + + CComPtr pBranchTargetInfo; + if (SUCCEEDED(CBranchTargetInfo::GetInstance(this, &pBranchTargetInfo))) + { + pBranchTargetInfo->Disconnect(); + } + return S_OK; } diff --git a/src/InstrumentationEngine/Instruction.h b/src/InstrumentationEngine/Instruction.h index a4dfabce..116d1d3c 100644 --- a/src/InstrumentationEngine/Instruction.h +++ b/src/InstrumentationEngine/Instruction.h @@ -35,6 +35,7 @@ namespace MicrosoftInstrumentationEngine // CONSIDER: Store the original length as well? The graph immediately changes short branches etc... ILOrdinalOpcode m_opcode; + DWORD m_offset; DWORD m_origOffset; InstructionGeneration m_instructionGeneration; @@ -68,11 +69,16 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast(this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return ImplQueryInterface( + static_cast(this), + static_cast(this), + riid, + ppvObject + ); + } + return S_OK; } public: @@ -85,8 +91,6 @@ namespace MicrosoftInstrumentationEngine HRESULT SetGraph(_In_ CInstructionGraph* pGraph); constexpr CInstructionGraph* GetGraph() { return m_pGraph; } - static HRESULT GetInstructionSize(_In_ IInstruction* pInstruction, _Out_ DWORD* pdwSize); - public: virtual HRESULT __stdcall GetOffset(_Out_ DWORD* pdwOffset) override; virtual HRESULT __stdcall GetOriginalOffset(_Out_ DWORD* pdwOffset) override; @@ -112,8 +116,6 @@ namespace MicrosoftInstrumentationEngine // set to true of this instruction has been removed or replaced from in the instruction stream. virtual HRESULT __stdcall GetIsRemoved(_Out_ BOOL* pbValue) override; - virtual HRESULT __stdcall GetInstructionSize(_Out_ DWORD* pdwSize); - virtual HRESULT __stdcall GetIsBranch(_Out_ BOOL* pbValue) override; virtual HRESULT __stdcall GetIsSwitch(_Out_ BOOL* pbValue) override; virtual HRESULT __stdcall GetIsCallInstruction(_Out_ BOOL* pbValue) override; @@ -135,6 +137,8 @@ namespace MicrosoftInstrumentationEngine public: // NOTE: these do not fixup the rest of hte graph. Call the versions on instruction graph if you want // previous, and next to be udpated correctly + + virtual DWORD GetInstructionSize(); HRESULT SetNextInstruction(_In_opt_ CInstruction* pInstruction, _In_ bool setOrig); HRESULT SetPreviousInstruction(_In_opt_ CInstruction* pInstruction, _In_ bool setOrig); @@ -146,7 +150,6 @@ namespace MicrosoftInstrumentationEngine static HRESULT InstructionFromBytes(_In_ LPCBYTE pCode, _In_ LPCBYTE pEndOfCode, _Out_ CInstruction** ppInstruction); static HRESULT OrdinalOpcodeFromBytes(_In_reads_to_ptr_(pEndOfCode) LPCBYTE pCode, _In_ LPCBYTE pEndOfCode, _Out_ ILOrdinalOpcode* pOpcode); - HRESULT EmitIL(_In_reads_bytes_(dwcbILBuffer) BYTE* pILBuffer, _In_ DWORD dwcbILBuffer); HRESULT LogInstruction(bool ignoreTest); @@ -158,26 +161,40 @@ namespace MicrosoftInstrumentationEngine constexpr bool GetIsSwitchInternal() const { return m_opcode == Cee_Switch; } bool GetIsBranchInternal() const { return IsFlagSet(s_ilOpcodeInfo[m_opcode].m_flags, ILOpcodeFlag_Branch); } + // Attempt to cast the given IInstruction to a more specific TCast type. pInstruction may + // be null, in which case, the result will also be null. + template + static HRESULT CastTo(_In_opt_ IInstruction* pInstruction, _Outptr_result_maybenull_ TCast** pResult) + { + IfNullRet(pResult); + if (pInstruction == nullptr) + { + *pResult = nullptr; + } + + return pInstruction->QueryInterface(pResult); + } + protected: - template - HRESULT QueryInterfaceInternal(_In_ REFIID riid, _Out_ void** ppvObject, Args... pThisArgs) + + template + HRESULT QueryBaseType(_In_ REFIID riid, _Out_ void** ppvObject) { // Workaround for the diamond inheritence problem. // Both CDataContainer and IInstruction inherit from IUnknown and // ImplQueryInterface() doesn't know which one to pick when casting CInstruction* to IUnknown*. - // A more comprehensive fix will be covered by https://github.com/microsoft/CLRInstrumentationEngine/issues/311 - if (__uuidof(CInstruction) == riid && ppvObject != nullptr) + // A more comprehensive fix will be covered by https://github.com/microsoft/CLRInstrumentationEngine/issues/311 + + IfNullRet(ppvObject); + + if (__uuidof(TBase) == riid) { - *ppvObject = static_cast(this); + *ppvObject = static_cast(this); AddRef(); return S_OK; } - return ImplQueryInterface( - std::forward(pThisArgs)..., - riid, - ppvObject - ); + return E_NOINTERFACE; } private: @@ -241,12 +258,18 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(COperandInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast(this), - static_cast(this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + if FAILED(CInstruction::QueryInterface(riid, ppvObject)) + { + return ImplQueryInterface( + static_cast(this), + riid, + ppvObject + ); + } + } + return S_OK; } // IOperandInstruction methods @@ -278,12 +301,12 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadConstInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -296,16 +319,16 @@ namespace MicrosoftInstrumentationEngine ); public: - DEFINE_DELEGATED_REFCOUNT_ADDREF(CLoadConstInstruction); - DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadConstInstruction); + DEFINE_DELEGATED_REFCOUNT_ADDREF(CLoadLocalInstruction); + DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadLocalInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -322,12 +345,12 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadLocalAddrInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -345,12 +368,12 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CStoreLocalInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -367,12 +390,12 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadArg); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -389,12 +412,12 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CLoadArgAddrInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast((COperandInstruction*)this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + return COperandInstruction::QueryInterface(riid, ppvObject); + } + + return S_OK; } }; @@ -405,8 +428,8 @@ namespace MicrosoftInstrumentationEngine // NOTE: This is the target offset for the original decoded instruction. // It should not be used if m_pBranchTarget has been set. DWORD m_decodedTargetOffset; - CComPtr m_pBranchTarget; - CComPtr m_pOrigBranchTarget; + CComPtr m_pBranchTarget; + CComPtr m_pOrigBranchTarget; public: CBranchInstruction( @@ -424,12 +447,20 @@ namespace MicrosoftInstrumentationEngine DEFINE_DELEGATED_REFCOUNT_RELEASE(CILBranch); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast(this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + if (FAILED(CInstruction::QueryInterface(riid, ppvObject))) + { + return ImplQueryInterface( + static_cast(this), + riid, + ppvObject + ); + } + + } + + return S_OK; } // IBranchInstruction methods @@ -464,19 +495,13 @@ namespace MicrosoftInstrumentationEngine // Vector of actual branch targets. Offsets etc... should be read from here // after the initialization phase is over. - std::vector> m_branchTargets; + std::vector> m_branchTargets; public: - CSwitchInstruction( - _In_ ILOrdinalOpcode opcode, - _In_ bool isNew - ); - CSwitchInstruction( _In_ ILOrdinalOpcode opcode, _In_ bool isNew, - _In_ DWORD cBranchTargets, - _In_reads_(cBranchTargets) IInstruction** ppBranchTargets + _In_ DWORD initialCount = 0 ); virtual HRESULT InitializeFromBytes( @@ -484,17 +509,27 @@ namespace MicrosoftInstrumentationEngine _In_ LPCBYTE pEndOfCode ) override; + virtual DWORD GetInstructionSize() override; + public: DEFINE_DELEGATED_REFCOUNT_ADDREF(CSwitchInstruction); DEFINE_DELEGATED_REFCOUNT_RELEASE(CSwitchInstruction); STDMETHOD(QueryInterface)(_In_ REFIID riid, _Out_ void **ppvObject) override { - return QueryInterfaceInternal( - riid, - ppvObject, - static_cast(this), - static_cast((CInstruction*)this), - static_cast(this)); + if (FAILED(QueryBaseType(riid, ppvObject))) + { + if (FAILED(CInstruction::QueryInterface(riid, ppvObject))) + { + return ImplQueryInterface( + static_cast(this), + riid, + ppvObject + ); + } + + } + + return S_OK; } public: diff --git a/src/InstrumentationEngine/InstructionFactory.cpp b/src/InstrumentationEngine/InstructionFactory.cpp index dadd4156..8e9bac81 100644 --- a/src/InstrumentationEngine/InstructionFactory.cpp +++ b/src/InstrumentationEngine/InstructionFactory.cpp @@ -219,12 +219,19 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionFactory::CreateSwitchInstruc IfNullRetPointer(ppInstruction); CComPtr pInstruction; - pInstruction.Attach(new CSwitchInstruction(opcode, TRUE, cBranchTargets, ppBranchTargets)); + pInstruction.Attach(new CSwitchInstruction(opcode, TRUE, cBranchTargets)); if (pInstruction == NULL) { return E_OUTOFMEMORY; } + for (DWORD i = 0; i < cBranchTargets; i++) + { + CComPtr pTarget; + IfFailRet(ppBranchTargets[i]->QueryInterface(&pTarget)); + IfFailRet(pInstruction->SetBranchTarget(i, ppBranchTargets[i])); + } + *ppInstruction = (IInstruction*)(pInstruction.p); (*ppInstruction)->AddRef(); diff --git a/src/InstrumentationEngine/InstructionGraph.cpp b/src/InstrumentationEngine/InstructionGraph.cpp index b588a7c8..59adbae5 100644 --- a/src/InstrumentationEngine/InstructionGraph.cpp +++ b/src/InstrumentationEngine/InstructionGraph.cpp @@ -4,6 +4,7 @@ #include "stdafx.h" #include "Instruction.h" #include "InstructionGraph.h" +#include "BranchTargetInfo.h" MicrosoftInstrumentationEngine::CInstructionGraph::CInstructionGraph() : m_pMethodInfo(NULL), m_bHasBaselineBeenSet(false), m_bAreInstructionsStale(true) { @@ -265,9 +266,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::DecodeInstructions(_I pPrevious = pNewInstr.p; // Jump to the next instruction - DWORD instructionSize = 0; - IfFailRet(pNewInstr->GetInstructionSize(&instructionSize)); - pCode += instructionSize; + pCode += pNewInstr->GetInstructionSize(); if (pCode >= pEndOfCode) { @@ -382,8 +381,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::EncodeIL( CInstruction* pInstruction = m_pFirstInstruction; while (pInstruction != NULL) { - DWORD dwInstructionSize = 0; - IfFailRet(pInstruction->GetInstructionSize(&dwInstructionSize)); + DWORD dwInstructionSize = pInstruction->GetInstructionSize(); BOOL bIsNewInstruction = FALSE; pInstruction->GetIsNew(&bIsNewInstruction); @@ -490,7 +488,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::EncodeIL( return S_OK; } -HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtEndOffset(_In_ DWORD offset, _Out_ IInstruction** ppInstruction) +HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtEndOffset(_In_ DWORD offset, _Out_ CInstruction** ppInstruction) { HRESULT hr = S_OK; CCriticalSectionHolder lock(&m_cs); @@ -501,8 +499,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtEndOf DWORD currOffset = 0; IfFailRet(pCurrent->GetOffset(&currOffset)); - DWORD currSize = 0; - IfFailRet(pCurrent->GetInstructionSize(&currSize)); + DWORD currSize = pCurrent->GetInstructionSize(); if ((currOffset + currSize) == offset) { @@ -533,7 +530,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::CalculateInstructionO if (pPrev != NULL) { IfFailRet(pPrev->GetOffset(&dwPrevOffset)); - IfFailRet(pPrev->GetInstructionSize(&dwPrevLength)); + dwPrevLength = pPrev->GetInstructionSize(); } IfFailRet(pCurrent->SetOffset(dwPrevOffset + dwPrevLength)); @@ -705,9 +702,18 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetUninstrumentedLast } HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOffset(_In_ DWORD offset, _Out_ IInstruction** ppInstruction) +{ + HRESULT hr; + IfNullRetPointer(ppInstruction); + CComPtr pResult; + IfFailRet(GetInstructionAtOffsetInternal(offset, &pResult)); + *ppInstruction = pResult.Detach(); + return S_OK; +} + +HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOffsetInternal(_In_ DWORD offset, _Out_ CInstruction** ppInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstructionGraph::GetInstructionAtOffset")); CCriticalSectionHolder lock(&m_cs); IfNullRetPointer(ppInstruction); @@ -729,15 +735,22 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOffse pCurrInstruction = pCurrInstruction->NextInstructionInternal(); } - CLogging::LogMessage(_T("End CInstructionGraph::GetInstructionAtOffset")); - return E_FAIL; } HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOriginalOffset(_In_ DWORD offset, _Out_ IInstruction** ppInstruction) +{ + IfNullRetPointer(ppInstruction); + HRESULT hr = S_OK; + CComPtr pResult; + IfFailRet(GetInstructionAtOriginalOffsetInternal(offset, &pResult)); + *ppInstruction = pResult.Detach(); + return S_OK; +} + +HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOriginalOffsetInternal(_In_ DWORD offset, _Out_ CInstruction** ppInstruction) { HRESULT hr = S_OK; - CLogging::LogMessage(_T("Starting CInstructionGraph::GetInstructionAtOriginalOffset")); CCriticalSectionHolder lock(&m_cs); IfNullRetPointer(ppInstruction); @@ -767,8 +780,6 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::GetInstructionAtOrigi } } - CLogging::LogMessage(_T("End CInstructionGraph::GetInstructionAtOriginalOffset")); - return E_FAIL; } @@ -924,40 +935,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::InsertBeforeAndRetarg // Search the graph for any branch whose branch target is the old instruction. Set the branch target // to the new instruction. - CInstruction* pCurr = m_pFirstInstruction; - while (pCurr != nullptr) - { - bool isBranch = pCurr->GetIsBranchInternal(); - - if (isBranch) - { - CComPtr pBranch = (CBranchInstruction*)pCurr; - - const IInstruction* pBranchTarget = pBranch->GetBranchTargetInternal(); - - // If the branch target is the original instruction AND the branch is not the new instruction, - // update it. Note the second condition is important because for things like leave instructions, - // often the next instruction is the branch target and resetting this would create an infinite loop. - if (pBranchTarget == pInstructionOrig && pCurr != pInstructionNew) - { - IfFailRet(pBranch->SetBranchTarget(pInstructionNew)); - } - } - else - { - bool isSwitch = pCurr->GetIsSwitchInternal(); - - if (isSwitch) - { - CComPtr pSwitch; - IfFailRet(pCurr->QueryInterface(__uuidof(ISwitchInstruction), (LPVOID*)&pSwitch)); - IfFailRet(pSwitch->ReplaceBranchTarget(pInstructionOrig, pInstructionNew)); - } - - } - - pCurr = pCurr->NextInstructionInternal(); - } + IfFailRet(CBranchTargetInfo::RetargetBranches(pInstrOrig, pInstrNew)); // Search the exception clauses for anything that points to the old instruction // Set that target to the new instruction @@ -1123,28 +1101,14 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::RemoveAll() CLogging::LogMessage(_T("Starting CInstructionGraph::RemoveAll")); CCriticalSectionHolder lock(&m_cs); - CInstruction* pInstr = (CInstruction*)m_pFirstInstruction; + CInstruction* pInstr = m_pFirstInstruction.p; while (pInstr != NULL) { pInstr->SetIsRemoved(); - - CInstruction* pPreviousInstruction = pInstr->PreviousInstructionInternal(); CInstruction* pNextInstruction = pInstr->NextInstructionInternal(); - - if (pPreviousInstruction != NULL) - { - IfFailRet(pPreviousInstruction->SetNextInstruction(NULL, false)); - } - - if (pNextInstruction != NULL) - { - IfFailRet(pNextInstruction->SetPreviousInstruction(NULL, false)); - } - IfFailRet(pInstr->SetNextInstruction(NULL, false)); IfFailRet(pInstr->SetPreviousInstruction(NULL, false)); - pInstr = pNextInstruction; } @@ -1178,7 +1142,7 @@ HRESULT MicrosoftInstrumentationEngine::CInstructionGraph::CalculateMaxStack(_Ou // is not taken into account. It makes a linear pass over the instructions looking at // the stack impact of each instruction and maintaining a theoretical maximum. // The actual maximum may be slightly smaller. - CInstruction* pInstr = (CInstruction*)m_pFirstInstruction; + CInstruction* pInstr = m_pFirstInstruction.p; while (pInstr != NULL) { CInstruction* pNextInstruction = pInstr->NextInstructionInternal(); diff --git a/src/InstrumentationEngine/InstructionGraph.h b/src/InstrumentationEngine/InstructionGraph.h index cc843649..b8e68c32 100644 --- a/src/InstrumentationEngine/InstructionGraph.h +++ b/src/InstrumentationEngine/InstructionGraph.h @@ -71,12 +71,15 @@ namespace MicrosoftInstrumentationEngine HRESULT CalculateInstructionOffsets(); - HRESULT GetInstructionAtEndOffset(_In_ DWORD offset, _Out_ IInstruction** ppInstruction); + HRESULT GetInstructionAtEndOffset(_In_ DWORD offset, _Out_ CInstruction** ppInstruction); HRESULT CalculateMaxStack(_Out_ DWORD* pMaxStack); constexpr CInstruction* FirstInstructionInternal() { return m_pFirstInstruction.p; } constexpr CInstruction* OriginalFirstInstructionInternal() { return m_pOrigFirstInstruction.p; } + HRESULT GetInstructionAtOffsetInternal(_In_ DWORD offset, _Out_ CInstruction** ppInstruction); + HRESULT GetInstructionAtOriginalOffsetInternal(_In_ DWORD offset, _Out_ CInstruction** ppInstruction); + HRESULT RefreshInstructions(); // IInstructionGraph methods diff --git a/src/InstrumentationEngine/InstrumentationEngine.vcxproj b/src/InstrumentationEngine/InstrumentationEngine.vcxproj index f61f3f66..31dfe37e 100644 --- a/src/InstrumentationEngine/InstrumentationEngine.vcxproj +++ b/src/InstrumentationEngine/InstrumentationEngine.vcxproj @@ -117,6 +117,7 @@ + diff --git a/tests/InstrEngineTests/InstrEngineTests/ProfilerHelpers.cs b/tests/InstrEngineTests/InstrEngineTests/ProfilerHelpers.cs index b58dea61..7888998b 100644 --- a/tests/InstrEngineTests/InstrEngineTests/ProfilerHelpers.cs +++ b/tests/InstrEngineTests/InstrEngineTests/ProfilerHelpers.cs @@ -11,6 +11,7 @@ using System.Text; using System.Text.RegularExpressions; using System.Xml; +using System.Runtime.InteropServices; namespace InstrEngineTests { @@ -46,6 +47,8 @@ internal class ProfilerHelpers // startup to allow attaching a debugger. private static bool ThrowMessageBoxAtStartup = false; + private static bool WaitForDebugger = false; + private static bool BinaryRecompiled = false; // Enable ref recording to track down memory leaks. For debug only. @@ -102,6 +105,11 @@ public static void LaunchAppUnderProfiler(string testApp, string testScript, str psi.EnvironmentVariables.Add("MicrosoftInstrumentationEngine_MessageboxAtAttach", @"1"); } + if (WaitForDebugger) + { + psi.EnvironmentVariables.Add("MicrosoftInstrumentationEngine_DebugWait", @"1"); + } + if (TestParameters.DisableMethodSignatureValidation) { psi.EnvironmentVariables.Add("MicrosoftInstrumentationEngine_DisableCodeSignatureValidation", @"1"); @@ -417,10 +425,8 @@ private static void SetBitness(bool isWow64) flag |= COMPLUS_ENABLE_64BIT; } - if(!NativeMethods.SetComPlusPackageInstallStatus(flag)) - { - throw new InvalidOperationException(); - } + // safely ignore the result. + _ = NativeMethods.SetComPlusPackageInstallStatus(flag); } } }