summaryrefslogtreecommitdiff
path: root/pkgs/development/compilers/dotnet/source/vmr-compiler-opt-v9.patch
blob: cffa4be253107565131276fa4822d02b652914ee (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
From 4e333377f97ab8f0f47ba7606844c34cb61d1db0 Mon Sep 17 00:00:00 2001
From: Omair Majid <omajid@redhat.com>
Date: Mon, 9 Dec 2024 17:44:10 -0500
Subject: [PATCH 1/4] Avoid all compiler optimization on embedded apphost hash

We assume that there is a single copy of the apphost hash in the apphost
binary. And that it hasn't been modified by the compiler. However, the
compiler can optimize the hash multiple ways, including re-ordering
elements of the hash  or duplicating the contents of the hash. This can
currently happen under certain compiler versions and optimization flags.

Try and avoid that by marking the hash as a volatile string and
implementing comparisons/copying/initialization that respects that.

Fixes: #109611
---
 src/runtime/src/native/corehost/corehost.cpp | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
index 6de7acfbd08576..6d40a337d574a2 100644
--- a/src/runtime/src/native/corehost/corehost.cpp
+++ b/src/runtime/src/native/corehost/corehost.cpp
@@ -40,6 +40,24 @@
 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
 #define EMBED_HASH_FULL_UTF8    (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
 
+void to_non_volatile(volatile const char* cstr, char* output, size_t length)
+{
+    for (size_t i = 0; i < length; i++)
+    {
+        output[i] = cstr[i];
+    }
+}
+
+bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
+{
+    for (size_t i = 0; i < length; i++)
+    {
+        if (*a++ != *b++)
+            return false;
+    }
+    return true;
+}
+
 bool is_exe_enabled_for_execution(pal::string_t* app_dll)
 {
     constexpr int EMBED_SZ = sizeof(EMBED_HASH_FULL_UTF8) / sizeof(EMBED_HASH_FULL_UTF8[0]);
@@ -48,18 +66,21 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
     // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
     // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
     // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
-    static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8;     // series of NULs followed by embed hash string
+    volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8;     // series of NULs followed by embed hash string
 
     static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
     static const char lo_part[] = EMBED_HASH_LO_PART_UTF8;
 
-    if (!pal::clr_palstring(embed, app_dll))
+    char working_copy_embed[EMBED_MAX];
+    to_non_volatile(embed, working_copy_embed, EMBED_MAX);
+
+    if (!pal::clr_palstring(&working_copy_embed[0], app_dll))
     {
         trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image."));
         return false;
     }
 
-    std::string binding(&embed[0]);
+    std::string binding(&working_copy_embed[0]);
 
     // Check if the path exceeds the max allowed size
     if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
@@ -74,8 +95,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
     size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1;
     size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1;
     if (binding.size() >= (hi_len + lo_len)
-        && binding.compare(0, hi_len, &hi_part[0]) == 0
-        && binding.compare(hi_len, lo_len, &lo_part[0]) == 0)
+        && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len)
+        && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len))
     {
         trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str());
         return false;

From 2c67debff3f84519b7b5cba49232aaa2396a9f3e Mon Sep 17 00:00:00 2001
From: Aaron R Robinson <arobins@microsoft.com>
Date: Wed, 26 Mar 2025 20:40:51 -0700
Subject: [PATCH 2/4] Apply feedback

---
 src/runtime/src/native/corehost/corehost.cpp | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
index 6d40a337d574a2..9d2648c0ba84fa 100644
--- a/src/runtime/src/native/corehost/corehost.cpp
+++ b/src/runtime/src/native/corehost/corehost.cpp
@@ -40,14 +40,9 @@
 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
 #define EMBED_HASH_FULL_UTF8    (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
 
-void to_non_volatile(volatile const char* cstr, char* output, size_t length)
-{
-    for (size_t i = 0; i < length; i++)
-    {
-        output[i] = cstr[i];
-    }
-}
-
+// This is a workaround for a compiler workaround that
+// causes issues with inserting multiple static strings.
+// See https://github.com/dotnet/runtime/issues/109611 for more details.
 bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
 {
     for (size_t i = 0; i < length; i++)
@@ -66,21 +61,18 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
     // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
     // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
     // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
-    volatile static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8;     // series of NULs followed by embed hash string
+    static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8;     // series of NULs followed by embed hash string
 
     static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;
     static const char lo_part[] = EMBED_HASH_LO_PART_UTF8;
 
-    char working_copy_embed[EMBED_MAX];
-    to_non_volatile(embed, working_copy_embed, EMBED_MAX);
-
-    if (!pal::clr_palstring(&working_copy_embed[0], app_dll))
+    if (!pal::clr_palstring(embed, app_dll))
     {
         trace::error(_X("The managed DLL bound to this executable could not be retrieved from the executable image."));
         return false;
     }
 
-    std::string binding(&working_copy_embed[0]);
+    std::string binding(&embed[0]);
 
     // Check if the path exceeds the max allowed size
     if (binding.size() > EMBED_MAX - 1) // -1 for null terminator

From 854143d39e7725d82547032f1ab47ea5da062b9f Mon Sep 17 00:00:00 2001
From: Aaron R Robinson <arobins@microsoft.com>
Date: Thu, 27 Mar 2025 19:04:09 -0700
Subject: [PATCH 3/4] Feedback

---
 src/runtime/src/native/corehost/corehost.cpp | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
index 9d2648c0ba84fa..36902ccfa56c04 100644
--- a/src/runtime/src/native/corehost/corehost.cpp
+++ b/src/runtime/src/native/corehost/corehost.cpp
@@ -40,10 +40,10 @@
 #define EMBED_HASH_LO_PART_UTF8 "74e592c2fa383d4a3960714caef0c4f2"
 #define EMBED_HASH_FULL_UTF8    (EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8) // NUL terminated
 
-// This is a workaround for a compiler workaround that
-// causes issues with inserting multiple static strings.
+// This avoids compiler optimization which cause EMBED_HASH_HI_PART_UTF8 EMBED_HASH_LO_PART_UTF8
+// to be placed adjacent causing them to match EMBED_HASH_FULL_UTF8 when searched for replacing.
 // See https://github.com/dotnet/runtime/issues/109611 for more details.
-bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
+static bool compare_memory_nooptimization(volatile const char* a, volatile const char* b, size_t length)
 {
     for (size_t i = 0; i < length; i++)
     {
@@ -72,10 +72,10 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
         return false;
     }
 
-    std::string binding(&embed[0]);
+    size_t binding_len = strlen(&embed[0]);
 
     // Check if the path exceeds the max allowed size
-    if (binding.size() > EMBED_MAX - 1) // -1 for null terminator
+    if (binding_len > EMBED_MAX - 1) // -1 for null terminator
     {
         trace::error(_X("The managed DLL bound to this executable is longer than the max allowed length (%d)"), EMBED_MAX - 1);
         return false;
@@ -86,9 +86,9 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
     // So use two parts of the string that will be unaffected by the edit.
     size_t hi_len = (sizeof(hi_part) / sizeof(hi_part[0])) - 1;
     size_t lo_len = (sizeof(lo_part) / sizeof(lo_part[0])) - 1;
-    if (binding.size() >= (hi_len + lo_len)
-        && compare_memory_nooptimization(binding.c_str(), hi_part, hi_len)
-        && compare_memory_nooptimization(binding.substr(hi_len).c_str(), lo_part, lo_len))
+    if (binding_len >= (hi_len + lo_len)
+        && compare_memory_nooptimization(&embed[0], hi_part, hi_len)
+        && compare_memory_nooptimization(&embed[hi_len], lo_part, lo_len))
     {
         trace::error(_X("This executable is not bound to a managed DLL to execute. The binding value is: '%s'"), app_dll->c_str());
         return false;

From 842d62e499ce6511abf948cf5da8023cc6be8212 Mon Sep 17 00:00:00 2001
From: Aaron R Robinson <arobins@microsoft.com>
Date: Fri, 28 Mar 2025 15:44:47 -0700
Subject: [PATCH 4/4] Feedback

---
 src/runtime/src/native/corehost/corehost.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/runtime/src/native/corehost/corehost.cpp b/src/runtime/src/native/corehost/corehost.cpp
index 36902ccfa56c04..54eb128cb486bb 100644
--- a/src/runtime/src/native/corehost/corehost.cpp
+++ b/src/runtime/src/native/corehost/corehost.cpp
@@ -59,8 +59,8 @@ bool is_exe_enabled_for_execution(pal::string_t* app_dll)
     constexpr int EMBED_MAX = (EMBED_SZ > 1025 ? EMBED_SZ : 1025); // 1024 DLL name length, 1 NUL
 
     // Contains the EMBED_HASH_FULL_UTF8 value at compile time or the managed DLL name replaced by "dotnet build".
-    // Must not be 'const' because std::string(&embed[0]) below would bind to a const string ctor plus length
-    // where length is determined at compile time (=64) instead of the actual length of the string at runtime.
+    // Must not be 'const' because strlen below could be determined at compile time (=64) instead of the actual
+    // length of the string at runtime.
     static char embed[EMBED_MAX] = EMBED_HASH_FULL_UTF8;     // series of NULs followed by embed hash string
 
     static const char hi_part[] = EMBED_HASH_HI_PART_UTF8;