From 455f493e6b4aeaa3015f82121efe6de66261f4cf Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Wed, 22 Sep 2021 22:09:00 -0700 Subject: [PATCH 1/9] Add a no cache backed CLRConfig lookup mechanism Remove remaining locations that had a hardcoded COMPlus_ prefix. The new no cache CLRConfig is usable from within the PAL. --- src/coreclr/inc/clrconfig.h | 5 +- src/coreclr/inc/clrconfignocache.h | 82 +++++++++++++++++++ src/coreclr/inc/clrconfigvalues.h | 5 ++ src/coreclr/inc/cor.h | 6 -- src/coreclr/pal/src/exception/signal.cpp | 12 ++- src/coreclr/pal/src/init/pal.cpp | 27 +++--- .../pal/src/misc/tracepointprovider.cpp | 10 ++- src/coreclr/pal/src/thread/process.cpp | 80 ++++++++++-------- src/coreclr/utilcode/clrconfig.cpp | 32 ++++++-- src/coreclr/vm/autotrace.cpp | 9 +- src/coreclr/vm/perfmap.cpp | 19 +++-- 11 files changed, 202 insertions(+), 85 deletions(-) create mode 100644 src/coreclr/inc/clrconfignocache.h diff --git a/src/coreclr/inc/clrconfig.h b/src/coreclr/inc/clrconfig.h index f067824d241b5c..4348afe68696c9 100644 --- a/src/coreclr/inc/clrconfig.h +++ b/src/coreclr/inc/clrconfig.h @@ -126,10 +126,7 @@ class CLRConfig static HRESULT GetConfigValue(const ConfigStringInfo & info, __deref_out_z LPWSTR * outVal); // - // Check whether an option is specified (e.g. explicitly listed) in any of the CLRConfig - // locations: environment or registry (with or without COMPlus_) or any config file. - // The result is therefore a conservative approximation (some settings do not actually - // take effect everywhere and no setting is valid both with and without COMPlus_) + // Check whether an option is specified (e.g. explicitly listed) in the CLRConfig. // static BOOL IsConfigOptionSpecified(LPCWSTR name); diff --git a/src/coreclr/inc/clrconfignocache.h b/src/coreclr/inc/clrconfignocache.h new file mode 100644 index 00000000000000..e6364844674744 --- /dev/null +++ b/src/coreclr/inc/clrconfignocache.h @@ -0,0 +1,82 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// +// -------------------------------------------------------------------------------------------------- +// clrconfignocache.h +// +// Logic for resolving configuration names. +// + +#ifndef StrLen +#define StrLen(STR) ((sizeof(STR) / sizeof(STR[0])) - 1) +#endif // !StrLen + +// Config prefixes +#define COMPLUS_PREFIX_A "COMPlus_" +#define COMPLUS_PREFIX W("COMPlus_") +#define LEN_OF_COMPLUS_PREFIX StrLen(COMPLUS_PREFIX_A) + +#define DOTNET_PREFIX_A "DOTNET_" +#define DOTNET_PREFIX W("DOTNET_") +#define LEN_OF_DOTNET_PREFIX StrLen(DOTNET_PREFIX_A) + +class CLRConfigNoCache +{ + const char* _value; + + CLRConfigNoCache() = default; + CLRConfigNoCache(LPCSTR cfg) : _value { cfg } + { } + +public: + bool IsSet() const { return _value != NULL; } + + LPCSTR AsString() const + { + _ASSERTE(IsSet()); + return _value; + } + + bool TryAsInteger(int radix, DWORD& result) const + { + _ASSERTE(IsSet()); + + errno = 0; + LPSTR endPtr; + result = strtoul(_value, &endPtr, radix); + bool fSuccess = (errno != ERANGE) && (endPtr != _value); + return fSuccess; + } + + static CLRConfigNoCache Get(LPCSTR cfg) + { + if (cfg == nullptr) + return {}; + + char nameBuffer[64]; + const char* fallbackPrefix = COMPLUS_PREFIX_A; + const size_t namelen = strlen(cfg); + + bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); + bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); + if (!dotnetValid || !complusValid) + { + _ASSERTE(!"Environment variable name too long."); + return {}; + } + + // Priority order is DOTNET_ and then COMPlus_. + strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); + strcat_s(nameBuffer, _countof(nameBuffer), cfg); + + LPCSTR val = getenv(nameBuffer); + if (val == NULL && fallbackPrefix != NULL) + { + strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); + strcat_s(nameBuffer, _countof(nameBuffer), cfg); + val = getenv(nameBuffer); + } + + return { val }; + } +}; diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index 60457fd5fa455a..ef078c9c334063 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -677,6 +677,11 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeCircularMB, W("EventPipeCircularMB"), RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeProcNumbers, W("EventPipeProcNumbers"), 0, "Enable/disable capturing processor numbers in EventPipe event headers") RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeOutputStreaming, W("EventPipeOutputStreaming"), 0, "Enable/disable streaming for trace file set in COMPlus_EventPipeOutputPath. Non-zero values enable streaming.") +#ifdef FEATURE_AUTO_TRACE +RETAIL_CONFIG_DWORD_INFO(INTERNAL_AutoTrace_N_Tracers, W("AutoTrace_N_Tracers"), 0, "") +RETAIL_CONFIG_STRING_INFO(INTERNAL_AutoTrace_Command, W("AutoTrace_Command"), "") +#endif // FEATURE_AUTO_TRACE + // // Generational Aware Analysis // diff --git a/src/coreclr/inc/cor.h b/src/coreclr/inc/cor.h index a77a9f10777424..ee82e433695ebe 100644 --- a/src/coreclr/inc/cor.h +++ b/src/coreclr/inc/cor.h @@ -1634,12 +1634,6 @@ DECLARE_INTERFACE_(IMetaDataInfo, IUnknown) // Native Link method custom value definitions. This is for N-direct support. // -#define COR_NATIVE_LINK_CUSTOM_VALUE L"COMPLUS_NativeLink" -#define COR_NATIVE_LINK_CUSTOM_VALUE_ANSI "COMPLUS_NativeLink" - -// count of chars for COR_NATIVE_LINK_CUSTOM_VALUE(_ANSI) -#define COR_NATIVE_LINK_CUSTOM_VALUE_CC 18 - #include typedef struct { diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index 59b513b87204e9..66bd19bb403d9a 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -31,6 +31,8 @@ SET_DEFAULT_DEBUG_CHANNEL(EXCEPT); // some headers have code with asserts, so do #include "pal/palinternal.h" +#include + #include #include @@ -145,9 +147,15 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags) TRACE("Initializing signal handlers %04x\n", flags); #if !HAVE_MACH_EXCEPTIONS - char* enableAlternateStackCheck = getenv("COMPlus_EnableAlternateStackCheck"); + g_enable_alternate_stack_check = false; - g_enable_alternate_stack_check = enableAlternateStackCheck && (strtoul(enableAlternateStackCheck, NULL, 10) != 0); + CLRConfigNoCache stackCheck = CLRConfigNoCache::Get("EnableAlternateStackCheck"); + if (stackCheck.IsSet()) + { + DWORD value; + if (stackCheck.TryGetInteger(10, value)) + g_enable_alternate_stack_check = (value != 0); + } #endif if (flags & PAL_INITIALIZE_REGISTER_SIGNALS) diff --git a/src/coreclr/pal/src/init/pal.cpp b/src/coreclr/pal/src/init/pal.cpp index fee6957306635e..0bce2e4acb44f7 100644 --- a/src/coreclr/pal/src/init/pal.cpp +++ b/src/coreclr/pal/src/init/pal.cpp @@ -89,6 +89,7 @@ int CacheLineSize; #endif #include +#include using namespace CorUnix; @@ -268,17 +269,13 @@ EnsureStackSize(SIZE_T stackSize) void InitializeDefaultStackSize() { - char* defaultStackSizeStr = getenv("COMPlus_DefaultStackSize"); - if (defaultStackSizeStr != NULL) + CLRConfigNoCache defStackSize = CLRConfigNoCache::Get("DefaultStackSize"); + if (defStackSize.IsSet()) { - errno = 0; - // Like all numeric values specific by the COMPlus_xxx variables, it is a - // hexadecimal string without any prefix. - long int size = strtol(defaultStackSizeStr, NULL, 16); - - if (errno == 0) + DWORD size; + if (defStackSize.TryAsInteger(16, size)) { - g_defaultStackSize = std::max(size, (long int)PTHREAD_STACK_MIN); + g_defaultStackSize = std::max(size, (DWORD)PTHREAD_STACK_MIN); } } @@ -406,15 +403,11 @@ Initialize( #endif // ENSURE_PRIMARY_STACK_SIZE #ifdef FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION - char* useDefaultBaseAddr = getenv("COMPlus_UseDefaultBaseAddr"); - if (useDefaultBaseAddr != NULL) + CLRConfigNoCache useDefaultBaseAddr = CLRConfigNoCache::Get("UseDefaultBaseAddr"); + if (useDefaultBaseAddr.IsSet()) { - errno = 0; - // Like all numeric values specific by the COMPlus_xxx variables, it is a - // hexadecimal string without any prefix. - long int flag = strtol(useDefaultBaseAddr, NULL, 16); - - if (errno == 0) + DWORD flag; + if (useDefaultBaseAddr.TryAsInteger(16, flag)) { g_useDefaultBaseAddr = (BOOL) flag; } diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index 125654d4488dd3..590a1a156a5d54 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -23,6 +23,8 @@ Revision History: #include "pal/malloc.hpp" #include "pal/stackstring.hpp" +#include + #include #include #include @@ -62,10 +64,12 @@ PAL_InitializeTracing(void) // Check if loading the LTTng providers should be disabled. // Note: this env var is formally declared in clrconfigvalues.h, but // this code is executed too early to use the mechanics that come with that definition. - char *disableValue = getenv("COMPlus_LTTng"); - if (disableValue != NULL) + CLRConfigNoCache disableLTTng = CLRConfigNoCache::Get("LTTng"); + if (disableLTTng.IsSet()) { - fShouldLoad = strtol(disableValue, NULL, 10); + DWORD value; + if (disableLTTng.TryGetInteger(10, value)) + fShouldLoad = (int)value; } // Get the path to the currently executing shared object (libcoreclr.so). diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 0ae3f2eecafd05..1d1ab193a22c6b 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -37,6 +37,8 @@ SET_DEFAULT_DEBUG_CHANNEL(PROCESS); // some headers have code with asserts, so d #include "pal/stackstring.hpp" #include "pal/signal.hpp" +#include + #include #if HAVE_POLL #include @@ -3108,6 +3110,8 @@ PROCFormatInt(ULONG32 value) return buffer; } +static const INT UndefinedDumpType = 0; + /*++ Function PROCBuildCreateDumpCommandLine @@ -3124,8 +3128,8 @@ PROCBuildCreateDumpCommandLine( std::vector& argv, char** pprogram, char** ppidarg, - char* dumpName, - char* dumpType, + const char* dumpName, + INT dumpType, BOOL diag, BOOL crashReport) { @@ -3170,24 +3174,19 @@ PROCBuildCreateDumpCommandLine( argv.push_back(dumpName); } - if (dumpType != nullptr) + switch (dumpType) { - if (strcmp(dumpType, "1") == 0) - { - argv.push_back("--normal"); - } - else if (strcmp(dumpType, "2") == 0) - { - argv.push_back("--withheap"); - } - else if (strcmp(dumpType, "3") == 0) - { - argv.push_back("--triage"); - } - else if (strcmp(dumpType, "4") == 0) - { - argv.push_back("--full"); - } + case 1: argv.push_back("--normal"); + break; + case 2: argv.push_back("--withheap"); + break; + case 3: argv.push_back("--triage"); + break; + case 4: argv.push_back("--full"); + break; + case UndefinedDumpType: + default: + break; } if (diag) @@ -3277,19 +3276,37 @@ Return BOOL PROCAbortInitialize() { - char* enabled = getenv("COMPlus_DbgEnableMiniDump"); - if (enabled != nullptr && _stricmp(enabled, "1") == 0) + CLRConfigNoCache enabledCfg= CLRConfigNoCache::Get("DbgEnableMiniDump"); + + DWORD enabled = 0; + if (enabledCfg.IsSet() + && enabledCfg.TryAsInteger(10, enabled) + && enabled) { - char* dumpName = getenv("COMPlus_DbgMiniDumpName"); - char* dumpType = getenv("COMPlus_DbgMiniDumpType"); - char* diagStr = getenv("COMPlus_CreateDumpDiagnostics"); - BOOL diag = diagStr != nullptr && strcmp(diagStr, "1") == 0; - char* crashReportStr = getenv("COMPlus_EnableCrashReport"); - BOOL crashReport = crashReportStr != nullptr && strcmp(crashReportStr, "1") == 0; + CLRConfigNoCache dmpNameCfg = CLRConfigNoCache::Get("DbgMiniDumpName"); + + CLRConfigNoCache dmpTypeCfg = CLRConfigNoCache::Get("DbgMiniDumpType"); + DWORD dumpType = UndefinedDumpType; + if (dmpTypeCfg.IsSet()) + { + (void)dmpTypeCfg.TryAsInteger(10, dumpType); + if (dumpType < 1 || dumpType > 4) + { + dumpType = UndefinedDumpType; + } + } + + CLRConfigNoCache createDumpCfg = CLRConfigNoCache::Get("CreateDumpDiagnostics"); + DWORD val = 0; + BOOL diag = createDumpCfg.IsSet() && createDumpCfg.TryAsInteger(10, val) && val == 1; + + CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport"); + val = 0; + BOOL crashReport = enabldReportCfg.IsSet() && enabldReportCfg.TryAsInteger(10, val) && val == 1; char* program = nullptr; char* pidarg = nullptr; - if (!PROCBuildCreateDumpCommandLine(g_argvCreateDump, &program, &pidarg, dumpName, dumpType, diag, crashReport)) + if (!PROCBuildCreateDumpCommandLine(g_argvCreateDump, &program, &pidarg, dmpNameCfg.AsString(), dumpType, diag, crashReport)) { return FALSE; } @@ -3325,23 +3342,18 @@ PAL_GenerateCoreDump( BOOL diag) { std::vector argvCreateDump; - char dumpTypeStr[16]; if (dumpType < 1 || dumpType > 4) { return FALSE; } - if (_itoa_s(dumpType, dumpTypeStr, sizeof(dumpTypeStr), 10) != 0) - { - return FALSE; - } if (dumpName != nullptr && dumpName[0] == '\0') { dumpName = nullptr; } char* program = nullptr; char* pidarg = nullptr; - BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, (char*)dumpName, dumpTypeStr, diag, false); + BOOL result = PROCBuildCreateDumpCommandLine(argvCreateDump, &program, &pidarg, dumpName, dumpType, diag, false); if (result) { result = PROCCreateCrashDump(argvCreateDump); diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index 59d32c0c43091d..066f0d5237594e 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -9,12 +9,7 @@ #include "sstring.h" #include "ex.h" -// Config prefixes -#define COMPLUS_PREFIX W("COMPlus_") -#define LEN_OF_COMPLUS_PREFIX StrLen(COMPLUS_PREFIX) - -#define DOTNET_PREFIX W("DOTNET_") -#define LEN_OF_DOTNET_PREFIX StrLen(DOTNET_PREFIX) +#include "clrconfignocache.h" using ConfigDWORDInfo = CLRConfig::ConfigDWORDInfo; using ConfigStringInfo = CLRConfig::ConfigStringInfo; @@ -199,7 +194,23 @@ namespace } if (len != 0) + { ret = temp.GetCopyOfUnicodeString(); + +#if defined(DEBUG) && !defined(SELF_NO_HOST) + // Validate the cache and no-cache logic result in the same answer + SString nameToConvert(name); + SString nameAsUTF8; + nameToConvert.ConvertToUTF8(nameAsUTF8); + SString valueAsUTF8; + temp.ConvertToUTF8(valueAsUTF8); + + CLRConfigNoCache nonCache = CLRConfigNoCache::Get(nameAsUTF8.GetUTF8NoConvert()); + LPCSTR valueNoCache = nonCache.AsString(); + + _ASSERTE(SString::_stricmp(valueNoCache, valueAsUTF8.GetUTF8NoConvert()) == 0); +#endif // defined(DEBUG) && !defined(SELF_NO_HOST) + } } EX_CATCH_HRESULT(hr); @@ -405,6 +416,13 @@ namespace #undef CONFIG_DWORD_INFO_EX #undef CONFIG_STRING_INFO_EX +BOOL CLRConfig::IsConfigEnabled(const ConfigDWORDInfo & info) +{ + WRAPPER_NO_CONTRACT; + + return IsConfigOptionSpecified(info.name); +} + // // Look up a DWORD config value. // @@ -471,6 +489,8 @@ DWORD CLRConfig::GetConfigValue(const ConfigDWORDInfo & info, DWORD defaultValue // static DWORD CLRConfig::GetConfigValue(const ConfigDWORDInfo & info) { + WRAPPER_NO_CONTRACT; + bool unused; return GetConfigValue(info, &unused); } diff --git a/src/coreclr/vm/autotrace.cpp b/src/coreclr/vm/autotrace.cpp index 8ebfdd90766631..9d266d3d9f893d 100644 --- a/src/coreclr/vm/autotrace.cpp +++ b/src/coreclr/vm/autotrace.cpp @@ -27,20 +27,19 @@ HANDLE auto_trace_event; static size_t g_n_tracers = 1; -static const WCHAR* command_format = W("%hs -p %d"); +static const WCHAR* command_format = W("%s -p %d"); static WCHAR* command = nullptr; void auto_trace_init() { - char *nAutoTracersValue = getenv("COMPlus_AutoTrace_N_Tracers"); - if (nAutoTracersValue != NULL) + if (CLRConfig::IsConfigEnabled(CLRConfig::INTERNAL_AutoTrace_N_Tracers)) { - g_n_tracers = strtoul(nAutoTracersValue, NULL, 10); + g_n_tracers = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_AutoTrace_N_Tracers); } // Get the command to run auto-trace. Note that the `-p ` option // will be automatically added for you - char *commandTextValue = getenv("COMPlus_AutoTrace_Command"); + LPWSTR commandTextValue = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_AutoTrace_Command); if (commandTextValue != NULL) { DWORD currentProcessId = GetCurrentProcessId(); diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index f79460fc4fce3f..50c13059b8fc6b 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -7,6 +7,7 @@ #include "common.h" #if defined(FEATURE_PERFMAP) && !defined(DACCESS_COMPILE) +#include #include "perfmap.h" #include "perfinfo.h" #include "pal.h" @@ -49,16 +50,18 @@ void PerfMap::Initialize() s_enabled = true; - char jitdumpPath[4096]; + char* jitdumpPath; + char jitdumpPathBuffer[4096]; - // CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_PerfMapJitDumpPath) returns a LPWSTR - // Use GetEnvironmentVariableA because it is simpler. - // Keep comment here to make it searchable. - DWORD len = GetEnvironmentVariableA("COMPlus_PerfMapJitDumpPath", jitdumpPath, sizeof(jitdumpPath) - 1); - - if (len == 0) + CLRConfigNoCache value = CLRConfigNoCache::Get("PerfMapJitDumpPath"); + if (value.IsSet()) + { + jitdumpPath = value.AsString() + } + else { - GetTempPathA(sizeof(jitdumpPath) - 1, jitdumpPath); + GetTempPathA(sizeof(jitdumpPathBuffer) - 1, jitdumpPathBuffer); + jitdumpPath = jitdumpPathBuffer; } PAL_PerfJitDump_Start(jitdumpPath); From 749728dedb93ec95f01e0e6a8fe5dfc0c5ad03ee Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 09:10:43 -0700 Subject: [PATCH 2/9] Build issue on Linux Respect the "no prefix" case for no-cache config look up. --- src/coreclr/inc/clrconfignocache.h | 33 ++++++++++++++----- src/coreclr/pal/src/exception/signal.cpp | 2 +- .../pal/src/misc/tracepointprovider.cpp | 10 +++--- src/coreclr/utilcode/clrconfig.cpp | 2 +- 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/coreclr/inc/clrconfignocache.h b/src/coreclr/inc/clrconfignocache.h index e6364844674744..2ecc5a545f283d 100644 --- a/src/coreclr/inc/clrconfignocache.h +++ b/src/coreclr/inc/clrconfignocache.h @@ -48,25 +48,40 @@ class CLRConfigNoCache return fSuccess; } - static CLRConfigNoCache Get(LPCSTR cfg) + static CLRConfigNoCache Get(LPCSTR cfg, bool noPrefix = false) { if (cfg == nullptr) return {}; char nameBuffer[64]; - const char* fallbackPrefix = COMPLUS_PREFIX_A; + const char* fallbackPrefix = NULL; const size_t namelen = strlen(cfg); - bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); - bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); - if (!dotnetValid || !complusValid) + if (noPrefix) { - _ASSERTE(!"Environment variable name too long."); - return {}; + if (namelen >= _countof(nameBuffer)) + { + _ASSERTE(!"Environment variable name too long."); + return {}; + } + + *nameBuffer = W('\0'); + } + else + { + bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); + bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); + if (!dotnetValid || !complusValid) + { + _ASSERTE(!"Environment variable name too long."); + return {}; + } + + // Priority order is DOTNET_ and then COMPlus_. + strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); + fallbackPrefix = COMPLUS_PREFIX_A; } - // Priority order is DOTNET_ and then COMPlus_. - strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); strcat_s(nameBuffer, _countof(nameBuffer), cfg); LPCSTR val = getenv(nameBuffer); diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index 66bd19bb403d9a..fc7c0ff50631fa 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -153,7 +153,7 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags) if (stackCheck.IsSet()) { DWORD value; - if (stackCheck.TryGetInteger(10, value)) + if (stackCheck.TryAsInteger(10, value)) g_enable_alternate_stack_check = (value != 0); } #endif diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index 590a1a156a5d54..5095d946ec342e 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -23,13 +23,13 @@ Revision History: #include "pal/malloc.hpp" #include "pal/stackstring.hpp" -#include - #include #include #include #include +#include + SET_DEFAULT_DEBUG_CHANNEL(MISC); /*++ @@ -64,11 +64,11 @@ PAL_InitializeTracing(void) // Check if loading the LTTng providers should be disabled. // Note: this env var is formally declared in clrconfigvalues.h, but // this code is executed too early to use the mechanics that come with that definition. - CLRConfigNoCache disableLTTng = CLRConfigNoCache::Get("LTTng"); - if (disableLTTng.IsSet()) + CLRConfigNoCache cfgLTTng = CLRConfigNoCache::Get("LTTng"); + if (cfgLTTng.IsSet()) { DWORD value; - if (disableLTTng.TryGetInteger(10, value)) + if (cfgLTTng.TryAsInteger(10, value)) fShouldLoad = (int)value; } diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index 066f0d5237594e..98518c4dd8ebc1 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -205,7 +205,7 @@ namespace SString valueAsUTF8; temp.ConvertToUTF8(valueAsUTF8); - CLRConfigNoCache nonCache = CLRConfigNoCache::Get(nameAsUTF8.GetUTF8NoConvert()); + CLRConfigNoCache nonCache = CLRConfigNoCache::Get(nameAsUTF8.GetUTF8NoConvert(), noPrefix); LPCSTR valueNoCache = nonCache.AsString(); _ASSERTE(SString::_stricmp(valueNoCache, valueAsUTF8.GetUTF8NoConvert()) == 0); From e058c3662f1439945d6da44f0063f535950d502c Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 09:51:47 -0700 Subject: [PATCH 3/9] Move header inclusion post debug channel definition. --- src/coreclr/pal/src/misc/tracepointprovider.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index 5095d946ec342e..35aac5ceb55e33 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -28,10 +28,12 @@ Revision History: #include #include -#include - SET_DEFAULT_DEBUG_CHANNEL(MISC); +// clrconfignocache.h uses macro _ASSERTE, which needd to use variable +// defdbgchan defined by SET_DEFAULT_DEBUG_CHANNEL. +#include + /*++ Initialization logic for LTTng tracepoint providers. From 92ad1d6a075abe09456f5eba3889f823caf7eb60 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 10:08:56 -0700 Subject: [PATCH 4/9] Missing const on local declaration. --- src/coreclr/vm/perfmap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 50c13059b8fc6b..a7ee1287479e83 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -50,7 +50,7 @@ void PerfMap::Initialize() s_enabled = true; - char* jitdumpPath; + const char* jitdumpPath; char jitdumpPathBuffer[4096]; CLRConfigNoCache value = CLRConfigNoCache::Get("PerfMapJitDumpPath"); From 6994a17e6e0112be34e5f841525bc713a7075e88 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 10:47:31 -0700 Subject: [PATCH 5/9] Missing ; --- src/coreclr/vm/perfmap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index a7ee1287479e83..31da16cac5586d 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -56,7 +56,7 @@ void PerfMap::Initialize() CLRConfigNoCache value = CLRConfigNoCache::Get("PerfMapJitDumpPath"); if (value.IsSet()) { - jitdumpPath = value.AsString() + jitdumpPath = value.AsString(); } else { From 4aab95786dca51a3b79d18298e4b3581d68424b4 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 16:23:53 -0700 Subject: [PATCH 6/9] Address some linker issues when consuming from the PAL. --- src/coreclr/inc/clrconfignocache.h | 84 +++++++++++-------- src/coreclr/pal/src/exception/signal.cpp | 2 +- src/coreclr/pal/src/init/pal.cpp | 4 +- .../pal/src/misc/tracepointprovider.cpp | 2 +- src/coreclr/pal/src/thread/process.cpp | 10 +-- src/coreclr/utilcode/clrconfig.cpp | 2 +- src/coreclr/vm/perfmap.cpp | 2 +- 7 files changed, 58 insertions(+), 48 deletions(-) diff --git a/src/coreclr/inc/clrconfignocache.h b/src/coreclr/inc/clrconfignocache.h index 2ecc5a545f283d..31f1785727924c 100644 --- a/src/coreclr/inc/clrconfignocache.h +++ b/src/coreclr/inc/clrconfignocache.h @@ -7,6 +7,9 @@ // Logic for resolving configuration names. // +#ifndef _CLRCONFIGNOCACHE_H_ +#define _CLRCONFIGNOCACHE_H_ + #ifndef StrLen #define StrLen(STR) ((sizeof(STR) / sizeof(STR[0])) - 1) #endif // !StrLen @@ -48,50 +51,57 @@ class CLRConfigNoCache return fSuccess; } - static CLRConfigNoCache Get(LPCSTR cfg, bool noPrefix = false) - { - if (cfg == nullptr) - return {}; + friend CLRConfigNoCache GetCLRConfigNoCache(LPCSTR cfg, bool noPrefix); +}; - char nameBuffer[64]; - const char* fallbackPrefix = NULL; - const size_t namelen = strlen(cfg); +// This function is not a member of CLRConfigNoCache and marked inline so linkage +// is relative to the compilation unit it is included in. The linkage concerns here +// are due to usage in the PAL regarding getenv. +inline CLRConfigNoCache GetCLRConfigNoCache(LPCSTR cfg, bool noPrefix = false) +{ + if (cfg == nullptr) + return {}; - if (noPrefix) - { - if (namelen >= _countof(nameBuffer)) - { - _ASSERTE(!"Environment variable name too long."); - return {}; - } + char nameBuffer[64]; + const char* fallbackPrefix = NULL; + const size_t namelen = strlen(cfg); - *nameBuffer = W('\0'); - } - else + if (noPrefix) + { + if (namelen >= _countof(nameBuffer)) { - bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); - bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); - if (!dotnetValid || !complusValid) - { - _ASSERTE(!"Environment variable name too long."); - return {}; - } - - // Priority order is DOTNET_ and then COMPlus_. - strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); - fallbackPrefix = COMPLUS_PREFIX_A; + _ASSERTE(!"Environment variable name too long."); + return {}; } - strcat_s(nameBuffer, _countof(nameBuffer), cfg); - - LPCSTR val = getenv(nameBuffer); - if (val == NULL && fallbackPrefix != NULL) + *nameBuffer = W('\0'); + } + else + { + bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); + bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); + if (!dotnetValid || !complusValid) { - strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); - strcat_s(nameBuffer, _countof(nameBuffer), cfg); - val = getenv(nameBuffer); + _ASSERTE(!"Environment variable name too long."); + return {}; } - return { val }; + // Priority order is DOTNET_ and then COMPlus_. + strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); + fallbackPrefix = COMPLUS_PREFIX_A; } -}; + + strcat_s(nameBuffer, _countof(nameBuffer), cfg); + + LPCSTR val = getenv(nameBuffer); + if (val == NULL && fallbackPrefix != NULL) + { + strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); + strcat_s(nameBuffer, _countof(nameBuffer), cfg); + val = getenv(nameBuffer); + } + + return { val }; +} + +#endif // _CLRCONFIGNOCACHE_H_ diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index fc7c0ff50631fa..20ca8d778a6e1b 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -149,7 +149,7 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags) #if !HAVE_MACH_EXCEPTIONS g_enable_alternate_stack_check = false; - CLRConfigNoCache stackCheck = CLRConfigNoCache::Get("EnableAlternateStackCheck"); + CLRConfigNoCache stackCheck = GetCLRConfigNoCache("EnableAlternateStackCheck"); if (stackCheck.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/init/pal.cpp b/src/coreclr/pal/src/init/pal.cpp index 0bce2e4acb44f7..d808dd6dcde7cc 100644 --- a/src/coreclr/pal/src/init/pal.cpp +++ b/src/coreclr/pal/src/init/pal.cpp @@ -269,7 +269,7 @@ EnsureStackSize(SIZE_T stackSize) void InitializeDefaultStackSize() { - CLRConfigNoCache defStackSize = CLRConfigNoCache::Get("DefaultStackSize"); + CLRConfigNoCache defStackSize = GetCLRConfigNoCache("DefaultStackSize"); if (defStackSize.IsSet()) { DWORD size; @@ -403,7 +403,7 @@ Initialize( #endif // ENSURE_PRIMARY_STACK_SIZE #ifdef FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION - CLRConfigNoCache useDefaultBaseAddr = CLRConfigNoCache::Get("UseDefaultBaseAddr"); + CLRConfigNoCache useDefaultBaseAddr = GetCLRConfigNoCache("UseDefaultBaseAddr"); if (useDefaultBaseAddr.IsSet()) { DWORD flag; diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index 35aac5ceb55e33..f45c572f3a8f69 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -66,7 +66,7 @@ PAL_InitializeTracing(void) // Check if loading the LTTng providers should be disabled. // Note: this env var is formally declared in clrconfigvalues.h, but // this code is executed too early to use the mechanics that come with that definition. - CLRConfigNoCache cfgLTTng = CLRConfigNoCache::Get("LTTng"); + CLRConfigNoCache cfgLTTng = GetCLRConfigNoCache("LTTng"); if (cfgLTTng.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 1d1ab193a22c6b..b6d00d62a9367e 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -3276,16 +3276,16 @@ Return BOOL PROCAbortInitialize() { - CLRConfigNoCache enabledCfg= CLRConfigNoCache::Get("DbgEnableMiniDump"); + CLRConfigNoCache enabledCfg= GetCLRConfigNoCache("DbgEnableMiniDump"); DWORD enabled = 0; if (enabledCfg.IsSet() && enabledCfg.TryAsInteger(10, enabled) && enabled) { - CLRConfigNoCache dmpNameCfg = CLRConfigNoCache::Get("DbgMiniDumpName"); + CLRConfigNoCache dmpNameCfg = GetCLRConfigNoCache("DbgMiniDumpName"); - CLRConfigNoCache dmpTypeCfg = CLRConfigNoCache::Get("DbgMiniDumpType"); + CLRConfigNoCache dmpTypeCfg = GetCLRConfigNoCache("DbgMiniDumpType"); DWORD dumpType = UndefinedDumpType; if (dmpTypeCfg.IsSet()) { @@ -3296,11 +3296,11 @@ PROCAbortInitialize() } } - CLRConfigNoCache createDumpCfg = CLRConfigNoCache::Get("CreateDumpDiagnostics"); + CLRConfigNoCache createDumpCfg = GetCLRConfigNoCache("CreateDumpDiagnostics"); DWORD val = 0; BOOL diag = createDumpCfg.IsSet() && createDumpCfg.TryAsInteger(10, val) && val == 1; - CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport"); + CLRConfigNoCache enabldReportCfg = GetCLRConfigNoCache("EnableCrashReport"); val = 0; BOOL crashReport = enabldReportCfg.IsSet() && enabldReportCfg.TryAsInteger(10, val) && val == 1; diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index 98518c4dd8ebc1..81aeeab11ba300 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -205,7 +205,7 @@ namespace SString valueAsUTF8; temp.ConvertToUTF8(valueAsUTF8); - CLRConfigNoCache nonCache = CLRConfigNoCache::Get(nameAsUTF8.GetUTF8NoConvert(), noPrefix); + CLRConfigNoCache nonCache = GetCLRConfigNoCache(nameAsUTF8.GetUTF8NoConvert(), noPrefix); LPCSTR valueNoCache = nonCache.AsString(); _ASSERTE(SString::_stricmp(valueNoCache, valueAsUTF8.GetUTF8NoConvert()) == 0); diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 31da16cac5586d..7fa9e39e6f8bd6 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -53,7 +53,7 @@ void PerfMap::Initialize() const char* jitdumpPath; char jitdumpPathBuffer[4096]; - CLRConfigNoCache value = CLRConfigNoCache::Get("PerfMapJitDumpPath"); + CLRConfigNoCache value = GetCLRConfigNoCache("PerfMapJitDumpPath"); if (value.IsSet()) { jitdumpPath = value.AsString(); From deff52e4d6e1c7c389c2840d1fd413a0566bc6a2 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 16:56:46 -0700 Subject: [PATCH 7/9] Revert "Address some linker issues when consuming from the PAL." This reverts commit 4aab95786dca51a3b79d18298e4b3581d68424b4. --- src/coreclr/inc/clrconfignocache.h | 84 ++++++++----------- src/coreclr/pal/src/exception/signal.cpp | 2 +- src/coreclr/pal/src/init/pal.cpp | 4 +- .../pal/src/misc/tracepointprovider.cpp | 2 +- src/coreclr/pal/src/thread/process.cpp | 10 +-- src/coreclr/utilcode/clrconfig.cpp | 2 +- src/coreclr/vm/perfmap.cpp | 2 +- 7 files changed, 48 insertions(+), 58 deletions(-) diff --git a/src/coreclr/inc/clrconfignocache.h b/src/coreclr/inc/clrconfignocache.h index 31f1785727924c..2ecc5a545f283d 100644 --- a/src/coreclr/inc/clrconfignocache.h +++ b/src/coreclr/inc/clrconfignocache.h @@ -7,9 +7,6 @@ // Logic for resolving configuration names. // -#ifndef _CLRCONFIGNOCACHE_H_ -#define _CLRCONFIGNOCACHE_H_ - #ifndef StrLen #define StrLen(STR) ((sizeof(STR) / sizeof(STR[0])) - 1) #endif // !StrLen @@ -51,57 +48,50 @@ class CLRConfigNoCache return fSuccess; } - friend CLRConfigNoCache GetCLRConfigNoCache(LPCSTR cfg, bool noPrefix); -}; - -// This function is not a member of CLRConfigNoCache and marked inline so linkage -// is relative to the compilation unit it is included in. The linkage concerns here -// are due to usage in the PAL regarding getenv. -inline CLRConfigNoCache GetCLRConfigNoCache(LPCSTR cfg, bool noPrefix = false) -{ - if (cfg == nullptr) - return {}; + static CLRConfigNoCache Get(LPCSTR cfg, bool noPrefix = false) + { + if (cfg == nullptr) + return {}; - char nameBuffer[64]; - const char* fallbackPrefix = NULL; - const size_t namelen = strlen(cfg); + char nameBuffer[64]; + const char* fallbackPrefix = NULL; + const size_t namelen = strlen(cfg); - if (noPrefix) - { - if (namelen >= _countof(nameBuffer)) + if (noPrefix) { - _ASSERTE(!"Environment variable name too long."); - return {}; - } + if (namelen >= _countof(nameBuffer)) + { + _ASSERTE(!"Environment variable name too long."); + return {}; + } - *nameBuffer = W('\0'); - } - else - { - bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); - bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); - if (!dotnetValid || !complusValid) + *nameBuffer = W('\0'); + } + else { - _ASSERTE(!"Environment variable name too long."); - return {}; + bool dotnetValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_DOTNET_PREFIX); + bool complusValid = namelen < (size_t)(_countof(nameBuffer) - 1 - LEN_OF_COMPLUS_PREFIX); + if (!dotnetValid || !complusValid) + { + _ASSERTE(!"Environment variable name too long."); + return {}; + } + + // Priority order is DOTNET_ and then COMPlus_. + strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); + fallbackPrefix = COMPLUS_PREFIX_A; } - // Priority order is DOTNET_ and then COMPlus_. - strcpy_s(nameBuffer, _countof(nameBuffer), DOTNET_PREFIX_A); - fallbackPrefix = COMPLUS_PREFIX_A; - } - - strcat_s(nameBuffer, _countof(nameBuffer), cfg); - - LPCSTR val = getenv(nameBuffer); - if (val == NULL && fallbackPrefix != NULL) - { - strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); strcat_s(nameBuffer, _countof(nameBuffer), cfg); - val = getenv(nameBuffer); - } - return { val }; -} + LPCSTR val = getenv(nameBuffer); + if (val == NULL && fallbackPrefix != NULL) + { + strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); + strcat_s(nameBuffer, _countof(nameBuffer), cfg); + val = getenv(nameBuffer); + } -#endif // _CLRCONFIGNOCACHE_H_ + return { val }; + } +}; diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index 20ca8d778a6e1b..fc7c0ff50631fa 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -149,7 +149,7 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags) #if !HAVE_MACH_EXCEPTIONS g_enable_alternate_stack_check = false; - CLRConfigNoCache stackCheck = GetCLRConfigNoCache("EnableAlternateStackCheck"); + CLRConfigNoCache stackCheck = CLRConfigNoCache::Get("EnableAlternateStackCheck"); if (stackCheck.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/init/pal.cpp b/src/coreclr/pal/src/init/pal.cpp index d808dd6dcde7cc..0bce2e4acb44f7 100644 --- a/src/coreclr/pal/src/init/pal.cpp +++ b/src/coreclr/pal/src/init/pal.cpp @@ -269,7 +269,7 @@ EnsureStackSize(SIZE_T stackSize) void InitializeDefaultStackSize() { - CLRConfigNoCache defStackSize = GetCLRConfigNoCache("DefaultStackSize"); + CLRConfigNoCache defStackSize = CLRConfigNoCache::Get("DefaultStackSize"); if (defStackSize.IsSet()) { DWORD size; @@ -403,7 +403,7 @@ Initialize( #endif // ENSURE_PRIMARY_STACK_SIZE #ifdef FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION - CLRConfigNoCache useDefaultBaseAddr = GetCLRConfigNoCache("UseDefaultBaseAddr"); + CLRConfigNoCache useDefaultBaseAddr = CLRConfigNoCache::Get("UseDefaultBaseAddr"); if (useDefaultBaseAddr.IsSet()) { DWORD flag; diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index f45c572f3a8f69..35aac5ceb55e33 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -66,7 +66,7 @@ PAL_InitializeTracing(void) // Check if loading the LTTng providers should be disabled. // Note: this env var is formally declared in clrconfigvalues.h, but // this code is executed too early to use the mechanics that come with that definition. - CLRConfigNoCache cfgLTTng = GetCLRConfigNoCache("LTTng"); + CLRConfigNoCache cfgLTTng = CLRConfigNoCache::Get("LTTng"); if (cfgLTTng.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index b6d00d62a9367e..1d1ab193a22c6b 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -3276,16 +3276,16 @@ Return BOOL PROCAbortInitialize() { - CLRConfigNoCache enabledCfg= GetCLRConfigNoCache("DbgEnableMiniDump"); + CLRConfigNoCache enabledCfg= CLRConfigNoCache::Get("DbgEnableMiniDump"); DWORD enabled = 0; if (enabledCfg.IsSet() && enabledCfg.TryAsInteger(10, enabled) && enabled) { - CLRConfigNoCache dmpNameCfg = GetCLRConfigNoCache("DbgMiniDumpName"); + CLRConfigNoCache dmpNameCfg = CLRConfigNoCache::Get("DbgMiniDumpName"); - CLRConfigNoCache dmpTypeCfg = GetCLRConfigNoCache("DbgMiniDumpType"); + CLRConfigNoCache dmpTypeCfg = CLRConfigNoCache::Get("DbgMiniDumpType"); DWORD dumpType = UndefinedDumpType; if (dmpTypeCfg.IsSet()) { @@ -3296,11 +3296,11 @@ PROCAbortInitialize() } } - CLRConfigNoCache createDumpCfg = GetCLRConfigNoCache("CreateDumpDiagnostics"); + CLRConfigNoCache createDumpCfg = CLRConfigNoCache::Get("CreateDumpDiagnostics"); DWORD val = 0; BOOL diag = createDumpCfg.IsSet() && createDumpCfg.TryAsInteger(10, val) && val == 1; - CLRConfigNoCache enabldReportCfg = GetCLRConfigNoCache("EnableCrashReport"); + CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport"); val = 0; BOOL crashReport = enabldReportCfg.IsSet() && enabldReportCfg.TryAsInteger(10, val) && val == 1; diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index 81aeeab11ba300..98518c4dd8ebc1 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -205,7 +205,7 @@ namespace SString valueAsUTF8; temp.ConvertToUTF8(valueAsUTF8); - CLRConfigNoCache nonCache = GetCLRConfigNoCache(nameAsUTF8.GetUTF8NoConvert(), noPrefix); + CLRConfigNoCache nonCache = CLRConfigNoCache::Get(nameAsUTF8.GetUTF8NoConvert(), noPrefix); LPCSTR valueNoCache = nonCache.AsString(); _ASSERTE(SString::_stricmp(valueNoCache, valueAsUTF8.GetUTF8NoConvert()) == 0); diff --git a/src/coreclr/vm/perfmap.cpp b/src/coreclr/vm/perfmap.cpp index 7fa9e39e6f8bd6..31da16cac5586d 100644 --- a/src/coreclr/vm/perfmap.cpp +++ b/src/coreclr/vm/perfmap.cpp @@ -53,7 +53,7 @@ void PerfMap::Initialize() const char* jitdumpPath; char jitdumpPathBuffer[4096]; - CLRConfigNoCache value = GetCLRConfigNoCache("PerfMapJitDumpPath"); + CLRConfigNoCache value = CLRConfigNoCache::Get("PerfMapJitDumpPath"); if (value.IsSet()) { jitdumpPath = value.AsString(); From 950246ea8f7483c7160cef7e3c2591dc205114b9 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 17:25:59 -0700 Subject: [PATCH 8/9] Another attempt addressing PAL usage. --- src/coreclr/inc/clrconfignocache.h | 9 +++------ src/coreclr/pal/src/exception/signal.cpp | 2 +- src/coreclr/pal/src/init/pal.cpp | 4 ++-- src/coreclr/pal/src/misc/tracepointprovider.cpp | 2 +- src/coreclr/pal/src/thread/process.cpp | 10 +++++----- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/coreclr/inc/clrconfignocache.h b/src/coreclr/inc/clrconfignocache.h index 2ecc5a545f283d..46bc2182686b23 100644 --- a/src/coreclr/inc/clrconfignocache.h +++ b/src/coreclr/inc/clrconfignocache.h @@ -48,11 +48,8 @@ class CLRConfigNoCache return fSuccess; } - static CLRConfigNoCache Get(LPCSTR cfg, bool noPrefix = false) + static CLRConfigNoCache Get(LPCSTR cfg, bool noPrefix = false, char*(*getEnvFptr)(const char*) = nullptr) { - if (cfg == nullptr) - return {}; - char nameBuffer[64]; const char* fallbackPrefix = NULL; const size_t namelen = strlen(cfg); @@ -84,12 +81,12 @@ class CLRConfigNoCache strcat_s(nameBuffer, _countof(nameBuffer), cfg); - LPCSTR val = getenv(nameBuffer); + LPCSTR val = getEnvFptr != NULL ? getEnvFptr(nameBuffer) : getenv(nameBuffer); if (val == NULL && fallbackPrefix != NULL) { strcpy_s(nameBuffer, _countof(nameBuffer), fallbackPrefix); strcat_s(nameBuffer, _countof(nameBuffer), cfg); - val = getenv(nameBuffer); + val = getEnvFptr != NULL ? getEnvFptr(nameBuffer) : getenv(nameBuffer); } return { val }; diff --git a/src/coreclr/pal/src/exception/signal.cpp b/src/coreclr/pal/src/exception/signal.cpp index fc7c0ff50631fa..521150530fa33e 100644 --- a/src/coreclr/pal/src/exception/signal.cpp +++ b/src/coreclr/pal/src/exception/signal.cpp @@ -149,7 +149,7 @@ BOOL SEHInitializeSignals(CorUnix::CPalThread *pthrCurrent, DWORD flags) #if !HAVE_MACH_EXCEPTIONS g_enable_alternate_stack_check = false; - CLRConfigNoCache stackCheck = CLRConfigNoCache::Get("EnableAlternateStackCheck"); + CLRConfigNoCache stackCheck = CLRConfigNoCache::Get("EnableAlternateStackCheck", /*noprefix*/ false, &getenv); if (stackCheck.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/init/pal.cpp b/src/coreclr/pal/src/init/pal.cpp index 0bce2e4acb44f7..0774ca7bf696ac 100644 --- a/src/coreclr/pal/src/init/pal.cpp +++ b/src/coreclr/pal/src/init/pal.cpp @@ -269,7 +269,7 @@ EnsureStackSize(SIZE_T stackSize) void InitializeDefaultStackSize() { - CLRConfigNoCache defStackSize = CLRConfigNoCache::Get("DefaultStackSize"); + CLRConfigNoCache defStackSize = CLRConfigNoCache::Get("DefaultStackSize", /*noprefix*/ false, &getenv); if (defStackSize.IsSet()) { DWORD size; @@ -403,7 +403,7 @@ Initialize( #endif // ENSURE_PRIMARY_STACK_SIZE #ifdef FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION - CLRConfigNoCache useDefaultBaseAddr = CLRConfigNoCache::Get("UseDefaultBaseAddr"); + CLRConfigNoCache useDefaultBaseAddr = CLRConfigNoCache::Get("UseDefaultBaseAddr", /*noprefix*/ false, &getenv); if (useDefaultBaseAddr.IsSet()) { DWORD flag; diff --git a/src/coreclr/pal/src/misc/tracepointprovider.cpp b/src/coreclr/pal/src/misc/tracepointprovider.cpp index 35aac5ceb55e33..3cfc5e0690e20e 100644 --- a/src/coreclr/pal/src/misc/tracepointprovider.cpp +++ b/src/coreclr/pal/src/misc/tracepointprovider.cpp @@ -66,7 +66,7 @@ PAL_InitializeTracing(void) // Check if loading the LTTng providers should be disabled. // Note: this env var is formally declared in clrconfigvalues.h, but // this code is executed too early to use the mechanics that come with that definition. - CLRConfigNoCache cfgLTTng = CLRConfigNoCache::Get("LTTng"); + CLRConfigNoCache cfgLTTng = CLRConfigNoCache::Get("LTTng", /*noprefix*/ false, &getenv); if (cfgLTTng.IsSet()) { DWORD value; diff --git a/src/coreclr/pal/src/thread/process.cpp b/src/coreclr/pal/src/thread/process.cpp index 1d1ab193a22c6b..4272105c91b53d 100644 --- a/src/coreclr/pal/src/thread/process.cpp +++ b/src/coreclr/pal/src/thread/process.cpp @@ -3276,16 +3276,16 @@ Return BOOL PROCAbortInitialize() { - CLRConfigNoCache enabledCfg= CLRConfigNoCache::Get("DbgEnableMiniDump"); + CLRConfigNoCache enabledCfg= CLRConfigNoCache::Get("DbgEnableMiniDump", /*noprefix*/ false, &getenv); DWORD enabled = 0; if (enabledCfg.IsSet() && enabledCfg.TryAsInteger(10, enabled) && enabled) { - CLRConfigNoCache dmpNameCfg = CLRConfigNoCache::Get("DbgMiniDumpName"); + CLRConfigNoCache dmpNameCfg = CLRConfigNoCache::Get("DbgMiniDumpName", /*noprefix*/ false, &getenv); - CLRConfigNoCache dmpTypeCfg = CLRConfigNoCache::Get("DbgMiniDumpType"); + CLRConfigNoCache dmpTypeCfg = CLRConfigNoCache::Get("DbgMiniDumpType", /*noprefix*/ false, &getenv); DWORD dumpType = UndefinedDumpType; if (dmpTypeCfg.IsSet()) { @@ -3296,11 +3296,11 @@ PROCAbortInitialize() } } - CLRConfigNoCache createDumpCfg = CLRConfigNoCache::Get("CreateDumpDiagnostics"); + CLRConfigNoCache createDumpCfg = CLRConfigNoCache::Get("CreateDumpDiagnostics", /*noprefix*/ false, &getenv); DWORD val = 0; BOOL diag = createDumpCfg.IsSet() && createDumpCfg.TryAsInteger(10, val) && val == 1; - CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport"); + CLRConfigNoCache enabldReportCfg = CLRConfigNoCache::Get("EnableCrashReport", /*noprefix*/ false, &getenv); val = 0; BOOL crashReport = enabldReportCfg.IsSet() && enabldReportCfg.TryAsInteger(10, val) && val == 1; From 471ec628f8db03553695261ea3868ad9fb886d09 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 23 Sep 2021 19:21:07 -0700 Subject: [PATCH 9/9] Define the AutoTrace_N_Tracers config to use 10-based parsing. --- src/coreclr/inc/clrconfigvalues.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index ef078c9c334063..312017742f9992 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -678,7 +678,7 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeProcNumbers, W("EventPipeProcNumbers" RETAIL_CONFIG_DWORD_INFO(INTERNAL_EventPipeOutputStreaming, W("EventPipeOutputStreaming"), 0, "Enable/disable streaming for trace file set in COMPlus_EventPipeOutputPath. Non-zero values enable streaming.") #ifdef FEATURE_AUTO_TRACE -RETAIL_CONFIG_DWORD_INFO(INTERNAL_AutoTrace_N_Tracers, W("AutoTrace_N_Tracers"), 0, "") +RETAIL_CONFIG_DWORD_INFO_EX(INTERNAL_AutoTrace_N_Tracers, W("AutoTrace_N_Tracers"), 0, "", CLRConfig::LookupOptions::ParseIntegerAsBase10) RETAIL_CONFIG_STRING_INFO(INTERNAL_AutoTrace_Command, W("AutoTrace_Command"), "") #endif // FEATURE_AUTO_TRACE