jonp's picture

x86 ABI assumptions in OpenAL aren't portable to other platforms

OpenTK/Audio/OpenAL/Alc/Alc.cs has embedded assumptions about the ABI it's running on that mirror those of x86. Unfortunately, those assumptions aren't portable.

In this case, the assumption is this: you can treat a structure <= 8 bytes as if it were an integral type. This allows you to transparently treat a pointer return value as a structure return value:

unsafe public static extern ContextHandle CreateContext([In] IntPtr device, [In] int* attrlist);

alcCreateContext() returns an actual pointer, ContextHandle is a struct, and on x86 things Just Work because, as it happens, sizeof(ContextHandle) == sizeof(IntPtr) so x86 registers are used for the return value.

Other platforms aren't as forgiving. Case in point: ARM. ARM only uses registers for integral types & pointers, and NEVER uses registers for returned structures. Structures always use a "pointer indirection" (as x86 does for structures larger than 8 bytes).

Consequently, that CreateContext() call breaks when running under ARM (at least the ARM ABI as used by the iPhone).

The fix is to use respect the ABI and return IntPtr when actual pointers are being returned. (Or use SafeHandles, which are reference types and thus pointers already.)

The following patch allows OpenAL to be used on the iPhone with MonoTouch's OpenTK.dll:

Index: OpenTK/Audio/OpenAL/Alc/Alc.cs
===================================================================
--- OpenTK/Audio/OpenAL/Alc/Alc.cs	(revision 2360)
+++ OpenTK/Audio/OpenAL/Alc/Alc.cs	(working copy)
@@ -89,7 +89,14 @@
         /// <returns>Returns a pointer to the new context (NULL on failure). The attribute list can be NULL, or a zero terminated list of integer pairs composed of valid ALC attribute tokens and requested values.</returns>
         [DllImport(Alc.Lib, EntryPoint = "alcCreateContext", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity]
         [CLSCompliant(false)]
-        unsafe public static extern ContextHandle CreateContext([In] IntPtr device, [In] int* attrlist);
+        unsafe private static extern IntPtr sys_CreateContext([In] IntPtr device, [In] int* attrlist);
+
+        [CLSCompliant(false)]
+        unsafe public static ContextHandle CreateContext([In] IntPtr device, [In] int* attrlist)
+        {
+            return new ContextHandle(sys_CreateContext(device, attrlist));
+        }
+
         // ALC_API ALCcontext *    ALC_APIENTRY alcCreateContext( ALCdevice *device, const ALCint* attrlist );
 
         /// <summary>This function creates a context using a specified device.</summary>
@@ -138,7 +145,12 @@
         /// <summary>This function retrieves the current context.</summary>
         /// <returns>Returns a pointer to the current context.</returns>
         [DllImport(Alc.Lib, EntryPoint = "alcGetCurrentContext", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern ContextHandle GetCurrentContext();
+        private static extern IntPtr sys_GetCurrentContext();
+
+        public static ContextHandle GetCurrentContext()
+        {
+            return new ContextHandle(sys_GetCurrentContext());
+        }
         // ALC_API ALCcontext *    ALC_APIENTRY alcGetCurrentContext( void );
 
         /// <summary>This function retrieves a context's device pointer.</summary>

Comments

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
the Fiddler's picture

Thanks, this patch is committed in rev. 2386.

Does the same issue affect structure parameters? For example:

        [DllImport(Alc.Lib, EntryPoint = "alcMakeContextCurrent", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
        public static extern bool MakeContextCurrent([In] ContextHandle context);
jonp's picture

Yes, this same issue does effect method parameters. Looks like I need to cook up another patch...

jonp's picture

And a patch:

Index: OpenTK/Audio/OpenAL/Alc/Alc.cs
===================================================================
--- OpenTK/Audio/OpenAL/Alc/Alc.cs	(revision 2387)
+++ OpenTK/Audio/OpenAL/Alc/Alc.cs	(working copy)
@@ -120,25 +120,41 @@
         /// <param name="context">A pointer to the new context.</param>
         /// <returns>Returns True on success, or False on failure.</returns>
         [DllImport(Alc.Lib, EntryPoint = "alcMakeContextCurrent", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern bool MakeContextCurrent([In] ContextHandle context);
+        static extern bool MakeContextCurrent(IntPtr context);
+        public static bool MakeContextCurrent(ContextHandle context)
+        {
+            return MakeContextCurrent(context.Handle);
+        }
         // ALC_API ALCboolean      ALC_APIENTRY alcMakeContextCurrent( ALCcontext *context );
 
         /// <summary>This function tells a context to begin processing. When a context is suspended, changes in OpenAL state will be accepted but will not be processed. alcSuspendContext can be used to suspend a context, and then all the OpenAL state changes can be applied at once, followed by a call to alcProcessContext to apply all the state changes immediately. In some cases, this procedure may be more efficient than application of properties in a non-suspended state. In some implementations, process and suspend calls are each a NOP.</summary>
         /// <param name="context">a pointer to the new context</param>
         [DllImport(Alc.Lib, EntryPoint = "alcProcessContext", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern void ProcessContext([In] ContextHandle context);
+        static extern void ProcessContext(IntPtr context);
+        public static void ProcessContext(ContextHandle context)
+        {
+            ProcessContext(context.Handle);
+        }
         // ALC_API void            ALC_APIENTRY alcProcessContext( ALCcontext *context );
 
         /// <summary>This function suspends processing on a specified context. When a context is suspended, changes in OpenAL state will be accepted but will not be processed. A typical use of alcSuspendContext would be to suspend a context, apply all the OpenAL state changes at once, and then call alcProcessContext to apply all the state changes at once. In some cases, this procedure may be more efficient than application of properties in a non-suspended state. In some implementations, process and suspend calls are each a NOP.</summary>
         /// <param name="context">a pointer to the context to be suspended.</param>
         [DllImport(Alc.Lib, EntryPoint = "alcSuspendContext", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern void SuspendContext([In] ContextHandle context);
+        static extern void SuspendContext(IntPtr context);
+        public static void SuspendContext(ContextHandle context)
+        {
+            SuspendContext(context.Handle);
+        }
         // ALC_API void            ALC_APIENTRY alcSuspendContext( ALCcontext *context );
 
         /// <summary>This function destroys a context.</summary>
         /// <param name="context">a pointer to the new context.</param>
         [DllImport(Alc.Lib, EntryPoint = "alcDestroyContext", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern void DestroyContext([In] ContextHandle context);
+        static extern void DestroyContext(IntPtr context);
+        public static void DestroyContext(ContextHandle context)
+        {
+            DestroyContext(context.Handle);
+        }
         // ALC_API void            ALC_APIENTRY alcDestroyContext( ALCcontext *context );
 
         /// <summary>This function retrieves the current context.</summary>
@@ -156,7 +172,11 @@
         /// <param name="context">a pointer to a context.</param>
         /// <returns>Returns a pointer to the specified context's device.</returns>
         [DllImport(Alc.Lib, EntryPoint = "alcGetContextsDevice", ExactSpelling = true, CallingConvention = Alc.Style), SuppressUnmanagedCodeSecurity()]
-        public static extern IntPtr GetContextsDevice([In] ContextHandle context);
+        static extern IntPtr GetContextsDevice(IntPtr context);
+        public static IntPtr GetContextsDevice(ContextHandle context)
+        {
+            return GetContextsDevice(context.Handle);
+        }
         // ALC_API ALCdevice*      ALC_APIENTRY alcGetContextsDevice( ALCcontext *context );
 
         #endregion Context Management

This really only impacts Alc.cs (at least, grep -rl --include=\*.cs 'extern.*ContextHandle' . didn't find any other references to files I'm using in MonoTouch).

the Fiddler's picture

Thanks for the patch. A quick audit didn't reveal any other instances of this issue, but I'll add this to the documentation just in case.

jonp's picture

Will you be committing this soon? I don't see it in r2387.

Thanks.

the Fiddler's picture

r2387 is the version you created the patch against and I lack time-traveling abilities, so... :)

Committed to r2390.