https://bugs.winehq.org/show_bug.cgi?id=48174
Bug ID: 48174 Summary: wine-mono creates incorrect strings Product: Wine Version: 4.20 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: mscoree Assignee: wine-bugs@winehq.org Reporter: galtgendo@o2.pl Distribution: ---
This bug is about Smile Game Builder - a game engine. As a testcase 「虚毒ノ夢」 will be used - a (it seems - I don't actually know Japanese) freeware game available from https://kanawo.wixsite.com/teritoma/kodoku (first link is the download: https://www.freem.ne.jp/dl/win/15104).
Problem 1.
As you might notice, the window title is purely mojibake. CreateWindowEx expects CP_ACP string in cs->lpszName, but it's getting an utf8 one. That's pretty much sums up what I could figure out about this part.
Problem 2.
This one is more tricky to notice; also, I couldn't figure out much about it, so just describing symptoms. Some of the strings printed on the screen (AFAICT neither by gdi, gdiplus nor user32 standard text functions) are read from memory then written on the screen past their actual length. What's more, they flicker - that is if such string is printed, some of the time its extra part changes upon redraw. As far as I understand mono (which, granted, isn't all that much), it would suggest that upon string object creation its length is set incorrectly.
In the example game, you can observe it with a few strings as you start a new game, then once you get control, if you walk to the closet (not the bookcase) and interact with it, it's another such string.
https://bugs.winehq.org/show_bug.cgi?id=48174
Vincent Povirk madewokherd@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |madewokherd@gmail.com
https://bugs.winehq.org/show_bug.cgi?id=48174
Vincent Povirk madewokherd@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://kanawo.wixsite.com/ | |teritoma/kodoku Keywords| |download
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #1 from Vincent Povirk madewokherd@gmail.com --- Note that a Japanese locale is necessary for these characters to be representable in ANSI encoding. Setting a Japanese locale doesn't fix the bug, though, it still passes the string in as utf8.
From a WINE_MONO_TRACE=wrapper WINEDEBUG=win log:
[00000009: 11.38921 0] ENTER:c (wrapper managed-to-native) <Module>:kmyPlatform.createWindow (int,int,*())(960, 544, 02A68B47) 0009:trace:win:alloc_winproc allocated 0xffff0024 for A 0x2a68b47 (37/4096 used) 0009:trace:win:WIN_CreateWindowEx "\xe8\x99\x9a\xe6\xaf\x92\xe3\x83\x8e\xe5\xa4\xa2" L"kmyWGLWindowClass" ex=00000000 style=00cf0000 -2147483648,-2147483648 968x571 parent=0x10020 menu=(nil) inst=(nil) params=(nil)
From what I can tell so far, kmyPlatform is a native dll which does the
CreateWindowExA call. It's being called via a managed wrapper presumably in SharpKmyCore.dll. But we're only passing in x, y, and winproc, at least at the time the window is created. The title may have been passed in incorrectly earlier, or kmyPlatform is getting it from somewhere else.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #2 from Vincent Povirk madewokherd@gmail.com --- Guessing it's this:
[00000009: 11.38838 0] ENTER:c (wrapper managed-to-native) <Module>:kmyPlatform.Platform.setWindowTitle (sbyte modopt(System.Runtime.CompilerServices.IsSignUnspecifiedByte) modopt(System.Runtime.CompilerServices.IsConst)*)(001816E8)
So the string is being converted sometime before that, and we need to figure out how.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #3 from Vincent Povirk madewokherd@gmail.com --- The string being passed to setWindowTitle comes from Marshal.StringToHGlobalAnsi. So presumably that's using the wrong encoding.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #4 from Rafał Mużyło galtgendo@o2.pl ---
Note that a Japanese locale is necessary for these characters to be representable in ANSI encoding.
Note that most Japanese programs I've encountered (which isn't all that high a number actually) need to be run in the Japanese locale to work properly. Some plain refuse to work at all otherwise.
OK, keeping the sarcasm in check from this point on.
As far I understand the Microsoft docs, as StringToHGlobalAnsi doesn't take an encoding as a parameter, it works with CP_ACP. Meaning the String object it's being fed is already wrong (unless, that is, mono implemented that call wrong). I didn't mention it before here (I did on the chat), but the mangled title is a result of the utf8 string with the expected title being converted as if it was in CP932 encoding. I'd suspect a MultiByteToWideChar call at some point, but didn't see it in WINEDEBUG, so an equivalent might be happening somewhere in mono code.
So, comment 3 seems to only move the goalposts - by that call, thing is (most likely) already broken.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #5 from Vincent Povirk madewokherd@gmail.com --- What I mean is that Mono's StringToHGlobalAnsi is using utf8 when it probably should be using CP_ACP (and I believe it does currently use its own code to do the conversion).
I'm just trying to figure out and give a complete explanation (and hopefully fix) what's happening. I'm not necessarily giving you advice or trying to say the program is wrong. Obviously, it works on Windows, so we're almost certainly doing something wrong.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #6 from Vincent Povirk madewokherd@gmail.com --- I believe String objects internally use UTF16, so StringToHGlobalAnsi has to do some sort of conversion.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #7 from Rafał Mużyło galtgendo@o2.pl --- You known, I'm either completely misunderstanding it or I just might have found something:
compare mono/netcore/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Unix.cs and
mono/netcore/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs
Your thoughts ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #8 from Rafał Mużyło galtgendo@o2.pl --- ...then again, I can't find that lib in wine-mono, even though the code needs to be there...
https://bugs.winehq.org/show_bug.cgi?id=48174
Sagawa sagawa.aki+winebugs@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |sagawa.aki+winebugs@gmail.c | |om
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #9 from Vincent Povirk madewokherd@gmail.com --- We don't use netcore in Wine Mono, at least not yet.
https://github.com/madewokherd/mono/blob/master/mcs/class/corlib/System.Runt...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #10 from Rafał Mużyło galtgendo@o2.pl --- in such case, I'm not sure whether mono/metadata/marshal.c or mono/metadata/marshal-windows.c is used, but both are basically mono_utf16_to_utf8...
Could that be problem 1 ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #11 from Rafał Mużyło galtgendo@o2.pl --- Also, for the tests, is there an easy way to make such program use .NET Core ? I've got a copy, that was bundled with another program (a download from github).
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #12 from Vincent Povirk madewokherd@gmail.com --- I have no idea.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #13 from Vincent Povirk madewokherd@gmail.com --- I wrote a patch for Marshal: https://github.com/madewokherd/mono/commit/59163403b04eb48e62df6f163473b7bea...
I'm still working on properly testing this so it can be added to master.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #14 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Vincent Povirk from comment #13)
I wrote a patch for Marshal: https://github.com/madewokherd/mono/commit/ 59163403b04eb48e62df6f163473b7bea2585d2c
Just a thought: won't that break the mono executable ? Not the executables run by mono, but mono itself.
I mean, there might be a reason why mono/netcore/System.Private.CoreLib/shared/System/Runtime/InteropServices/Marshal.Windows.cs and mono/metadata/marshal-windows.c differ as significantly and this seems the most likely one...
That is the bug might likely be still being allowed to build mono without netcore by the build system.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #15 from Vincent Povirk madewokherd@gmail.com --- It's possible that other parts of Mono rely on the Ansi functions using UTF8. If so, they'll just have to be updated to use the UTF8 functions instead. I ran the full test suite with this change and didn't find any cases like that (but it did find some bugs in my code).
I don't plan on committing this to 4.9.x for that reason (and also there's a good chance we won't have any more 4.9.x releases).
I pushed the change to master, since it is passing the tests.
It's also possible that something depends on Marshal Ansi functions working the same as Encoding.Default but doesn't really care what the default encoding is. Encoding.Default in Mono is still UTF8, but on Windows it matches the value of CP_ACP. Still, I don't think that's a likely scenario, and I hope to fix it before wine-mono 5.0 anyway.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #16 from Vincent Povirk madewokherd@gmail.com --- If I understand problem 2 correctly, it looks to me like strings aren't being null-terminated somewhere and it's drawing extra garbage memory from after the end.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #17 from Vincent Povirk madewokherd@gmail.com ---
From what I can tell, it uses Encoding.UTF8.GetBytes() to convert text to a
byte array and passes a pointer to the array to the rendering engine. To my knowledge, this does not and should not null-terminate the string.
Further, it does not pass in a length.
I can't figure out a way to hack GetBytes to actually test this, it's simply too widely-used and needs to work correctly.
It's possible that this is broken on Windows too, just less often. Or maybe for some reason there always happens to be a 0 byte after arrays in .NET.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #18 from Nikolay Sivov bunglehead@gmail.com --- This sounds exactly like issue I was looking at, most likely game was using same engine. Tests on Windows showed that allocated vector has padding, so this null-terminator assumption works in practice.
I posted allocator change for that [1], that was basically rejected.
[1] https://github.com/mono/mono/issues/16988
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #19 from Vincent Povirk madewokherd@gmail.com --- It does appear to be using the same engine.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #20 from Vincent Povirk madewokherd@gmail.com --- I'd be OK with changing this in wine-mono as long as we have tests for it. But since the string is "usually" null-terminated in practice, I'm not sure how to test it effectively.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #21 from Vincent Povirk madewokherd@gmail.com --- I wrote a test program that creates and fills a lot of arrays of different lengths, and it seems to reliably fail on Mono while passing on Windows.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #22 from Vincent Povirk madewokherd@gmail.com --- Nikolay, do you think your patch should be included as-is? It fixes my test case.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #23 from Vincent Povirk madewokherd@gmail.com --- Created attachment 65999 --> https://bugs.winehq.org/attachment.cgi?id=65999 test case for null bytes after the end of arrays
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #24 from Nikolay Sivov bunglehead@gmail.com --- Right, that's basically a test I was using - allocating char vectors of different sizes and dumping just enough to see that tail is reliably zeroed. I think patch should be fine.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #25 from Rafał Mużyło galtgendo@o2.pl --- Will either of you try to raise this with mono upstream again ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #26 from Nikolay Sivov bunglehead@gmail.com --- I won't be doing that, as I don't have any new arguments to push it harder. I can see why it could be considered too much of a corner case, or implementation detail that's not verifiable in a clean way.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #27 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Nikolay Sivov from comment #26)
I won't be doing that, as I don't have any new arguments to push it harder. I can see why it could be considered too much of a corner case, or implementation detail that's not verifiable in a clean way.
...well, on one hand...
Though on the other, their argument of "the fix should happen on the application code" was pretty much bogus in wine context.
Way I see it, both sides arguments are somewhat weak.
Aren't there any additional arguments in the portions of dotnet that were opensourced ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #28 from Rafał Mużyło galtgendo@o2.pl --- ...I might have a proper counterargument - Unity engine: it bundles mono libs. I've found a glitchy one, though I can't tell if this is the reason for the glitch in question. The glitch is an odd one: part of the strings are missing in in-game menus.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #29 from Rafał Mużyło galtgendo@o2.pl --- Correct me if I'm wrong, but going by git log, wine-mono 5.0.0 is supposed to contain this commit, isn't it ?
If so, well, still not fixed...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #30 from Rafał Mużyło galtgendo@o2.pl --- To clarify (as I've somewhat forgotten what this bug was about): problem 1 looks fixed, but problem 2 definitely isn't.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #31 from Esme Povirk madewokherd@gmail.com --- That's what happens when you make one bug for two things?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #32 from Rafał Mużyło galtgendo@o2.pl --- ...oh, well...
Anyhow, on one hand problem 2 seems like something not writing terminating null byte upon conversion, on the other the deeper I try to dive into mono source, the more it looks like all those conversions use mono_utf16_to_utf8 or some derivative, which definitely do write that byte...
So, while the hack from comment 18 does indeed feel wrong, one of the conversion paths must be missing that '+1'...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #33 from Rafał Mużyło galtgendo@o2.pl --- OK, I don't know if this is relevant, but I seen something interesting: in the applied patch, we allocate 'AllocCoTaskMem(mbstr_size + 1)', in dotnet/runtime '(s.Length + 1) * (long)SystemMaxDBCSCharSize' is allocated...
So, if I read it correctly, we allocate one byte less.
No idea if it changes anything.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #34 from Rafał Mużyło galtgendo@o2.pl --- Another somewhat interesting note: there is an English version on the game used as the testcase (though I've only seen 1.00, not 1.01 - probably it's an unofficial translation).
Obviously, it couldn't demonstrate problem 1, so I didn't use it, but even that one shows problem 2.
But the interesting parts is that the corruption there is not only seen on the title screen, but while consistent, it differs between different LANG values.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #35 from Esme Povirk madewokherd@gmail.com --- Looking back at this, I think I intended to commit Nikolay's fix as well, and forgot. I'll try to get it in for the next release.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #36 from Rafał Mużyło galtgendo@o2.pl --- Well, that's always a solution.
Though the part I kind of agree with the upstream is that fiddling with the things at vector level seems wrong, when most likely only code paths relevant to string conversion (and not all of them) are affected...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #37 from Nikolay Sivov bunglehead@gmail.com --- The issue with allocation was straightforward as I remember, and unrelated to conversion, and probably this report.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #38 from Rafał Mużyło galtgendo@o2.pl --- ...
So, I've kept digging in random places and stumbled upon this https://github.com/dotnet/runtime/pull/35190 - as of now still open.
Well, that seems to be the opposite direction, but that alignment change... What are the chances that native does it in other places ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #39 from Nikolay Sivov bunglehead@gmail.com --- I was referring to this one [1]. It's unrelated to string handling, let alone marshaling to and from some native string types; it's purely about assumptions on returned vector, exposed by unsafe access.
[1] https://github.com/mono/mono/issues/16988
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #40 from Rafał Mużyło galtgendo@o2.pl --- ...OK, lets hope I cooled down sufficiently to respond to that rationally...
I actually was thinking about your proposed patch when I've seen that PR.
For example, mono_gc_alloc_vector in metadata/sgen-mono.c already aligns to 8, what if it actually should align to 16 ?
That is, you *unconditionally* add that SIZEOF_VOID_P padding, which will a step later be aligned to 8. What if this problem means that 8 is simply not enough ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #41 from Nikolay Sivov bunglehead@gmail.com --- What happens if requested size is multiple of the size of alignment?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #42 from Rafał Mużyło galtgendo@o2.pl --- ...also, that's likely explained by some part of wine-mono build process, but I feel like asking: why doesn't your patch make the similar change in metadata/boehm-gc.c ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #43 from Rafał Mużyło galtgendo@o2.pl --- The question from comment 40 is mostly driven by that only *some* of the strings are affected by the problem. Maybe that's random, then again, maybe not ?
Though if it was just an alignment problem, the proportion would likely be reversed: there seem to be more unaffected strings than the affected ones...
Hmm...
What could the native be doing there...
...OK, maybe using bugzilla as a blackboard from brain dumps isn't exactly appropriate use.. Then again, hearing other people opinions on my ideas sometimes gives me better ideas.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #44 from Esme Povirk madewokherd@gmail.com --- Boehm is obsolete and we don't use it.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #45 from Rafał Mużyło galtgendo@o2.pl --- ...to continue the line of thought from yesterday: while I'm not sure about the part that modifies the bytecode, the other two blocks, given that the test app seems to be 32bit, are *statistically* non-ops a half of the time, as with the above, sizeof(SIZEOF_VOID_P) will be 4, so on average it will be swallowed with that rate by the 8 bytes alignment.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #46 from Rafał Mużyło galtgendo@o2.pl --- So, first thing: comment 42 is obviously bogus - the first chunk changes boehm - my bad, as I've looked at it, I've missed that I've scrolled that bit off view.
boehm does aligning too, though in arch-dependent way: 8 on 32bit, 16 on 64bit (twice the length of a pointer) - unfortunately, that's kind of hardcoded in the lib (gc_tiny_fl.h).
Still, the test from comment 23 looked promising, up until this bit: "I don't think it breaks until mono reuses memory".
Heisenbugs are nasty...
OK, slightly longer explanation: I got a thing or two wrong; more exactly I've switched the order.
So, to cut the chase, null-gc might not be aligning after all. sgen_alloc_obj_nolock does align, so mono_gc_alloc_vector in metadata/sgen-mono.c falls under this uncertainty.
Somewhat interesting might be that mono_array_new_full_checked does align arrays, but not vectors - then again mono_array_new_specific_internal doesn't align at all...
If not for the fact that I'd not simply need mono, but most likely one built for Windows to compile to Windows executables, I'd likely would give in at this point and just install it to do some testing on my own - no matter how much I dislike the language (well, more the pretense it's cross-platform, when it's not quite so).
Trying to tackle internal implementation details sucks.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #47 from Esme Povirk madewokherd@gmail.com --- I tried adding the GC patch, and I'm not able to complete a build with it. It seems to cause random crashes.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #48 from Rafał Mużyło galtgendo@o2.pl --- I've pretty much forgotten about this bug and am unsure of its current status, but decided to update a minor detail: that upstream PR35190 was turned into PR35804 and merged - not sure if it's relevant.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #49 from Esme Povirk madewokherd@gmail.com --- No idea, I didn't notice any upstream commits that looked relevant. Mono pull requests only go up to 21577 so I don't know what you're referencing there.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #50 from Rafał Mużyło galtgendo@o2.pl --- (In reply to Esme Povirk from comment #49)
Mono pull requests only go up to 21577 so I don't know what you're referencing there.
...https://github.com/dotnet/runtime/pull/35804
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #51 from Esme Povirk madewokherd@gmail.com --- We use mono, not dotnet.
Looks like it was ported over: https://github.com/mono/mono/pull/19701
But, that seems to be for string marshaling only, which should already be working correctly in wine-mono, the remaining problem is not having 0's after arrays.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #52 from Rafał Mużyło galtgendo@o2.pl --- ...
Anyway, leaving a note to myself (just in case it's not clear from the context), that the bug is still valid...
Also, what's EdgeBrowserControl ? On one hand, the portions of traceback suggest it's either a system component or at least a dotnet component, on the other there's next to no decent google results for it.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #53 from Rafał Mużyło galtgendo@o2.pl --- OK, sorry for grasping at straws, but couldn't portions of https://learn.microsoft.com/en-us/dotnet/framework/interop/default-marshalli... be read as that upon calling outside dotnet (as seems to be the case here - Encoding.UTF8.GetBytes call precedes a call to a function that quite obviously expects a null-terminated string) strings are *supposed* to be null-terminated ?
That is, it's not the result of GetBytes, that's supposed to be null-terminated, but its copy sent outside dotnet ?
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #54 from Rafał Mużyło galtgendo@o2.pl --- ,,,OK, first part is completely irrelevant, given that GetBytes produces an array of *bytes*, not characters; yet, marshaling seems the only stage where that those nulls get added on Windows - only thing that happens before the call is 'ref byte byte& = ref text[0];'
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #55 from Esme Povirk madewokherd@gmail.com --- We already have tests showing that byte arrays in .NET Framework have at least one NUL after the end. It's just a matter of getting an implementation of it that actually works.
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #56 from Rafał Mużyło galtgendo@o2.pl --- I wonder how the authors of this engine stumbled upon this implementation detail, as more I google for it, more it seems to not be documented anywhere...
Also, on a tangentially related topic: winedump's c++ demangling kinda sucks (that is seems to fail more often than not)...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #57 from Rafał Mużyło galtgendo@o2.pl --- Kinda related but not really, yet I feel worth mentioning - while googling for material for this bug, the most annoying part was that pretty much all of Stack Overflow like sources seemed to agree that if you want null terminated byte arrays from strings in C#, you need to null terminate them on your own, yet here we are...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #58 from Rafał Mużyło galtgendo@o2.pl --- So, I've stumbled upon something that might, but doesn't have to be related...
https://github.com/atom0s/Steamless/pull/97
The commit referenced there basically just changes a 4 byte char array (UnmanagedType.ByValArray) into UInt32 to make things work.
...
https://bugs.winehq.org/show_bug.cgi?id=48174
--- Comment #59 from Esme Povirk madewokherd@gmail.com --- That's odd, char is a 16-bit type so I'd expect that to be an 8-byte array. For this use case, byte[] would make more sense.