[Mono-devel-list] [PATCH?] Bugfix for AMD64 stopper plus some general thoughts...

Willibald Krenn Willibald.Krenn at gmx.at
Sat Jan 29 19:13:19 EST 2005


Ok, before anything else: If this has already been fixed, then please 
ignore this email.

Some while ago, I've already said in an email that I had to patch the 
RReader in RemotingConfiguration.cs to get things working here. At that 
time I did not investigate the problem, but just provided a patch that 
'fixed' the bug. Today, after I switched to my new mono-svn account (rw 
now - yey!) I stumbled into mentioned problem again and decided to see 
if I could fix it.

The problem is specific to AMD64 and is demonstrated by following code 
in RemotingConfiguration.cs:

String xml;
int pos = 0;
'loop' {
	try {
		return (int) xml[pos++];
	catch {
		return -1;
} until -1 is returned

I consider this piece of code worth to be contributed to the dustbin of 
history and I will back up my position with a few arguments:
  -> First, it's bad practice to rely on exceptions being thrown for 
events that are perfectly normal and - worse - even expected! Not only 
because exceptions are expensive.
  -> Second, this (and similar) code is not worth to live in the corlib, 
as it will fail and/or lead to security issues when bounds checking is 
not working properly
  -> Third, given that there exist switches in Mini that completely turn 
off bounds checking at all, the code has not the faintest right to live.

As already indicated, the AMD64 version of mini has a bug that prevents 
certain bounds checkes from working correctly. Given the asm below

     0x0000002a966ee2e7:     mov    0xffffffffffffffb0(%rbp),%rcx
     0x0000002a966ee2eb:     movslq 0xffffffffffffffac(%rbp),%rax
     0x0000002a966ee2ef:     cmp    %rax,0x10(%rcx)
     0x0000002a966ee2f3:     jbe    0x2a966ee395

- which actually is a String bounds-check  (rcx is ptr to MonoString) - 
you might already see the problem. It's the compare instruction:

(gdb) info registers rcx
rcx            0x779fc0	7839680
(gdb) x/1xg 0x779fd0
0x779fd0:	0x006500480000000b

Got it?
Yes, the length of a MonoString is a guint32 but Mini compares two 
guint64.. So the result is dependand on the next guint32 in memory (== 
AFAIK the string values).

The easiest fix I can provide is attached and does nothing but force any 
OP_X86_COMPARE_MEMBASE_REG to work with 32 bits only. Two reasons why I 
think this is ok:
  -> First, the opcode seems to be used for the bounds-check only. 
(Correct me, if I'm wrong) And all arrays seem to be limited to 2^32 
entries at max.
  -> Second, the X86 in the name supports the 32-bits only approach. If 
a 64 bits compare is needed, it could be named 

So the one-liner patch is attached to this email; Before going further 
and commiting it, I'd like to have some feedback regarding this..

Thanks & have a nice weekend,

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: quickdiff
Url: http://lists.ximian.com/pipermail/mono-devel-list/attachments/20050130/2998e188/attachment.pl 

More information about the Mono-devel-list mailing list