dotnet_{8,9}.stage0.vmr: fix build failure due to compiler optimizations (#421326)

authored by David McFarland and committed by GitHub b5ed0fdf 08ed4a9c

+271
+49
pkgs/development/compilers/dotnet/vmr-compiler-opt-v8.patch
··· 1 + diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp 2 + index 5edc2fbf5d5..1b3f5b1a23a 100644 3 + --- a/src/runtime/src/native/corehost/corehost.cpp 4 + +++ b/src/runtime/src/native/corehost/corehost.cpp 5 + @@ -40,14 +40,27 @@ 6 + #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2" 7 + #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated 8 + 9 + +// This avoids compiler optimization which cause EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8 10 + +// to be placed adjacent causing them to match EMBED_HASH_FULL_UTF8 when searched for replacing. 11 + +// See https://github.com/dotnet/runtime/issues/109611 for more details. 12 + +static bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length) 13 + +{ 14 + + for (size_t i = 0; i < length; i++) 15 + + { 16 + + if (*a++ != *b++) 17 + + return false; 18 + + } 19 + + return true; 20 + +} 21 + + 22 + bool is_exe_enabled_for_execution(pal::string_t* app_dll) 23 + { 24 + constexpr int EMBED_SZ = sizeof(EMBED_HASH_FULL_UTF8) / sizeof(EMBED_HASH_FULL_UTF8[0]); 25 + constexpr int EMBED_MAX = (EMBED_SZ > 1025 ? EMBED_SZ : 1025); // 1024 DLL name length, 1 NUL 26 + 27 + // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build". 28 + - // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length 29 + - // where length is determined at compile time (=64) instead of the actual length of the string at runtime. 30 + + // Must not be 'const' because strlen below could be determined at compile time (=64) instead of the actual 31 + + // length of the string at runtime. 32 + static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 33 + 34 + static const char hi_part[] = EMBED_HASH_HI_PART_UTF8; 35 + @@ -64,10 +77,10 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 36 + size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1; 37 + size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1; 38 + 39 + - std::string binding(&embed[0]); 40 + - if ((binding.size() >= (hi_len + lo_len)) && 41 + - binding.compare(0, hi_len, &hi_part[0]) == 0 && 42 + - binding.compare(hi_len, lo_len, &lo_part[0]) == 0) 43 + + size_t binding_len = strlen(&embed[0]); 44 + + if ((binding_len >= (hi_len + lo_len)) 45 + + && compare_memory_nooptimization(&embed[0], hi_part, hi_len) == 0 46 + + && compare_memory_nooptimization(&embed[hi_len], lo_part, lo_len)) 47 + { 48 + trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str()); 49 + return false;
+220
pkgs/development/compilers/dotnet/vmr-compiler-opt-v9.patch
··· 1 + From 4e333377f97ab8f0f47ba7606844c34cb61d1db0 Mon Sep 17 00:00:00 2001 2 + From: Omair Majid <omajid@redhat.com> 3 + Date: Mon, 9 Dec 2024 17:44:10 -0500 4 + Subject: [PATCH 1/4] Avoid all compiler optimization on embedded apphost hash 5 + 6 + We assume that there is a single copy of the apphost hash in the apphost 7 + binary. And that it hasn't been modified by the compiler. However, the 8 + compiler can optimize the hash multiple ways, including re-ordering 9 + elements of the hash or duplicating the contents of the hash. This can 10 + currently happen under certain compiler versions and optimization flags. 11 + 12 + Try and avoid that by marking the hash as a volatile string and 13 + implementing comparisons/copying/initialization that respects that. 14 + 15 + Fixes: #109611 16 + --- 17 + src/runtime/src/native/corehost/corehost.cpp | 31 ++++++++++++++++++++++++++----- 18 + 1 file changed, 26 insertions(+), 5 deletions(-) 19 + 20 + diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp 21 + index 6de7acfbd08576..6d40a337d574a2 100644 22 + --- a/src/runtime/src/native/corehost/corehost.cpp 23 + +++ b/src/runtime/src/native/corehost/corehost.cpp 24 + @@ -40,6 +40,24 @@ 25 + #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2" 26 + #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated 27 + 28 + +void to_non_volatile(volatile const char* cstr, char* output, size_t length) 29 + +{ 30 + + for (size_t i = 0; i < length; i++) 31 + + { 32 + + output[i] = cstr[i]; 33 + + } 34 + +} 35 + + 36 + +bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length) 37 + +{ 38 + + for (size_t i = 0; i < length; i++) 39 + + { 40 + + if (*a++ != *b++) 41 + + return false; 42 + + } 43 + + return true; 44 + +} 45 + + 46 + bool is_exe_enabled_for_execution(pal::string_t* app_dll) 47 + { 48 + constexpr int EMBED_SZ = sizeof(EMBED_HASH_FULL_UTF8) / sizeof(EMBED_HASH_FULL_UTF8[0]); 49 + @@ -48,18 +66,21 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 50 + // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build". 51 + // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length 52 + // where length is determined at compile time (=64) instead of the actual length of the string at runtime. 53 + - static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 54 + + volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 55 + 56 + static const char hi_part[] = EMBED_HASH_HI_PART_UTF8; 57 + static const char lo_part[] = EMBED_HASH_LO_PART_UTF8; 58 + 59 + - if (!pal::clr_palstring(embed, app_dll)) 60 + + char working_copy_embed[EMBED_MAX]; 61 + + to_non_volatile(embed, working_copy_embed, EMBED_MAX); 62 + + 63 + + if (!pal::clr_palstring(&working_copy_embed[0], app_dll)) 64 + { 65 + trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image.")); 66 + return false; 67 + } 68 + 69 + - std::string binding(&embed[0]); 70 + + std::string binding(&working_copy_embed[0]); 71 + 72 + // Check if the path exceeds the max allowed size 73 + if (binding.size() > EMBED_MAX - 1) // -1 for null terminator 74 + @@ -74,8 +95,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 75 + size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1; 76 + size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1; 77 + if (binding.size() >= (hi_len + lo_len) 78 + - && binding.compare(0, hi_len, &hi_part[0]) == 0 79 + - && binding.compare(hi_len, lo_len, &lo_part[0]) == 0) 80 + + && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len) 81 + + && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len)) 82 + { 83 + trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str()); 84 + return false; 85 + 86 + From 2c67debff3f84519b7b5cba49232aaa2396a9f3e Mon Sep 17 00:00:00 2001 87 + From: Aaron R Robinson <arobins@microsoft.com> 88 + Date: Wed, 26 Mar 2025 20:40:51 -0700 89 + Subject: [PATCH 2/4] Apply feedback 90 + 91 + --- 92 + src/runtime/src/native/corehost/corehost.cpp | 20 ++++++-------------- 93 + 1 file changed, 6 insertions(+), 14 deletions(-) 94 + 95 + diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp 96 + index 6d40a337d574a2..9d2648c0ba84fa 100644 97 + --- a/src/runtime/src/native/corehost/corehost.cpp 98 + +++ b/src/runtime/src/native/corehost/corehost.cpp 99 + @@ -40,14 +40,9 @@ 100 + #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2" 101 + #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated 102 + 103 + -void to_non_volatile(volatile const char* cstr, char* output, size_t length) 104 + -{ 105 + - for (size_t i = 0; i < length; i++) 106 + - { 107 + - output[i] = cstr[i]; 108 + - } 109 + -} 110 + - 111 + +// This is a workaround for a compiler workaround that 112 + +// causes issues with inserting multiple static strings. 113 + +// See https://github.com/dotnet/runtime/issues/109611 for more details. 114 + bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length) 115 + { 116 + for (size_t i = 0; i < length; i++) 117 + @@ -66,21 +61,18 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 118 + // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build". 119 + // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length 120 + // where length is determined at compile time (=64) instead of the actual length of the string at runtime. 121 + - volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 122 + + static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 123 + 124 + static const char hi_part[] = EMBED_HASH_HI_PART_UTF8; 125 + static const char lo_part[] = EMBED_HASH_LO_PART_UTF8; 126 + 127 + - char working_copy_embed[EMBED_MAX]; 128 + - to_non_volatile(embed, working_copy_embed, EMBED_MAX); 129 + - 130 + - if (!pal::clr_palstring(&working_copy_embed[0], app_dll)) 131 + + if (!pal::clr_palstring(embed, app_dll)) 132 + { 133 + trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image.")); 134 + return false; 135 + } 136 + 137 + - std::string binding(&working_copy_embed[0]); 138 + + std::string binding(&embed[0]); 139 + 140 + // Check if the path exceeds the max allowed size 141 + if (binding.size() > EMBED_MAX - 1) // -1 for null terminator 142 + 143 + From 854143d39e7725d82547032f1ab47ea5da062b9f Mon Sep 17 00:00:00 2001 144 + From: Aaron R Robinson <arobins@microsoft.com> 145 + Date: Thu, 27 Mar 2025 19:04:09 -0700 146 + Subject: [PATCH 3/4] Feedback 147 + 148 + --- 149 + src/runtime/src/native/corehost/corehost.cpp | 16 ++++++++-------- 150 + 1 file changed, 8 insertions(+), 8 deletions(-) 151 + 152 + diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp 153 + index 9d2648c0ba84fa..36902ccfa56c04 100644 154 + --- a/src/runtime/src/native/corehost/corehost.cpp 155 + +++ b/src/runtime/src/native/corehost/corehost.cpp 156 + @@ -40,10 +40,10 @@ 157 + #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2" 158 + #define EMBED_HASH_FULL_UTF8 (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated 159 + 160 + -// This is a workaround for a compiler workaround that 161 + -// causes issues with inserting multiple static strings. 162 + +// This avoids compiler optimization which cause EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8 163 + +// to be placed adjacent causing them to match EMBED_HASH_FULL_UTF8 when searched for replacing. 164 + // See https://github.com/dotnet/runtime/issues/109611 for more details. 165 + -bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length) 166 + +static bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length) 167 + { 168 + for (size_t i = 0; i < length; i++) 169 + { 170 + @@ -72,10 +72,10 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 171 + return false; 172 + } 173 + 174 + - std::string binding(&embed[0]); 175 + + size_t binding_len = strlen(&embed[0]); 176 + 177 + // Check if the path exceeds the max allowed size 178 + - if (binding.size() > EMBED_MAX - 1) // -1 for null terminator 179 + + if (binding_len > EMBED_MAX - 1) // -1 for null terminator 180 + { 181 + trace::error(_X("The managed DLL bound to this executable is longer than the max allowed length (%d)"), EMBED_MAX - 1); 182 + return false; 183 + @@ -86,9 +86,9 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 184 + // So use two parts of the string that will be unaffected by the edit. 185 + size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1; 186 + size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1; 187 + - if (binding.size() >= (hi_len + lo_len) 188 + - && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len) 189 + - && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len)) 190 + + if (binding_len >= (hi_len + lo_len) 191 + + && compare_memory_nooptimization(&embed[0], hi_part, hi_len) 192 + + && compare_memory_nooptimization(&embed[hi_len], lo_part, lo_len)) 193 + { 194 + trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str()); 195 + return false; 196 + 197 + From 842d62e499ce6511abf948cf5da8023cc6be8212 Mon Sep 17 00:00:00 2001 198 + From: Aaron R Robinson <arobins@microsoft.com> 199 + Date: Fri, 28 Mar 2025 15:44:47 -0700 200 + Subject: [PATCH 4/4] Feedback 201 + 202 + --- 203 + src/runtime/src/native/corehost/corehost.cpp | 4 ++-- 204 + 1 file changed, 2 insertions(+), 2 deletions(-) 205 + 206 + diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp 207 + index 36902ccfa56c04..54eb128cb486bb 100644 208 + --- a/src/runtime/src/native/corehost/corehost.cpp 209 + +++ b/src/runtime/src/native/corehost/corehost.cpp 210 + @@ -59,8 +59,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll) 211 + constexpr int EMBED_MAX = (EMBED_SZ > 1025 ? EMBED_SZ : 1025); // 1024 DLL name length, 1 NUL 212 + 213 + // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build". 214 + - // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length 215 + - // where length is determined at compile time (=64) instead of the actual length of the string at runtime. 216 + + // Must not be 'const' because strlen below could be determined at compile time (=64) instead of the actual 217 + + // length of the string at runtime. 218 + static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8; // series of NULs followed by embed hash string 219 + 220 + static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
+2
pkgs/development/compilers/dotnet/vmr.nix
··· 136 136 patches = 137 137 lib.optionals (lib.versionAtLeast version "9" && lib.versionOlder version "10") [ 138 138 ./UpdateNuGetConfigPackageSourcesMappings-don-t-add-em.patch 139 + ./vmr-compiler-opt-v9.patch 139 140 ] 140 141 ++ lib.optionals (lib.versionOlder version "9") [ 141 142 ./fix-aspnetcore-portable-build.patch 143 + ./vmr-compiler-opt-v8.patch 142 144 ] 143 145 ++ lib.optionals (lib.versionAtLeast version "10") [ 144 146 # src/repos/projects/Directory.Build.targets(106,5): error MSB4018: The "AddSourceToNuGetConfig" task failed unexpectedly.