Skip to content

Commit

Permalink
[runtime] Use calloc instead of malloc. (#20692)
Browse files Browse the repository at this point in the history
It's safer, since the returned memory is zero-initialized.

Also add tests.
  • Loading branch information
rolfbjarne authored Jun 7, 2024
1 parent e9d59d5 commit 34f58bb
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 15 deletions.
8 changes: 4 additions & 4 deletions runtime/launcher.m
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
len++;
}

v = value = (char *) malloc (len + 1);
v = value = (char *) calloc (1, len + 1);
while (start < inptr) {
if (*start == '\\')
start++;
Expand Down Expand Up @@ -162,7 +162,7 @@
break;
}

node = (ListNode *) malloc (sizeof (ListNode));
node = (ListNode *) calloc (1, sizeof (ListNode));
node->value = value;
node->next = NULL;
n++;
Expand All @@ -185,7 +185,7 @@
if (n == 0)
return NULL;

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

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

char **new_argv = (char **) malloc (sizeof (char *) * ((unsigned long) new_argc + 1 /* null terminated */));
char **new_argv = (char **) calloc (sizeof (char *), (unsigned long) new_argc + 1 /* null terminated */);
const char **ptr = (const char **) new_argv;
// binary
*ptr++ = argv [0];
Expand Down
4 changes: 2 additions & 2 deletions runtime/monotouch-debug.m
Original file line number Diff line number Diff line change
Expand Up @@ -932,7 +932,7 @@ static ssize_t sdb_recv (void *buf, size_t len)
return;
}

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

Expand Down Expand Up @@ -1457,7 +1457,7 @@ int monotouch_debug_connect (NSMutableArray *ips, int debug_port, int output_por
return 2;
}

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

Expand Down
2 changes: 1 addition & 1 deletion runtime/runtime.m
Original file line number Diff line number Diff line change
Expand Up @@ -2075,7 +2075,7 @@ -(void) xamarinSetFlags: (enum XamarinGCHandleFlags) flags;
return INVALID_GCHANDLE;
// PRINT ("New value: %x", (int) res);

nmp = (MethodAndPar *) malloc (sizeof (MethodAndPar));
nmp = (MethodAndPar *) calloc (1, sizeof (MethodAndPar));
*nmp = mp;

MONO_ENTER_GC_SAFE;
Expand Down
2 changes: 1 addition & 1 deletion runtime/slinked-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ typedef struct SList SList;
static inline SList *
s_list_prepend (SList *list, void *value)
{
SList *first = (SList *) malloc (sizeof (SList));
SList *first = (SList *) calloc (1, sizeof (SList));
first->next = list;
first->data = value;
return first;
Expand Down
4 changes: 2 additions & 2 deletions runtime/trampolines.m
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@
if (gc_handle == INVALID_GCHANDLE) {
CFDictionaryRemoveValue (gchandle_hash, self);
} else {
struct gchandle_dictionary_entry *entry = (struct gchandle_dictionary_entry *) malloc (sizeof (struct gchandle_dictionary_entry));
struct gchandle_dictionary_entry *entry = (struct gchandle_dictionary_entry *) calloc (1, sizeof (struct gchandle_dictionary_entry));
entry->gc_handle = gc_handle;
entry->flags = flags;
CFDictionarySetValue (gchandle_hash, self, entry);
Expand Down Expand Up @@ -1813,7 +1813,7 @@
if (length == 0)
return [NSArray array];

buf = (id *) malloc (sizeof (id) * length);
buf = (id *) calloc (sizeof (id), length);

#if !defined (CORECLR_RUNTIME)
MonoClass *object_class = mono_object_get_class ((MonoObject *) array);
Expand Down
4 changes: 2 additions & 2 deletions runtime/xamarin-support.m
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
}
NSData *data = [tz data];
*size = (uint32_t) [data length];
void* result = malloc (*size);
void* result = calloc (1, *size);
[data getBytes: result length: *size];
return result;
}
Expand All @@ -111,7 +111,7 @@
// COOP: no managed memory access: any mode.
NSArray *array = [NSTimeZone knownTimeZoneNames];
*count = (uint32_t) array.count;
char** result = (char**) malloc (sizeof (char*) * (*count));
char** result = (char**) calloc (sizeof (char*), *count);
for (unsigned long i = 0; i < *count; i++) {
NSString *s = [array objectAtIndex: i];
result [i] = strdup (s.UTF8String);
Expand Down
4 changes: 3 additions & 1 deletion tests/cecil-tests/Test.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ public void NoSystemConsoleReference (AssemblyInfo info)
"vsprintf", "vswprintf", "wcscat", "wcscpy", "wcslen", "wcsncat", "wcsncpy",
"wcstok", "wmemcpy", "wnsprintf", "wnsprintfa", "wnsprintfw", "wscanf", "wsprintf",
"wsprintfa", "wsprintfw", "wvnsprintf", "wvnsprintfa", "wvnsprintfw", "wvsprintf",
"wvsprintfa", "wvsprintfw"
"wvsprintfa", "wvsprintfw",
"malloc", // not banned by binscope, but we want to use calloc instead
// keep up-to-date with the 'bannedCApi' list in tests/mtouch/MiscTests.cs
};

[TestCaseSource (typeof (Helper), nameof (Helper.PlatformAssemblyDefinitions))]
Expand Down
14 changes: 12 additions & 2 deletions tests/common/Configuration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1247,9 +1247,9 @@ public static bool CanRunArm64 {
[DllImport ("libc")]
static extern int sysctlbyname (string name, ref int value, ref IntPtr size, IntPtr zero, IntPtr zeroAgain);

public static IEnumerable<string> GetNativeSymbols (string file, string arch = null)
public static IEnumerable<string> CallNM (string file, string nmArguments, string arch = null)
{
var arguments = new List<string> (new [] { "-gUjA", file });
var arguments = new List<string> (new [] { nmArguments, file });
if (!string.IsNullOrEmpty (arch)) {
arguments.Add ("-arch");
arguments.Add (arch);
Expand All @@ -1264,5 +1264,15 @@ public static IEnumerable<string> GetNativeSymbols (string file, string arch = n
return v.Substring (idx + 2);
});
}

public static IEnumerable<string> GetNativeSymbols (string file, string arch = null)
{
return CallNM (file, "-gUjA", arch);
}

public static IEnumerable<string> GetUndefinedNativeSymbols (string file, string arch = null)
{
return CallNM (file, "-gujA", arch);
}
}
}
62 changes: 62 additions & 0 deletions tests/mtouch/MiscTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Text;
using System.Text.RegularExpressions;
using Xamarin.Tests;
using Xamarin.Utils;

using NUnit.Framework;

Expand Down Expand Up @@ -207,5 +208,66 @@ public void PublicSymbols (Profile profile)

Assert.IsEmpty (failed.ToString (), "Failed libraries");
}

[TestCase (ApplePlatform.iOS)]
[TestCase (ApplePlatform.TVOS)]
[TestCase (ApplePlatform.MacCatalyst)]
[TestCase (ApplePlatform.MacOSX)]
public void NoBannedApi (ApplePlatform platform)
{
Configuration.IgnoreIfIgnoredPlatform (platform);
Configuration.AssertDotNetAvailable ();

// we should not p/invoke into API that are banned (by MS) from the C runtime
// list is copied from binscope for mac (not all of them actually exists on macOS)
var bannedCApi = new HashSet<string> () {
"_alloca", "_ftcscat", "_ftcscpy", "_getts", "_gettws", "_i64toa",
"_i64tow", "_itoa", "_itow", "_makepath", "_mbccat", "_mbccpy",
"_mbscat", "_mbscpy", "_mbslen", "_mbsnbcat", "_mbsnbcpy", "_mbsncat",
"_mbsncpy", "_mbstok", "_mbstrlen", "_snprintf", "_sntprintf",
"_sntscanf", "_snwprintf", "_splitpath", "_stprintf", "_stscanf",
"_tccat", "_tccpy", "_tcscat", "_tcscpy", "_tcsncat", "_tcsncpy",
"_tcstok", "_tmakepath", "_tscanf", "_tsplitpath", "_ui64toa",
"_ui64tot", "_ui64tow", "_ultoa", "_ultot", "_ultow", "_vsnprintf",
"_vsntprintf", "_vsnwprintf", "_vstprintf", "_wmakepath", "_wsplitpath",
"alloca", "changewindowmessagefilter", "chartooem", "chartooema",
"chartooembuffa", "chartooembuffw", "chartooemw", "copymemory", "gets",
"isbadcodeptr", "isbadhugereadptr", "isbadhugewriteptr", "isbadreadptr",
"isbadstringptr", "isbadwriteptr", "lstrcat", "lstrcata", "lstrcatn",
"lstrcatna", "lstrcatnw", "lstrcatw", "lstrcpy", "lstrcpya", "lstrcpyn",
"lstrcpyna", "lstrcpynw", "lstrcpyw", "lstrlen", "lstrncat", "makepath",
/* "memcpy", */ "oemtochar", "oemtochara", "oemtocharw", "rtlcopymemory", "scanf", // memcpy is banned, but we use it a bunch, and so does dotnet/runtime
"snscanf", "snwscanf", "sprintf", "sprintfa", "sprintfw", "sscanf", "strcat",
"strcata", "strcatbuff", "strcatbuffa", "strcatbuffw", "strcatchainw",
"strcatn", "strcatna", "strcatnw", "strcatw", "strcpy", "strcpya", "strcpyn",
"strcpyna", "strcpynw", "strcpyw", /* "strlen" , */ "strncat", "strncata", "strncatw", // strlen is banned, but we use it a bunch, and so does dotnet/runtime
"strncpy", "strncpya", "strncpyw", "strtok", "swprintf", "swscanf", "vsnprintf",
"vsprintf", "vswprintf", "wcscat", "wcscpy", "wcslen", "wcsncat", "wcsncpy",
"wcstok", "wmemcpy", "wnsprintf", "wnsprintfa", "wnsprintfw", "wscanf", "wsprintf",
"wsprintfa", "wsprintfw", "wvnsprintf", "wvnsprintfa", "wvnsprintfw", "wvsprintf",
"wvsprintfa", "wvsprintfw",
"malloc", // not banned by binscope, but we want to use calloc instead
// keep up-to-date with the 'BannedCApi' list in tests/cecil-tests/Test.cs
};

var runtimeIdentifiers = Configuration.GetRuntimeIdentifiers (platform);
var failed = new HashSet<string> ();
foreach (var rid in runtimeIdentifiers) {
var nativedir = Path.Combine (Configuration.GetRuntimeDirectory (platform, rid), "native");
var paths = new HashSet<string> ();
paths.UnionWith (Directory.GetFileSystemEntries (nativedir, "*.a", SearchOption.AllDirectories));
paths.UnionWith (Directory.GetFileSystemEntries (nativedir, "*.dylib", SearchOption.AllDirectories));

foreach (var path in paths) {
var symbols = Configuration.GetUndefinedNativeSymbols (path, arch: "all");

var bannedSymbols = symbols.Intersect (bannedCApi.Select (v => "_" + v));
if (bannedSymbols.Any ())
failed.Add ($"{path}:\n\t{string.Join ("\n\t", bannedSymbols.ToArray ())}");
}

}
Assert.IsEmpty (string.Join ("\n", failed.OrderBy (v => v)), "Libraries referencing banned symbols");
}
}
}

8 comments on commit 34f58bb

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Please sign in to comment.