Skip to content

Commit 34f58bb

Browse files
authored
[runtime] Use calloc instead of malloc. (dotnet#20692)
It's safer, since the returned memory is zero-initialized. Also add tests.
1 parent e9d59d5 commit 34f58bb

File tree

9 files changed

+89
-15
lines changed

9 files changed

+89
-15
lines changed

runtime/launcher.m

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@
102102
len++;
103103
}
104104

105-
v = value = (char *) malloc (len + 1);
105+
v = value = (char *) calloc (1, len + 1);
106106
while (start < inptr) {
107107
if (*start == '\\')
108108
start++;
@@ -162,7 +162,7 @@
162162
break;
163163
}
164164

165-
node = (ListNode *) malloc (sizeof (ListNode));
165+
node = (ListNode *) calloc (1, sizeof (ListNode));
166166
node->value = value;
167167
node->next = NULL;
168168
n++;
@@ -185,7 +185,7 @@
185185
if (n == 0)
186186
return NULL;
187187

188-
argv = (char **) malloc (sizeof (char *) * ((unsigned long) n + 1));
188+
argv = (char **) calloc (sizeof (char *), (unsigned long) n + 1);
189189
i = 0;
190190

191191
while (list != NULL) {
@@ -608,7 +608,7 @@ int xamarin_main (int argc, char **argv, enum XamarinLaunchMode launch_mode)
608608
if (xamarin_mac_modern)
609609
new_argc += 1;
610610

611-
char **new_argv = (char **) malloc (sizeof (char *) * ((unsigned long) new_argc + 1 /* null terminated */));
611+
char **new_argv = (char **) calloc (sizeof (char *), (unsigned long) new_argc + 1 /* null terminated */);
612612
const char **ptr = (const char **) new_argv;
613613
// binary
614614
*ptr++ = argv [0];

runtime/monotouch-debug.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ static ssize_t sdb_recv (void *buf, size_t len)
932932
return;
933933
}
934934

935-
sockets = (int *) malloc (sizeof (int) * ip_count);
935+
sockets = (int *) calloc (sizeof (int), ip_count);
936936
for (i = 0; i < ip_count; i++)
937937
sockets[i] = -2;
938938

@@ -1457,7 +1457,7 @@ int monotouch_debug_connect (NSMutableArray *ips, int debug_port, int output_por
14571457
return 2;
14581458
}
14591459

1460-
sockets = (int *) malloc (sizeof (int) * ip_count);
1460+
sockets = (int *) calloc (sizeof (int), ip_count);
14611461
for (i = 0; i < ip_count; i++)
14621462
sockets[i] = -1;
14631463

runtime/runtime.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,7 @@ -(void) xamarinSetFlags: (enum XamarinGCHandleFlags) flags;
20752075
return INVALID_GCHANDLE;
20762076
// PRINT ("New value: %x", (int) res);
20772077

2078-
nmp = (MethodAndPar *) malloc (sizeof (MethodAndPar));
2078+
nmp = (MethodAndPar *) calloc (1, sizeof (MethodAndPar));
20792079
*nmp = mp;
20802080

20812081
MONO_ENTER_GC_SAFE;

runtime/slinked-list.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ typedef struct SList SList;
1111
static inline SList *
1212
s_list_prepend (SList *list, void *value)
1313
{
14-
SList *first = (SList *) malloc (sizeof (SList));
14+
SList *first = (SList *) calloc (1, sizeof (SList));
1515
first->next = list;
1616
first->data = value;
1717
return first;

runtime/trampolines.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -881,7 +881,7 @@
881881
if (gc_handle == INVALID_GCHANDLE) {
882882
CFDictionaryRemoveValue (gchandle_hash, self);
883883
} else {
884-
struct gchandle_dictionary_entry *entry = (struct gchandle_dictionary_entry *) malloc (sizeof (struct gchandle_dictionary_entry));
884+
struct gchandle_dictionary_entry *entry = (struct gchandle_dictionary_entry *) calloc (1, sizeof (struct gchandle_dictionary_entry));
885885
entry->gc_handle = gc_handle;
886886
entry->flags = flags;
887887
CFDictionarySetValue (gchandle_hash, self, entry);
@@ -1813,7 +1813,7 @@
18131813
if (length == 0)
18141814
return [NSArray array];
18151815

1816-
buf = (id *) malloc (sizeof (id) * length);
1816+
buf = (id *) calloc (sizeof (id), length);
18171817

18181818
#if !defined (CORECLR_RUNTIME)
18191819
MonoClass *object_class = mono_object_get_class ((MonoObject *) array);

runtime/xamarin-support.m

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
}
101101
NSData *data = [tz data];
102102
*size = (uint32_t) [data length];
103-
void* result = malloc (*size);
103+
void* result = calloc (1, *size);
104104
[data getBytes: result length: *size];
105105
return result;
106106
}
@@ -111,7 +111,7 @@
111111
// COOP: no managed memory access: any mode.
112112
NSArray *array = [NSTimeZone knownTimeZoneNames];
113113
*count = (uint32_t) array.count;
114-
char** result = (char**) malloc (sizeof (char*) * (*count));
114+
char** result = (char**) calloc (sizeof (char*), *count);
115115
for (unsigned long i = 0; i < *count; i++) {
116116
NSString *s = [array objectAtIndex: i];
117117
result [i] = strdup (s.UTF8String);

tests/cecil-tests/Test.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ public void NoSystemConsoleReference (AssemblyInfo info)
122122
"vsprintf", "vswprintf", "wcscat", "wcscpy", "wcslen", "wcsncat", "wcsncpy",
123123
"wcstok", "wmemcpy", "wnsprintf", "wnsprintfa", "wnsprintfw", "wscanf", "wsprintf",
124124
"wsprintfa", "wsprintfw", "wvnsprintf", "wvnsprintfa", "wvnsprintfw", "wvsprintf",
125-
"wvsprintfa", "wvsprintfw"
125+
"wvsprintfa", "wvsprintfw",
126+
"malloc", // not banned by binscope, but we want to use calloc instead
127+
// keep up-to-date with the 'bannedCApi' list in tests/mtouch/MiscTests.cs
126128
};
127129

128130
[TestCaseSource (typeof (Helper), nameof (Helper.PlatformAssemblyDefinitions))]

tests/common/Configuration.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1247,9 +1247,9 @@ public static bool CanRunArm64 {
12471247
[DllImport ("libc")]
12481248
static extern int sysctlbyname (string name, ref int value, ref IntPtr size, IntPtr zero, IntPtr zeroAgain);
12491249

1250-
public static IEnumerable<string> GetNativeSymbols (string file, string arch = null)
1250+
public static IEnumerable<string> CallNM (string file, string nmArguments, string arch = null)
12511251
{
1252-
var arguments = new List<string> (new [] { "-gUjA", file });
1252+
var arguments = new List<string> (new [] { nmArguments, file });
12531253
if (!string.IsNullOrEmpty (arch)) {
12541254
arguments.Add ("-arch");
12551255
arguments.Add (arch);
@@ -1264,5 +1264,15 @@ public static IEnumerable<string> GetNativeSymbols (string file, string arch = n
12641264
return v.Substring (idx + 2);
12651265
});
12661266
}
1267+
1268+
public static IEnumerable<string> GetNativeSymbols (string file, string arch = null)
1269+
{
1270+
return CallNM (file, "-gUjA", arch);
1271+
}
1272+
1273+
public static IEnumerable<string> GetUndefinedNativeSymbols (string file, string arch = null)
1274+
{
1275+
return CallNM (file, "-gujA", arch);
1276+
}
12671277
}
12681278
}

tests/mtouch/MiscTests.cs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text;
77
using System.Text.RegularExpressions;
88
using Xamarin.Tests;
9+
using Xamarin.Utils;
910

1011
using NUnit.Framework;
1112

@@ -207,5 +208,66 @@ public void PublicSymbols (Profile profile)
207208

208209
Assert.IsEmpty (failed.ToString (), "Failed libraries");
209210
}
211+
212+
[TestCase (ApplePlatform.iOS)]
213+
[TestCase (ApplePlatform.TVOS)]
214+
[TestCase (ApplePlatform.MacCatalyst)]
215+
[TestCase (ApplePlatform.MacOSX)]
216+
public void NoBannedApi (ApplePlatform platform)
217+
{
218+
Configuration.IgnoreIfIgnoredPlatform (platform);
219+
Configuration.AssertDotNetAvailable ();
220+
221+
// we should not p/invoke into API that are banned (by MS) from the C runtime
222+
// list is copied from binscope for mac (not all of them actually exists on macOS)
223+
var bannedCApi = new HashSet<string> () {
224+
"_alloca", "_ftcscat", "_ftcscpy", "_getts", "_gettws", "_i64toa",
225+
"_i64tow", "_itoa", "_itow", "_makepath", "_mbccat", "_mbccpy",
226+
"_mbscat", "_mbscpy", "_mbslen", "_mbsnbcat", "_mbsnbcpy", "_mbsncat",
227+
"_mbsncpy", "_mbstok", "_mbstrlen", "_snprintf", "_sntprintf",
228+
"_sntscanf", "_snwprintf", "_splitpath", "_stprintf", "_stscanf",
229+
"_tccat", "_tccpy", "_tcscat", "_tcscpy", "_tcsncat", "_tcsncpy",
230+
"_tcstok", "_tmakepath", "_tscanf", "_tsplitpath", "_ui64toa",
231+
"_ui64tot", "_ui64tow", "_ultoa", "_ultot", "_ultow", "_vsnprintf",
232+
"_vsntprintf", "_vsnwprintf", "_vstprintf", "_wmakepath", "_wsplitpath",
233+
"alloca", "changewindowmessagefilter", "chartooem", "chartooema",
234+
"chartooembuffa", "chartooembuffw", "chartooemw", "copymemory", "gets",
235+
"isbadcodeptr", "isbadhugereadptr", "isbadhugewriteptr", "isbadreadptr",
236+
"isbadstringptr", "isbadwriteptr", "lstrcat", "lstrcata", "lstrcatn",
237+
"lstrcatna", "lstrcatnw", "lstrcatw", "lstrcpy", "lstrcpya", "lstrcpyn",
238+
"lstrcpyna", "lstrcpynw", "lstrcpyw", "lstrlen", "lstrncat", "makepath",
239+
/* "memcpy", */ "oemtochar", "oemtochara", "oemtocharw", "rtlcopymemory", "scanf", // memcpy is banned, but we use it a bunch, and so does dotnet/runtime
240+
"snscanf", "snwscanf", "sprintf", "sprintfa", "sprintfw", "sscanf", "strcat",
241+
"strcata", "strcatbuff", "strcatbuffa", "strcatbuffw", "strcatchainw",
242+
"strcatn", "strcatna", "strcatnw", "strcatw", "strcpy", "strcpya", "strcpyn",
243+
"strcpyna", "strcpynw", "strcpyw", /* "strlen" , */ "strncat", "strncata", "strncatw", // strlen is banned, but we use it a bunch, and so does dotnet/runtime
244+
"strncpy", "strncpya", "strncpyw", "strtok", "swprintf", "swscanf", "vsnprintf",
245+
"vsprintf", "vswprintf", "wcscat", "wcscpy", "wcslen", "wcsncat", "wcsncpy",
246+
"wcstok", "wmemcpy", "wnsprintf", "wnsprintfa", "wnsprintfw", "wscanf", "wsprintf",
247+
"wsprintfa", "wsprintfw", "wvnsprintf", "wvnsprintfa", "wvnsprintfw", "wvsprintf",
248+
"wvsprintfa", "wvsprintfw",
249+
"malloc", // not banned by binscope, but we want to use calloc instead
250+
// keep up-to-date with the 'BannedCApi' list in tests/cecil-tests/Test.cs
251+
};
252+
253+
var runtimeIdentifiers = Configuration.GetRuntimeIdentifiers (platform);
254+
var failed = new HashSet<string> ();
255+
foreach (var rid in runtimeIdentifiers) {
256+
var nativedir = Path.Combine (Configuration.GetRuntimeDirectory (platform, rid), "native");
257+
var paths = new HashSet<string> ();
258+
paths.UnionWith (Directory.GetFileSystemEntries (nativedir, "*.a", SearchOption.AllDirectories));
259+
paths.UnionWith (Directory.GetFileSystemEntries (nativedir, "*.dylib", SearchOption.AllDirectories));
260+
261+
foreach (var path in paths) {
262+
var symbols = Configuration.GetUndefinedNativeSymbols (path, arch: "all");
263+
264+
var bannedSymbols = symbols.Intersect (bannedCApi.Select (v => "_" + v));
265+
if (bannedSymbols.Any ())
266+
failed.Add ($"{path}:\n\t{string.Join ("\n\t", bannedSymbols.ToArray ())}");
267+
}
268+
269+
}
270+
Assert.IsEmpty (string.Join ("\n", failed.OrderBy (v => v)), "Libraries referencing banned symbols");
271+
}
210272
}
211273
}

0 commit comments

Comments
 (0)