fix: Revert recent Profiler warning fixes to address reported instabi…

…lity (#2663)

src/Agent/NewRelic/Home/Home.csproj

@@ -13,7 +13,7 @@
  </Target>
 
  <ItemGroup>
- <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.27.0.15"/>
+ <PackageReference Include="NewRelic.Agent.Internal.Profiler" Version="10.27.0.20"/>
  </ItemGroup>
 
 </Project>

src/Agent/NewRelic/Profiler/Common/CorStandIn.h

@@ -3,8 +3,6 @@
 * SPDX-License-Identifier: Apache-2.0
 */
 #pragma warning(push)
-// Since this isn't our code, we don't want to mess with it. These warnings can be safely ignored.
-#pragma warning(disable: 4458) // Scope hides class member with same name.
-#pragma warning(disable: 26495) // Uninitialized member variable, even if it's always set before use.
+#pragma warning(disable : 4458)
 #include <corhlpr.h>
 #pragma warning(pop)

src/Agent/NewRelic/Profiler/Configuration/Configuration.h

@@ -47,24 +47,24 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  , _ignoreList(new IgnoreInstrumentationList())
  {
  try {
- auto globalNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
- globalNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
+ rapidxml::xml_document<xchar_t> globalNewRelicConfigurationDocument;
+ globalNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(globalNewRelicConfiguration.c_str()));
 
- auto globalNewRelicConfigurationNode = GetConfigurationNode(globalNewRelicConfigurationDocument);
+ auto globalNewRelicConfigurationNode  = GetConfigurationNode(globalNewRelicConfigurationDocument);
  if (globalNewRelicConfigurationNode == nullptr) 
  {
  LogError(L"Unable to locate configuration node in the global newrelic.config file.");
  throw ConfigurationException();
  }
 
  auto appliedNewRelicConfigurationNode = globalNewRelicConfigurationNode;
- auto localNewRelicConfigurationDocument = std::make_shared<rapidxml::xml_document<xchar_t>>();
 
  if (localNewRelicConfiguration.second)
  {
  try
  {
- localNewRelicConfigurationDocument->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
+ rapidxml::xml_document<xchar_t> localNewRelicConfigurationDocument;
+ localNewRelicConfigurationDocument.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(localNewRelicConfiguration.first.c_str()));
 
  auto localNewRelicConfigurationNode = GetConfigurationNode(localNewRelicConfigurationDocument);
  if (localNewRelicConfigurationNode == nullptr)
@@ -92,7 +92,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  SetLogLevel(appliedNewRelicConfigurationNode);
  SetInstrumentationData(appliedNewRelicConfigurationNode);
  SetApplicationPools(appliedNewRelicConfigurationNode);
- 
+
  } catch (const rapidxml::parse_error& exception) {
  // We log two separate error messages here because sometimes the logging macros hang when
  // logging the "where" contents
@@ -196,15 +196,15 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  return _logLevel;
  }
 
- bool GetConsoleLogging() const
+ bool GetConsoleLogging()
  {
  return _consoleLogging;
  }
- bool GetLoggingEnabled() const
+ bool GetLoggingEnabled()
  {
  return _loggingEnabled;
  }
- IgnoreInstrumentationListPtr GetIgnoreInstrumentationList() const
+ IgnoreInstrumentationListPtr GetIgnoreInstrumentationList()
  {
  return _ignoreList;
  }
@@ -224,9 +224,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  std::shared_ptr<NewRelic::Profiler::Logger::IFileDestinationSystemCalls> _systemCalls;
  IgnoreInstrumentationListPtr _ignoreList;
 
- rapidxml::xml_node<xchar_t>* GetConfigurationNode(const std::shared_ptr<rapidxml::xml_document<xchar_t>> document)
+ rapidxml::xml_node<xchar_t>* GetConfigurationNode(const rapidxml::xml_document<xchar_t>& document)
  {
- auto configurationNode = document->first_node(_X("configuration"), 0, false);
+ auto configurationNode = document.first_node(_X("configuration"), 0, false);
  if (configurationNode == nullptr) {
  return nullptr;
  }
@@ -294,7 +294,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  _logLevel = TryParseLogLevel(level);
  }
 
- Logger::Level TryParseLogLevel(const xstring_t& logText) const
+ Logger::Level TryParseLogLevel(const xstring_t& logText)
  {
  if (Strings::AreEqualCaseInsensitive(logText, _X("off"))) {
  return Logger::Level::LEVEL_ERROR;
@@ -423,8 +423,8 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  if (applicationConfiguration.empty())
  return;
 
- auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
- document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
+ rapidxml::xml_document<xchar_t> document;
+ document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(applicationConfiguration.c_str()));
  auto configurationNode = GetConfigurationNode(document);
 
  auto appSettingsNode = configurationNode->first_node(_X("appSettings"), 0, false);
@@ -468,7 +468,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  static bool IsProcessInProcessList(const ProcessesPtr& processes, const xstring_t& processName)
  {
  // check the processes loaded from configuration
- for (auto& validProcessName : *processes) {
+ for (auto validProcessName : *processes) {
  if (Strings::EndsWith(processName, validProcessName)) {
  return true;
  }
@@ -498,7 +498,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration {
  return isIis;
  }
 
- bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId) const
+ bool ShouldInstrumentApplicationPool(const xstring_t& appPoolId)
  {
  if (ApplicationPoolIsOnBlackList(appPoolId, _applicationPoolsBlackList)) {
  LogInfo(_X("This application pool (") + appPoolId + _X(") is explicitly configured to NOT be instrumented."));

src/Agent/NewRelic/Profiler/Configuration/IgnoreInstrumentation.h

@@ -50,7 +50,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  }
 
  private:
- bool Matches(xstring_t assembly, xstring_t className) const
+ bool Matches(xstring_t assembly, xstring_t className)
  {
  if (!Strings::AreEqualCaseInsensitive(AssemblyName, assembly))
  {

src/Agent/NewRelic/Profiler/Configuration/InstrumentationConfiguration.h

@@ -33,7 +33,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  , _foundServerlessInstrumentationPoint(false)
  {
  // pull instrumentation points from every xml string
- for (auto& instrumentationXml : *instrumentationXmls)
+ for (auto instrumentationXml : *instrumentationXmls)
  {
  try
  {
@@ -64,13 +64,13 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  , _systemCalls(nullptr)
  , _foundServerlessInstrumentationPoint(false)
  {
- for (auto& instrumentationPoint : *instrumentationPoints)
+ for (auto instrumentationPoint : *instrumentationPoints)
  {
  AddInstrumentationPointToCollectionsIfNotIgnored(instrumentationPoint);
  }
  }
 
- uint16_t GetInvalidFileCount() const
+ uint16_t GetInvalidFileCount()
  {
  return _invalidFileCount;
  }
@@ -100,7 +100,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  // We may have multiple matching instrumentation points that target different assembly versions. See if we can find one that meets
  // the version requirements
  AssemblyVersion foundVersion(function->GetAssemblyProps());
- for (auto& instPoint : instPoints)
+ for (auto instPoint : instPoints)
  {
  if ((instPoint->MinVersion != nullptr) && (foundVersion < *instPoint->MinVersion))
  {
@@ -236,9 +236,9 @@ namespace NewRelic { namespace Profiler { namespace Configuration
 
  void GetInstrumentationPoints(xstring_t instrumentationXml)
  {
- auto document = std::make_shared<rapidxml::xml_document<xchar_t>>();
- document->parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
- auto extensionNode = document->first_node(_X("extension"), 0, false);
+ rapidxml::xml_document<xchar_t> document;
+ document.parse<rapidxml::parse_trim_whitespace | rapidxml::parse_normalize_whitespace>(const_cast<xchar_t*>(instrumentationXml.c_str()));
+ auto extensionNode = document.first_node(_X("extension"), 0, false);
  if (extensionNode == nullptr)
  {
  LogWarn(L"extension node not found in instrumentation file. Please validate your instrumentation files against extensions/extension.xsd or contact New Relic support.");
@@ -405,7 +405,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  // if the ClassName includes multiple classes, we have to split this into multiple instrumentation points
  auto instrumentationPoints = SplitInstrumentationPointsOnClassNames(instrumentationPoint);
 
- for (auto& iPoint : instrumentationPoints) {
+ for (auto iPoint : instrumentationPoints) {
 
  // finally add the new instrumentation point(s) to our set of instrumentation points
  // Note that there may be "duplicated" instrumentation points that target different assembly versions

src/Agent/NewRelic/Profiler/Configuration/InstrumentationPoint.h

@@ -52,7 +52,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  ParametersMatch(other);
  }
 
- xstring_t ToString() const
+ xstring_t ToString()
  {
  if (Parameters == nullptr)
  {
@@ -63,7 +63,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
  }
  }
 
- xstring_t GetMatchKey() const
+ xstring_t GetMatchKey()
  {
  return Parameters == nullptr
  ? GetMatchKey(AssemblyName, ClassName, MethodName)
@@ -82,7 +82,7 @@ namespace NewRelic { namespace Profiler { namespace Configuration
 
 
  private:
- bool ParametersMatch(const InstrumentationPoint& other) const
+ bool ParametersMatch(const InstrumentationPoint& other)
  {
  // nullptr means no parameters attribute was supplied in configuration, suggesting that we should instrument all overloads
  if (this->Parameters == nullptr)

src/Agent/NewRelic/Profiler/MethodRewriter/ExceptionHandlerManipulator.h

@@ -262,12 +262,12 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
  // shift the original clauses up to the correct
  for (uint32_t i = 0; i < _originalExceptionClauseCount; ++i)
  {
- auto& clause = _exceptionClauses[i];
+ auto clause = _exceptionClauses[i];
  clause->ShiftOffsets(userCodeOffset);
  }
 
  // append the clauses
- for (auto& clause : _exceptionClauses)
+ for (auto clause : _exceptionClauses)
  {
  auto clauseBytes = clause->GetBytes();
  bytes->insert(bytes->end(), clauseBytes->begin(), clauseBytes->end());
@@ -276,7 +276,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
  return bytes;
  }
 
- uint32_t GetOriginalExceptionClauseCount() const
+ uint32_t GetOriginalExceptionClauseCount()
  {
  return _originalExceptionClauseCount;
  }

src/Agent/NewRelic/Profiler/MethodRewriter/FunctionManipulator.h

@@ -350,7 +350,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
  _instructions->Append(CEE_NEWARR, _X("[mscorlib]System.Object"));
  uint32_t index = 0;
 
- for (auto& func : elementLoadLambdas)
+ for (auto func : elementLoadLambdas)
  {
  auto nextIndex = index++;
  // get an extra copy of the array (it will be popped off the stack each time we add an element to it)

src/Agent/NewRelic/Profiler/MethodRewriter/InstructionSet.h

@@ -403,7 +403,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
 
  void AppendTryEnd()
  {
- auto& exception = _exceptionStack.top();
+ auto exception = _exceptionStack.top();
  if (exception->_tryLength != 0)
  {
  LogError(L"Attempted to set try close on the same exception twice.");
@@ -414,7 +414,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
 
  void AppendCatchStart(uint32_t typeToken)
  {
- auto& exception = _exceptionStack.top();
+ auto exception = _exceptionStack.top();
  if (exception->_handlerOffset != 0)
  {
  LogError(L"Attempted to set catch start on the same exception twice.");
@@ -439,7 +439,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
 
  void AppendCatchEnd()
  {
- auto& exception = _exceptionStack.top();
+ auto exception = _exceptionStack.top();
  if (exception->_handlerLength != 0)
  {
  LogError(L"Attempted to set catch end on the same exception twice.");

src/Agent/NewRelic/Profiler/MethodRewriter/InstrumentFunctionManipulator.h

@@ -16,10 +16,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
  public:
  InstrumentFunctionManipulator(IFunctionPtr function, InstrumentationSettingsPtr instrumentationSettings) : 
  FunctionManipulator(function), 
- _instrumentationSettings(instrumentationSettings),
- _userExceptionLocalIndex(0),
- _resultLocalIndex(0),
- _tracerLocalIndex(0)
+ _instrumentationSettings(instrumentationSettings)
  {
  if (_function->Preprocess()) {
  Initialize();
@@ -231,4 +228,4 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter
  _resultLocalIndex = AppendReturnTypeLocal(_newLocalVariablesSignature, _methodSignature);
  }
  };
-}}}
+}}}

src/Agent/NewRelic/Profiler/MethodRewriter/MethodRewriter.h

@@ -54,7 +54,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter {
 
  auto instrumentationPoints = _instrumentationConfiguration->GetInstrumentationPoints();
 
- for (auto& instrumentationPoint : *instrumentationPoints) {
+ for (auto instrumentationPoint : *instrumentationPoints) {
 
  _instrumentedAssemblies->emplace(instrumentationPoint->AssemblyName);
  _instrumentedFunctionNames->emplace(instrumentationPoint->MethodName);
@@ -74,7 +74,7 @@ namespace NewRelic { namespace Profiler { namespace MethodRewriter {
  std::set<Configuration::InstrumentationPointPtr> GetAssemblyInstrumentation(xstring_t assemblyName)
  {
  std::set<Configuration::InstrumentationPointPtr> set;
- for (auto& instrumentationPoint : *_instrumentationConfiguration->GetInstrumentationPoints().get()) {
+ for (auto instrumentationPoint : *_instrumentationConfiguration->GetInstrumentationPoints().get()) {
  if (assemblyName == instrumentationPoint->AssemblyName) {
  set.emplace(instrumentationPoint);
  }

src/Agent/NewRelic/Profiler/ModuleInjector/ModuleInjector.h

@@ -10,7 +10,6 @@
 #include "../Logging/Logger.h"
 #include "../Sicily/Sicily.h"
 #include "IModule.h"
-#include "../Profiler/Exceptions.h"
 
 namespace NewRelic { namespace Profiler { namespace ModuleInjector
 {