Bug 311941

Summary:	[Qyoto] QVariant values are marshalled incorrectly to virtual calls
Product:	[Unmaintained] bindings	Reporter:	David McFarland <corngood>
Component:	general	Assignee:	bindings-bugs-null
Status:	RESOLVED FIXED
Severity:	normal	CC:	dpldobrev
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed/Implemented In:
Sentry Crash Report:
Attachments:	patch

Description David McFarland 2012-12-19 16:27:46 UTC

When a primitive type QVariant is marshalled to a virtual function call, the typeid is not properly passed to the managed code, so the managed side will always try to read a GCHandle off the stack, rather than the primitive type.  This results in an invalid GCHandle exception, or an unrelated object being passed into the user function. 

Reproducible: Always

Steps to Reproduce:
1. Create a class derived from QAbstractItemModel
2. provide some data, and set ItemFlag.ItemIsUserCheckable
3. override bool SetData(QModelIndex index, object value, int role)
4. hook the model up to a view that will show the checkbox
5. observe the 'value' argument to SetData when the item is checked
Actual Results:  
'value' will either  be an unrelated clr object, or it will throw an exception because of an invalid GCHandle

Expected Results:  
'value' should be a numeric CheckState

Comment 1 David McFarland 2012-12-19 16:31:12 UTC

Created attachment 75930 [details]
patch

Here's my local patch.  I've also put it on:

https://github.com/corngood/assemblygen

It includes a couple of changes to make it easier to debug the binding code.

Comment 2 Dimitar Dobrev 2012-12-22 18:19:10 UTC

Thanks a lot for the patch. It looks good. However, wouldn't the "-debug" compile option cause the produced library to always be in debug mode thus decreasing performance?

Comment 3 David McFarland 2012-12-22 18:50:21 UTC

I don't believe so.  The documentation just says that it will cause an .mdb file to be generated.  Ideally it would match the configuration used in make, but I think turning it on is a better default.  The IMDB files won't be installed.

Comment 4 Dimitar Dobrev 2012-12-22 22:31:29 UTC

Patch applied. Thanks. Sorry for the delay but there seem to be some problems with the e-mail notifications of the KDE bug tracker - I received notice about your bug just a few hours ago. I've only made some changes for the build to work on Windows as well:
codeFiles.Add(arg);	->	codeFiles.Add(arg.Replace('/', Path.DirectorySeparatorChar));
...
compilerOptions.Append(" -debug");	->	compilerOptions.Append(" /debug:pdbonly");
(see http://msdn.microsoft.com/en-us/library/8cw0bt21.aspx)
...
if (codeFile == null)	->	if (string.IsNullOrEmpty(codeFile))