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

IFELanguage.GetJMorphResult() return broken MORRSLT struct #571

Closed
manju-summoner opened this issue Jul 12, 2021 · 8 comments
Closed

IFELanguage.GetJMorphResult() return broken MORRSLT struct #571

manju-summoner opened this issue Jul 12, 2021 · 8 comments
Assignees
Labels
broken api An API is inaccurate and could lead to runtime failure

Comments

@manju-summoner
Copy link

Calling IFELanguage.GetJMorphResult() returns a corrupted MORRSLT because the type of MORRSLT.BLKBuff is not specified correctly.
Replacing the type of MORRSLT.BLKBuff with void* instead of char will return correct results.

[StructLayout(LayoutKind.Sequential, Pack = 1)]
internal partial struct MORRSLT
{
	internal uint dwSize;
	internal win32.Foundation.PWSTR pwchOutput;
	internal ushort cchOutput;
	internal win32.Globalization.MORRSLT._Anonymous1_e__Union Anonymous1;
	internal win32.Globalization.MORRSLT._Anonymous2_e__Union Anonymous2;
	internal unsafe ushort* pchInputPos;
	internal unsafe ushort* pchOutputIdxWDD;
	internal win32.Globalization.MORRSLT._Anonymous3_e__Union Anonymous3;
	internal unsafe ushort* paMonoRubyPos;
	internal unsafe win32.Globalization.WDD* pWDD;
	internal int cWDD;
	internal unsafe void* pPrivate;
	//internal char BLKBuff;      //generated code
	internal unsafe void* BLKBuff;//fixed

Broken MORRSLT:
image

Correct MORRSLT:
image

Sample code:
https://github.com/manju-summoner/CsWin32GetJMorphResultTest/tree/master/CsWin32GetJMorphResultTest

Execution result:
image

@mikebattista mikebattista added the broken api An API is inaccurate and could lead to runtime failure label Aug 6, 2021
@mikebattista
Copy link
Contributor

@AArnott this field is typed as char in the metadata. Is this a C#/Win32 issue?

From the shared code snippet, it looks like you explicitly remap it to void*.

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Aug 6, 2021

In <msime.h>, it is a flexible array member WCHAR BLKBuff[]; so the void* replacement isn't right either. GetJMorphResult allocates this structure so there is no risk of the caller allocating too little memory. I don't understand what goes wrong in the generated code, though.

@AArnott
Copy link
Member

AArnott commented Aug 17, 2021

this field is typed as char in the metadata. Is this a C#/Win32 issue?

@mikebattista the OP's point is that char is wrong. So if that's what's in the metadata, then the metadata is wrong. See this from the OP:

	//internal char BLKBuff;      //generated code
	internal unsafe void* BLKBuff;//fixed

Looking more at the header definition, it's not clear what this should be. I agree void* isn't right either. I think it's a variable-length, inline character array. I can't remember how the metadata should be describing it.

@AArnott AArnott removed their assignment Aug 17, 2021
@mikebattista
Copy link
Contributor

Looking more at the header definition, it's not clear what this should be. I agree void* isn't right either. I think it's a variable-length, inline character array. I can't remember how the metadata should be describing it.

Ok. Thanks. @sotteson1 can you take a look?

@sotteson1
Copy link
Contributor

sotteson1 commented Aug 24, 2021

It should be char[0..0], which we're using to imply a variable-length array. I'll fix it.

@manju-summoner
Copy link
Author

manju-summoner commented Sep 12, 2021

I tried CsWin32 0.1.560-beta using win32metadata 10.2.163-preview but it still broken.

This seems to be a matter of char marshalling, not BLKBuff type.
If I set CharSet=CharSet.Unicode or CharSet=CharSet.Auto in StructLayout, it works fine.

[StructLayout(LayoutKind.Sequential, Pack = 1)]
internal partial struct MORRSLT
{
	/* ... */
	internal __char_1 BLKBuff;

	[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] // <- 
	internal struct __char_1
	{

As well, in the previous implementation, specifying CharSet=CharSet.Unicode worked fine.

[StructLayout(LayoutKind.Sequential, Pack = 1, CharSet = CharSet.Unicode)] // <- 
internal partial struct MORRSLT
{
	/* ... */
	char BLKBuff;
}

@AArnott
Copy link
Member

AArnott commented Sep 13, 2021

@manju-summoner This isn't expected to work in CsWin32 until microsoft/CsWin32#387 is fixed.

@manju-summoner
Copy link
Author

I understand. I'll wait for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broken api An API is inaccurate and could lead to runtime failure
Projects
None yet
Development

No branches or pull requests

5 participants