Add proper error checking around GetModuleMetadata

DataDog · Sep 5, 2024 · 82a0277 · 82a0277
1 parent 3800b4e
commit 82a0277
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 26 deletions.
diff --git a/...ive/instrumented_assembly_generator/instrumented_assembly_generator_cor_profiler_info.cpp b/...ive/instrumented_assembly_generator/instrumented_assembly_generator_cor_profiler_info.cpp
@@ -70,7 +70,13 @@ HRESULT CorProfilerInfo::GetModuleMetaData(ModuleID moduleId, DWORD dwOpenFlags,
 {
     HRESULT hr;
     ComPtr<IUnknown> temp;
-    IfFailRet(m_corProfilerInfo->GetModuleMetaData(moduleId, dwOpenFlags, riid, temp.GetAddressOf()));
+    hr = m_corProfilerInfo->GetModuleMetaData(moduleId, dwOpenFlags, riid, temp.GetAddressOf());
+
+    if (hr != S_OK)
+    {
+        return hr;
+    }
+
     try
     {
         const auto metadataInterfaces = new MetadataInterfaces(temp);

diff --git a/...rProfiler.Native/instrumented_assembly_generator/instrumented_assembly_generator_helper.h b/...rProfiler.Native/instrumented_assembly_generator/instrumented_assembly_generator_helper.h
@@ -288,8 +288,13 @@ inline HRESULT WriteILChanges(ModuleID moduleId, mdMethodDef methodToken, LPCBYT
     try
     {
         ComPtr<IUnknown> metadataInterfaces;
-        IfFailRet(corProfilerInfo->GetModuleMetaData(moduleId, CorOpenFlags::ofRead, IID_IMetaDataImport,
-                                                     metadataInterfaces.GetAddressOf()));
+        hr = corProfilerInfo->GetModuleMetaData(moduleId, CorOpenFlags::ofRead, IID_IMetaDataImport,
+                                                     metadataInterfaces.GetAddressOf());
+
+        if (hr != S_OK)
+        {
+            return hr;
+        }
 
         auto metadataImport = metadataInterfaces.As<IMetaDataImport>(IID_IMetaDataImport);
 

diff --git a/tracer/src/Datadog.Tracer.Native/clr_helpers.cpp b/tracer/src/Datadog.Tracer.Native/clr_helpers.cpp
@@ -1107,7 +1107,7 @@ HRESULT ResolveTypeInternal(ICorProfilerInfo4* info,
         ComPtr<IUnknown> metadata_interfaces;
         auto hr = info->GetModuleMetaData(moduleId, ofRead, IID_IMetaDataImport2,
                                                             metadata_interfaces.GetAddressOf());
-        if (FAILED(hr))
+        if (hr != S_OK)
         {
             Logger::Warn("[ResolveTypeInternal] GetModuleMetaData has failed with: ", shared::WSTRING(refTypeName.data()));
             continue;

diff --git a/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp b/tracer/src/Datadog.Tracer.Native/cor_profiler.cpp
@@ -403,7 +403,7 @@ HRESULT STDMETHODCALLTYPE CorProfiler::AssemblyLoadFinished(AssemblyID assembly_
         auto hr = this->info_->GetModuleMetaData(assembly_info.manifest_module_id, ofRead,
                                                  IID_IMetaDataImport2, metadata_interfaces.GetAddressOf());
 
-        if (FAILED(hr))
+        if (hr != S_OK)
         {
             Logger::Warn("AssemblyLoadFinished failed to get metadata interface for module id ",
                          assembly_info.manifest_module_id, " from assembly ", assembly_info.name, " HRESULT=0x",
@@ -891,7 +891,7 @@ HRESULT CorProfiler::TryRejitModule(ModuleID module_id, std::vector<ModuleID>& m
         auto hr = this->info_->GetModuleMetaData(module_id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                  metadata_interfaces.GetAddressOf());
 
-        if (FAILED(hr))
+        if (hr != S_OK)
         {
             Logger::Warn("ModuleLoadFinished failed to get metadata interface for ", module_id, " ",
                          module_info.assembly.name);
@@ -989,7 +989,7 @@ HRESULT CorProfiler::TryRejitModule(ModuleID module_id, std::vector<ModuleID>& m
             auto hr = this->info_->GetModuleMetaData(module_id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                      metadata_interfaces.GetAddressOf());
 
-            if (FAILED(hr))
+            if (hr != S_OK)
             {
                 Logger::Warn("ModuleLoadFinished failed to get metadata interface for ", module_id, " ",
                              module_info.assembly.name);
@@ -1044,7 +1044,7 @@ HRESULT CorProfiler::TryRejitModule(ModuleID module_id, std::vector<ModuleID>& m
             auto hr = this->info_->GetModuleMetaData(module_id, ofRead, IID_IMetaDataImport2,
                                                      metadata_interfaces.GetAddressOf());
 
-            if (FAILED(hr))
+            if (hr != S_OK)
             {
                 Logger::Warn("ModuleLoadFinished failed to get metadata interface for ", module_id, " ",
                              module_info.assembly.name);
@@ -3163,7 +3163,7 @@ HRESULT CorProfiler::GenerateVoidILStartupMethod(const ModuleID module_id, mdMet
     ComPtr<IUnknown> metadata_interfaces;
     auto hr = this->info_->GetModuleMetaData(module_id, ofRead | ofWrite, IID_IMetaDataImport2,
                                              metadata_interfaces.GetAddressOf());
-    if (FAILED(hr))
+    if (hr != S_OK)
     {
         Logger::Warn("GenerateVoidILStartupMethod: failed to get metadata interface for ", module_id);
         return hr;
@@ -3925,7 +3925,7 @@ HRESULT CorProfiler::AddIISPreStartInitFlags(const ModuleID module_id, const mdT
     ComPtr<IUnknown> metadata_interfaces;
     auto hr = this->info_->GetModuleMetaData(module_id, ofRead | ofWrite, IID_IMetaDataImport2,
                                              metadata_interfaces.GetAddressOf());
-    if (FAILED(hr))
+    if (hr != S_OK)
     {
         Logger::Warn("GenerateVoidILStartupMethod: failed to get metadata interface for ", module_id);
         return hr;

diff --git a/tracer/src/Datadog.Tracer.Native/debugger_probes_instrumentation_requester.cpp b/tracer/src/Datadog.Tracer.Native/debugger_probes_instrumentation_requester.cpp
@@ -751,7 +751,7 @@ void DebuggerProbesInstrumentationRequester::ModuleLoadFinished_AddMetadataToMod
     Logger::Debug("  Loading Assembly Metadata...");
     auto hr = corProfilerInfo->GetModuleMetaData(moduleInfo.id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                  metadataInterfaces.GetAddressOf());
-    if (FAILED(hr))
+    if (hr != S_OK)
     {
         Logger::Warn(
             "DebuggerProbesInstrumentationRequester::sAddMetadataToModule failed to get metadata interface for ",

diff --git a/tracer/src/Datadog.Tracer.Native/debugger_rejit_preprocessor.cpp b/tracer/src/Datadog.Tracer.Native/debugger_rejit_preprocessor.cpp
@@ -47,7 +47,7 @@ ULONG DebuggerRejitPreprocessor::PreprocessLineProbes(
         Logger::Debug("  Loading Assembly Metadata...");
         auto hr = corProfilerInfo->GetModuleMetaData(moduleInfo.id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                      metadataInterfaces.GetAddressOf());
-        if (FAILED(hr))
+        if (hr != S_OK)
         {
             Logger::Warn("CallTarget_RequestRejitForModule failed to get metadata interface for ", moduleInfo.id, " ",
                          moduleInfo.assembly.name);

diff --git a/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp b/tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
@@ -406,45 +406,45 @@ HRESULT Dataflow::GetModuleInterfaces(ModuleID moduleId, IMetaDataImport2** ppMe
                                       IMetaDataAssemblyEmit** ppAssemblyEmit)
 {
     HRESULT hr = S_OK;
-    if (SUCCEEDED(hr))
+    if (hr == S_OK)
     {
         IUnknown* piUnk = nullptr;
         hr = _profiler->GetModuleMetaData(moduleId, ofRead | ofWrite, IID_IMetaDataImport2, &piUnk);
-        if (SUCCEEDED(hr))
+        if (hr == S_OK)
         {
             hr = piUnk->QueryInterface(IID_IMetaDataImport2, (void**) ppMetadataImport);
+            REL(piUnk);
         }
-        REL(piUnk);
     }
-    if (SUCCEEDED(hr))
+    if (hr == S_OK)
     {
         IUnknown* piUnk = nullptr;
         hr = _profiler->GetModuleMetaData(moduleId, ofRead | ofWrite, IID_IMetaDataEmit2, &piUnk);
-        if (SUCCEEDED(hr))
+        if (hr == S_OK)
         {
             hr = piUnk->QueryInterface(IID_IMetaDataEmit2, (void**) ppMetadataEmit);
+            REL(piUnk);
         }
-        REL(piUnk);
     }
-    if (SUCCEEDED(hr))
+    if (hr == S_OK)
     {
         IUnknown* piUnk = nullptr;
         hr = _profiler->GetModuleMetaData(moduleId, ofRead | ofWrite, IID_IMetaDataAssemblyImport, &piUnk);
-        if (SUCCEEDED(hr))
+        if (hr == S_OK)
         {
             hr = piUnk->QueryInterface(IID_IMetaDataAssemblyImport, (void**) ppAssemblyImport);
+            REL(piUnk);
         }
-        REL(piUnk);
     }
-    if (SUCCEEDED(hr))
+    if (hr == S_OK)
     {
         IUnknown* piUnk = nullptr;
         hr = _profiler->GetModuleMetaData(moduleId, ofRead | ofWrite, IID_IMetaDataAssemblyEmit, &piUnk);
-        if (SUCCEEDED(hr))
+        if (hr == S_OK)
         {
             hr = piUnk->QueryInterface(IID_IMetaDataAssemblyEmit, (void**) ppAssemblyEmit);
+            REL(piUnk);
         }
-        REL(piUnk);
     }
     return hr;
 }

diff --git a/tracer/src/Datadog.Tracer.Native/rejit_preprocessor.cpp b/tracer/src/Datadog.Tracer.Native/rejit_preprocessor.cpp
@@ -576,7 +576,7 @@ ULONG RejitPreprocessor<RejitRequestDefinition>::PreprocessRejitRequests(
                     Logger::Debug("  Loading Assembly Metadata...");
                     auto hr = corProfilerInfo->GetModuleMetaData(moduleInfo.id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                                  metadataInterfaces.GetAddressOf());
-                    if (FAILED(hr))
+                    if (hr != S_OK)
                     {
                         Logger::Warn("CallTarget_RequestRejitForModule failed to get metadata interface for ",
                                      moduleInfo.id, " ", moduleInfo.assembly.name);
@@ -799,7 +799,7 @@ ULONG RejitPreprocessor<RejitRequestDefinition>::PreprocessRejitRequests(
                     Logger::Debug("  Loading Assembly Metadata...");
                     auto hr = corProfilerInfo->GetModuleMetaData(moduleInfo.id, ofRead | ofWrite, IID_IMetaDataImport2,
                                                                  metadataInterfaces.GetAddressOf());
-                    if (FAILED(hr))
+                    if (hr != S_OK)
                     {
                         Logger::Warn("CallTarget_RequestRejitForModule failed to get metadata interface for ",
                                      moduleInfo.id, " ", moduleInfo.assembly.name);