Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[runtime] Remove internal memcpy p/invoke #8890

Merged
merged 2 commits into from
Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/AudioToolbox/AudioConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,8 @@ static AudioConverterError FillComplexBufferShared (IntPtr inAudioConverter, ref
inst.packetDescriptions = Marshal.AllocHGlobal (data.Length * size);
}
unsafe {
fixed (AudioStreamPacketDescription* inptr = data) {
Runtime.memcpy ((byte *) inst.packetDescriptions, (byte *) inptr, data.Length * size);
fixed (void* source = data) {
Buffer.MemoryCopy (source, (void*) inst.packetDescriptions, inst.packetDescriptionSize * size, data.Length * size);
}
}
Marshal.WriteIntPtr (outDataPacketDescription, inst.packetDescriptions);
Expand Down
8 changes: 2 additions & 6 deletions src/AudioToolbox/AudioQueue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,7 @@ public AudioStreamPacketDescription [] PacketDescriptions {

public unsafe void CopyToAudioData (IntPtr source, int size)
{
byte *t = (byte *) AudioData;
byte *s = (byte *) source;
Runtime.memcpy (t, s, size);
Buffer.MemoryCopy ((void*) source, (void*) AudioData, AudioDataByteSize, size);
}
}

Expand Down Expand Up @@ -481,9 +479,7 @@ public static void FillAudioData (IntPtr audioQueueBuffer, int offset, IntPtr so
{
IntPtr target = Marshal.ReadIntPtr (audioQueueBuffer, IntPtr.Size);
unsafe {
byte *targetp = (byte *) target;
byte *sourcep = (byte *) source;
Runtime.memcpy (targetp + offset, sourcep + sourceOffset, size);
Buffer.MemoryCopy ((void*) (source + sourceOffset), (void*) (target + offset), size, size);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/AudioToolbox/MusicTrack.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ internal override IntPtr ToUnmanaged ()
if (data != null)
Marshal.Copy (data, start, (IntPtr) rdata, len);
else
Runtime.memcpy (rdata, (byte *) buffer, len);
Buffer.MemoryCopy ((void*) buffer, (void*) rdata, len, len);
return (IntPtr) target;
}
}
Expand Down Expand Up @@ -166,7 +166,7 @@ internal override IntPtr ToUnmanaged ()
if (data != null)
Marshal.Copy (data, start, (IntPtr) rdata, len);
else
Runtime.memcpy (rdata, (byte *) buffer, len);
Buffer.MemoryCopy ((void*) buffer, (void*) rdata, len, len);
return (IntPtr) target;
}
}
Expand Down
16 changes: 9 additions & 7 deletions src/CoreMidi/MidiServices.cs
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ internal static MidiPacket [] ReadPacketList (IntPtr packetList)
return packets;
}

internal static IntPtr CreatePacketList (MidiPacket [] packets)
internal unsafe static IntPtr CreatePacketList (MidiPacket [] packets)
{
// calculate the total size of the data.
int size = 4;
Expand All @@ -849,16 +849,18 @@ internal static IntPtr CreatePacketList (MidiPacket [] packets)
Marshal.WriteInt32 (buffer, 0, packets.Length);
int dest = 4;
for (int i = 0; i < packets.Length; i++) {
Marshal.WriteInt64 (buffer, dest, packets [i].TimeStamp);
var packet = packets [i];
var packet_size = packet.Length;
Marshal.WriteInt64 (buffer, dest, packet.TimeStamp);
dest += 8;
Marshal.WriteInt16 (buffer, dest, (short) packets [i].Length);
Marshal.WriteInt16 (buffer, dest, (short) packet_size);
dest += 2;
if (packets [i].ByteArray == null) {
Runtime.memcpy (buffer + dest, packets [i].BytePointer, packets [i].Length);
if (packet.ByteArray == null) {
Buffer.MemoryCopy ((void*) packet.BytePointer, (void*) (buffer + dest), packet_size, packet_size);
} else {
Marshal.Copy (packets [i].ByteArray, packets [i].start, buffer + dest, packets [i].Length);
Marshal.Copy (packet.ByteArray, packet.start, buffer + dest, packet_size);
}
dest += GetPacketLength (packets [i].Length) - 10;
dest += GetPacketLength (packet_size) - 10;
}
return buffer;
}
Expand Down
24 changes: 12 additions & 12 deletions src/CoreMidi/MidiThruConnectionParams.cs
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ internal void ReadStruct (NSData data)
Controls = null;
else {
Controls = new MidiControlTransform[connectionParams.NumControlTransforms];
unsafe { // Lets use memcpy to avoid a for loop
fixed (MidiControlTransform* arrAddr = &Controls[0])
Runtime.memcpy ((IntPtr) arrAddr, bufferEnd, controlsSize * connectionParams.NumControlTransforms);
unsafe {
fixed (void* arrAddr = &Controls[0])
Buffer.MemoryCopy ((void*) bufferEnd, arrAddr, controlsSize * connectionParams.NumControlTransforms, controlsSize * connectionParams.NumControlTransforms);
}
}

Expand All @@ -281,11 +281,11 @@ internal void ReadStruct (NSData data)
else {
Maps = new MidiValueMap [connectionParams.NumMaps];
bufferEnd = IntPtr.Add (bufferEnd, controlsSize * connectionParams.NumControlTransforms);
unsafe { // Lets use memcpy to avoid a for loop
unsafe {
for (int i = 0; i < connectionParams.NumMaps; i++) {
Maps [i].Value = new byte [128];
fixed (byte* arrAddr = &Maps[i].Value [0])
Runtime.memcpy ((IntPtr) arrAddr, bufferEnd, 128);
fixed (void* arrAddr = &Maps[i].Value [0])
Buffer.MemoryCopy ((void*) bufferEnd, arrAddr, 128, 128);
}
}
}
Expand Down Expand Up @@ -327,19 +327,19 @@ internal NSData WriteStruct ()
Marshal.StructureToPtr (connectionParams, buffer, false);

if (connectionParams.NumControlTransforms > 0) {
unsafe { // Lets use memcpy to avoid a for loop
fixed (MidiControlTransform* arrAddr = &Controls[0])
Runtime.memcpy (bufferEnd, (IntPtr) arrAddr, controlsSize * connectionParams.NumControlTransforms);
unsafe {
fixed (void* arrAddr = &Controls[0])
Buffer.MemoryCopy (arrAddr, (void*) bufferEnd, controlsSize * connectionParams.NumControlTransforms, controlsSize * connectionParams.NumControlTransforms);
}
}

if (connectionParams.NumMaps > 0) {
// Set the destination buffer after controls arr if any
bufferEnd = IntPtr.Add (bufferEnd, controlsSize * connectionParams.NumControlTransforms);
unsafe { // Lets use memcpy to avoid a for loop
unsafe {
for (int i = 0; i < connectionParams.NumMaps; i++) {
fixed (byte* arrAddr = &Maps[i].Value [0])
Runtime.memcpy (bufferEnd, (IntPtr) arrAddr, 128);
fixed (void* arrAddr = &Maps[i].Value [0])
Buffer.MemoryCopy (arrAddr, (void*) bufferEnd, 128, 128);
bufferEnd += 128;
}
}
Expand Down
6 changes: 0 additions & 6 deletions src/ObjCRuntime/Runtime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,12 +1651,6 @@ internal unsafe static IntPtr CloneMemory (IntPtr source, long length)
return rv;
}

[DllImport (Constants.libSystemLibrary)]
extern internal static void memcpy (IntPtr target, IntPtr source, nint n);

[DllImport (Constants.libSystemLibrary)]
unsafe extern internal static void memcpy (byte * target, byte * source, nint n);

// This function will try to compare a native UTF8 string to a managed string without creating a temporary managed string for the native UTF8 string.
// Currently this only works if the UTF8 string only contains single-byte characters.
// If any multi-byte characters are found, the native utf8 string is converted to a managed string, and then normal managed comparison is done.
Expand Down
52 changes: 51 additions & 1 deletion tests/cecil-tests/Test.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using System.IO;

using NUnit.Framework;
Expand Down Expand Up @@ -103,5 +104,54 @@ public void NoSystemConsoleReference (string assemblyPath)
if (assembly.MainModule.TryGetTypeReference ("System.Console", out var _))
Assert.Fail ($"{assemblyPath} has a reference to `System.Console`. Please use `Runtime.NSLog` inside the platform assemblies");
}

// 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)
static HashSet<string> 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",
"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",
"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"
};

[TestCaseSource (typeof (Helper), "PlatformAssemblies")]
public void NoBannedApi (string assemblyPath)
{
var assembly = Helper.GetAssembly (assemblyPath);
if (assembly == null) {
Assert.Ignore ("{assemblyPath} could not be found (might be disabled in build)");
return; // just to help nullability
}
List<string> found = new List<string> ();
foreach (var m in Helper.FilterMethods (assembly!, (m) => m.IsPInvokeImpl)) {
var symbol = m.PInvokeInfo.EntryPoint;
if (BannedCApi.Contains (symbol))
found.Add (symbol);
}
// if multiple p/invoke are defined then the same symbol will show multiple times
// it's a feature :)
Assert.That (found, Is.Empty, string.Join (", ", found));
}
}
}
}
1 change: 0 additions & 1 deletion tests/xtro-sharpie/common-ObjCRuntime.ignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
!unknown-pinvoke! dlerror bound
!unknown-pinvoke! dlopen bound
!unknown-pinvoke! dlsym bound
!unknown-pinvoke! memcpy bound
!unknown-pinvoke! NXGetLocalArchInfo bound
!unknown-pinvoke! objc_allocateClassPair bound
!unknown-pinvoke! objc_allocateProtocol bound
Expand Down