---- Reported by vladimir@pobox.com 2004-05-23 18:22:53 MST ----

Given two code files, t37.c and t37.cs:

=== t37.cs ===
#include <stdio.h>
 
typedef struct {
    int i;
    int j;
} SomeStruct;
 
typedef void (*SomeStructCB) (SomeStruct *ss);
 
void
doit (SomeStructCB sscb)
{
    SomeStruct ss;
    ss.i = ss.j = 0;
    printf ("unmanaged &ss: 0x%08x\n", &ss);
    sscb (&ss);
    printf ("unmanaged ss.i: %d ss.j: %d\n", ss.i, ss.j);
}
=== end ===

=== t37.cs ===
using System;
using System.Runtime.InteropServices;
 
public class Driver {
    [StructLayout(LayoutKind.Sequential)]
    public struct SomeStruct {
        public int i;
        public int j;
    }
 
 
    [DllImport("t37.so")]
    static extern void doit (cbdelegate smcb);
 
#if true
    delegate void cbdelegate (ref SomeStruct ss);
    public static void sscb (ref SomeStruct ss) {
        unsafe {
            int *p = (int *) &ss;
            Console.WriteLine ("p = 0x{0}, *p = {1}", ((int)
p).ToString("x"), *p);
        }
        ss.i = 10;
        ss.j = 20;
    }
#else
    delegate void cbdelegate (IntPtr sptr);
    public static void sscb (IntPtr sptr) {
        Console.WriteLine ("sptr: {0}", ((int) sptr).ToString("x"));
    }
#endif
 
    public static void Main () {
        doit (new cbdelegate(sscb));
    }
}
=== end ===

If the code is built as is, with the #if true, the output is:
unmanaged &ss: 0xf68d583c
p = 0xf68d580c, *p = 0
unmanaged ss.i: 0 ss.j: 0

Note that the value for the unmanaged &ss is different from p (which is &ss
in managed code -- technically, that should only be allowed within a
fixed(), but using fixed() gives the same results).  Also the setting of
i/j has no effect on the struct passed in.

If the #if true is changed to #if false (where just an IntPtr is received
by the delegate), the resulting addresses match up:

&ss: 0xf68b783c
sptr: f68b783c

Is this a bug in unmanaged -> managed struct ref marshalling?



---- Additional Comments From vladimir@pobox.com 2004-05-23 20:17:04 MST ----

Just tried this with the .NET runtime on win32 -- in both cases the
pointer is the same, so this is a mono bug.



---- Additional Comments From vladimir@pobox.com 2004-05-23 21:26:56 MST ----

Attached is a patch; System.ValueType didn't have blittable set, and
all structs inherited its lack of blittable-ness.



---- Additional Comments From vladimir@pobox.com 2004-05-23 21:36:16 MST ----

Created an attachment (id=166082)
blittable.patch




---- Additional Comments From vladimir@pobox.com 2004-05-23 21:55:40 MST ----

Created an attachment (id=166083)
test case addition for mono/tests




---- Additional Comments From vladimir@pobox.com 2004-05-23 22:31:52 MST ----

Updated patch -- the dummy types created for pointers (i.e. void *)
were not marked as blittable, causing System.IntPtr to be
non-blittable.  Also, ->name of these types could probably be
something more descriptive than "System", but I didn't make that change.



---- Additional Comments From vladimir@pobox.com 2004-05-23 22:44:26 MST ----

Created an attachment (id=166084)
updated blittable patch




---- Additional Comments From vargaz@freemail.hu 2004-05-25 18:32:16 MST ----

The patch looks good to me, but I think it could wait until beta2 is 
out, since it is not really a bug fix, rather an optimization.



---- Additional Comments From vargaz@freemail.hu 2004-05-26 08:16:18 MST ----

There seems to be some bugs in our calculation of blittableness which
surface after this patch is applied. For example, tests/pinvoke2.exe
now fails.



---- Additional Comments From vladimir@pobox.com 2004-05-26 16:36:39 MST ----

Hmm, I'll take a look at the failing test.

It is a bug though; if you look at the original output from the sample
code, even though the i,j fields are modified in the struct in the
managed delegate, the unmanaged code never gets those modifications (i
and j == 0 after return from invoking the managed delegate).  The
runtime never copies the contents back, I assume because it's treating
the ref SomeStruct as [In] only?  (I assume that ref should imply
In/Out; otherwise, you have different behaviour depending on whether
your ref argument is blittable or not.)



---- Additional Comments From vladimir@pobox.com 2004-05-26 21:36:26 MST ----

This is related to struct layout alignment; according to
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpref/html/frlrfsystemruntimeinteropservicesstructlayoutattributeclasspacktopic.asp
managed types have a default alignment of 8, "except for unmanaged
structures that typically have a default packing size of 4."  I'm not
sure how their runtime decides what an "unmanaged structure" is. 
We're trying to blit types that have a managed alignment of 8 to
unmanaged blocks of memory with a different alignment.  There's two
fixes that I can think of -- one is to simply check if instance_size
is different from mono_class_native_size(), and if so, set blittable
to false.  This means that we can't blit within the managed side of
things either.  The other fix is to introduce a new native_blittable
flag within klass->marshal_info, and use that when deciding whether we
can blit to/from unmanaged data types.  I think the latter is a better
fix.




---- Additional Comments From vladimir@pobox.com 2004-05-28 05:22:49 MST ----

New patch attached.  All previous tests pass; I added a new test to
pinvoke2.cs which currently fails -- this is the case that's also
failing now, byref non-blittable structs (so yes, this patch doesn't
fix the underlying problem, but it hides it in a lot of cases!). 
That's next on my to-do list...  setenv.exe also fails, but I believe
that's because getenv() is prototyped to return a string, so the
static data gets freed.



---- Additional Comments From vladimir@pobox.com 2004-05-28 05:23:15 MST ----

Created an attachment (id=166085)
updated blittable patch (4)




---- Additional Comments From vargaz@freemail.hu 2004-05-28 14:42:00 MST ----

The struct marshalling problem should be fixed now. The patch looks 
good, but the new tests are probably not needed since I added similar
ones already. If they are needed, they should go into pinvoke3.cs,
since pinvoke2 - managed->native, pinvoke3 - native->managed.





---- Additional Comments From vargaz@freemail.hu 2004-05-28 15:39:51 MST ----

The patch is in (except the tests). Thanks!

Imported an attachment (id=166082)
Imported an attachment (id=166083)
Imported an attachment (id=166084)
Imported an attachment (id=166085)

Unknown operating system unknown. Setting to default OS "Other".