Bugzilla – Bug 321829
[PATCH] Marshal.StringToHGlobalAuto & PtrToStringAuto not working as expected
Last modified: 2007-09-15 21:24:46 UTC
---- Reported by gert.driesen@pandora.be 2006-08-20 05:22:39 MST ---- Mono's StringToHGlobalAuto and PtrToStringAuto methods of the Marshal class do not work correctly when interacting with the win32 API. To reproduce: 1. Extract the attached zip file. 2. Compile test.cs. 3. Run test.exe (on Windows). Expected result: The following text is output on the console: "389 389 Windows saved user drieseng registry while an application or service was still using the registry during log off. The memory used by the user's registry has not been freed. The registry will be unloaded when it is no longer in use. This is often caused by services running as a user account, try configuring the services to run in either the LocalService or NetworkService account." Note: the first number is the number of bytes returned by the FormatMessage API, while the second number is the length of the string after converting the pointer to a managed string using Marshal.PtrToStringAuto. Actual result: 403 1 W ---- Additional Comments From gert.driesen@pandora.be 2006-08-20 05:28:08 MST ---- Created an attachment (id=170278) Zip file containing repro. ---- Additional Comments From gert.driesen@pandora.be 2006-08-20 05:32:24 MST ---- The previous repro throw exceptions when the results did not match what I expect them to be (as I use it like that as part of my private test set). I'll upload a new repro that just outputs the values as shown in the description of the bug report. ---- Additional Comments From gert.driesen@pandora.be 2006-08-20 05:33:03 MST ---- Created an attachment (id=170279) Updated zip file containing repro ---- Additional Comments From robertj@gmx.net 2006-08-21 12:26:25 MST ---- They are working. The problem seems to be either the detection of Ansi/Unicode or an Ansi/Unicode mismatch in the p/invoke declaration of the external method. ---- Additional Comments From robertj@gmx.net 2006-08-21 13:46:26 MST ---- *** https://bugzilla.novell.com/show_bug.cgi?id=MONO79062 has been marked as a duplicate of this bug. *** ---- Additional Comments From robertj@gmx.net 2006-08-21 13:52:31 MST ---- The problem is DllImport.CharSet = CharSet.Auto. If I run the sample with MONO_LOG_LEVEL="debug" MONO_LOG_MASK="dll" mono test.exe I can see how "FormatMessageA" is looked up. That's wrong. It should be "FormatMessageW", because I'm running XP. Marshal.SystemDefaultCharSize is 2, which is correct. ---- Additional Comments From robertj@gmx.net 2006-08-21 14:09:23 MST ---- Sorry, I've misunterstood the dump output. It was LoadLibraryA, not FormatMessageA. FormatMessageW is looked up correctly :-) The problem is the following icall: {"PtrToStringAuto(intptr)", ves_icall_System_Runtime_InteropServices_Marshal_PtrToStringAnsi}, {"PtrToStringAuto(intptr,int)", ves_icall_System_Runtime_InteropServices_Marshal_PtrToStringAnsi_len}, As you see, "Auto" gets mapped to "Ansi". Under Windows this is wrong. It should be mapped to a function which depending on Marshal.SystemDefaultCharSize it calls either *Ansi or *Unicode. ---- Additional Comments From robertj@gmx.net 2006-08-21 15:26:20 MST ---- For performance reasons the patch uses compile time conditionals. Is this okay style-wise? ---- Additional Comments From robertj@gmx.net 2006-08-21 15:26:47 MST ---- Created an attachment (id=170280) icall.diff ---- Additional Comments From gert.driesen@pandora.be 2006-08-23 08:39:23 MST ---- Robert, some observations/questions: - SystemDefaultCharSize is hardcoded to 2, that doesn't seem to be right. - Are unix systems always ANSI (since all Marshal.***Auto methods are mapped to Ansi icalls) ? - If SystemDefaultCharSize would return the correct results, then wouldn't it be better modify the Marshal class to invoke the right icall depending on SystemDefaultCharSize instead of using compile time conditionals ? - mono_lookup_pinvoke_call in loader.c seems to use a wrong mechanism for looking up the entrypoint in the unmanaged dll. If exactspelling is false on the DllImport (PINVOKE_ATTRIBUTE_NO_MANGLE is not set), then it only searches for the mangled name instead of first searching for the unmangled name. Also, when the charset is set to PINVOKE_ATTRIBUTE_CHAR_SET_AUTO, it should use the character size of the system (1 or 2) to determine whether to look for the ANSI ("A") or Unicode ("W") function. Right now, the unicode function is always used on WIN32, and the ANSI one on all other platforms. More information on this topic is available here: http://msdn2.microsoft.com/en-us/library/7b93s42f.aspx - Marshal.StringToHGlobalAuto appears to be broken in the same way as PtrToStringAuto is. My initial repro demonstrates this as I get the following output after I applied your patch: 395 395 Windows saved user ?????????♥???ï registry while an application or service was ...... Updating your patch for StringToHGlobalAuto fixes this problem, but a better solution would probably be to fix SystemDefaultCharSize. Can you confirm my observations ? What do you propose ? Should I split this bug report into separate ones for: - SystemDefaultCharSize - mono_lookup_pinvoke_call - PtrToString*/SStringToHGlobal* Gert ---- Additional Comments From gert.driesen@pandora.be 2006-08-23 08:57:53 MST ---- Small correction, mono_lookup_pinvoke_call only uses the mangled name first if charset is PINVOKE_ATTRIBUTE_CHAR_SET_AUTO and the platform is WIN32. This is still wrong of course. ---- Additional Comments From robertj@gmx.net 2006-08-23 11:20:36 MST ---- Gert, if you'd look at the attached patch, you'll see that it already addresses both PtrToString*/StringToHGlobal* :-) SystemDefaultCharSize is a constant by now. Maybe something like that: Index: System.Runtime.InteropServices/Marshal.cs =================================================================== --- System.Runtime.InteropServices/Marshal.cs (revision 61596) +++ System.Runtime.InteropServices/Marshal.cs (working copy) @@ -54,6 +54,11 @@ public static readonly int SystemMaxDBCSCharSize = 2; // don't know what this is public static readonly int SystemDefaultCharSize = 2; + static Marshal () + { + SystemDefaultCharSize = Environment.OSVersion.Platform == PlatformID.Win32NT ? 2 : 1; + } + #if !NET_2_0 private Marshal () {} #endif > Also, when the charset is set to > PINVOKE_ATTRIBUTE_CHAR_SET_AUTO, it should use the character size of > the system (1 or 2) to determine whether to look for the ANSI ("A") > or Unicode ("W") function. Right now, the unicode function is always > used on WIN32, and the ANSI one on all other platforms. This is not a bug, because Mono doesn't run on Win9x/ME, and, if it would do, the unicode emulation layer must be used anyway, which leads to a SystemDefaultCharSize = 2 again. BTW, all patches assume that Mono doesn't run on Win9x/ME. ---- Additional Comments From gert.driesen@pandora.be 2006-08-23 11:26:21 MST ---- You're right about the patch of course, sorry about ... About your patch: we could still remove the ****Auto icalls, and modify the Marshal class to invoke ***Ansi or ***Unicode from ***Auto managed methods depending on SystemDefaultCharSize, but that just depends on the personal preference of the maintainer. ---- Additional Comments From robertj@gmx.net 2006-08-23 11:37:22 MST ---- About the mangling: CharSet.Ansi, ExactSpelling = true -> FormatMessageA CharSet.Unicode, ExactSpelling = true -> FormatMessageW CharSet.Auto, ExactSpelling = true -> FormatMessageW The latter doesn't make much sense, but it works, though. I don't see a bug here. ---- Additional Comments From gert.driesen@pandora.be 2006-08-23 12:30:40 MST ---- Robert, I thought the mangling issue was there when exactspelling was false (not true). You probably just made a typo, as the results you get match those you get when ExactSpelling is false. With ExactSpelling set to true you would have gotten an EntryPointNotFoundException in all three cases (on both Mono and MS.NET). You would not have seen the "problem" I described when pinvoking FormatMessage, as there's no plain FormatMessage entrypoint (it has only Ansi and Unicode versions). However, after looking close at the MS docs I noticed that there was indeed a difference between name mangling for Unicode and Ansi which matches the Mono behavior .... For Ansi, it should first search for the unmangled name, while for Unicode it should first probe for the mangled name ... Sorry for the bogus report ... ---- Additional Comments From robertj@gmx.net 2006-08-23 14:36:07 MST ---- > Robert, I thought the mangling issue was there when exactspelling was > false (not true). You probably just made a typo, as the results you > get match those you get when ExactSpelling is false. With > ExactSpelling set to true you would have gotten an > EntryPointNotFoundException in all three cases (on both Mono and > MS.NET). Oops, of course I changed the signature (*A, *W) in the C# code. Otherwise "ExactSpelling = true" won't make sense. Indeed, "ExactSpelling = false" behaves like you wrote. ---- Additional Comments From gert.driesen@pandora.be 2006-08-26 11:12:52 MST ---- *** https://bugzilla.novell.com/show_bug.cgi?id=MONO79116 has been marked as a duplicate of this bug. *** ---- Additional Comments From vargaz@gmail.com 2006-08-31 08:44:09 MST ---- About the patch: I would prefer adding Auto versions of the icalls, and putting the #ifdef PLATFORM_WIN32 into the body of the icalls. ---- Additional Comments From gert.driesen@pandora.be 2006-08-31 08:54:20 MST ---- Zoltan, Using SystemDefaultCharSize (1 or 2) in managed code to decide whether to call the Unicode or Ansi versions is not an option ? That way, you can avoid #ifdef PLATFORM_WIN32 altogether, and remove the *Auto icalls. ---- Additional Comments From robertj@gmx.net 2006-08-31 10:42:42 MST ---- If we can afford the additional "if", yes. It think it's ok, because these are methods which handle more then one char at a time. Zoltan, is it ok? ---- Additional Comments From vargaz@gmail.com 2006-08-31 12:39:11 MST ---- I guess its ok. ---- Additional Comments From robertj@gmx.net 2006-08-31 15:22:54 MST ---- Created an attachment (id=170281) icall.diff ---- Additional Comments From robertj@gmx.net 2006-08-31 15:23:22 MST ---- Created an attachment (id=170282) marshal.diff ---- Additional Comments From vargaz@gmail.com 2006-08-31 16:02:04 MST ---- This looks ok to check in. ---- Additional Comments From robertj@gmx.net 2006-08-31 16:57:10 MST ---- Fixed in SVN. Imported an attachment (id=170278) Imported an attachment (id=170279) Imported an attachment (id=170280) Imported an attachment (id=170281) Imported an attachment (id=170282) Unknown operating system unknown. Setting to default OS "Other".