qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Patch to fix Mac OS X compilation
@ 2005-02-12 20:47 Jonas Maebe
  2005-02-13 13:30 ` Daniel Egger
  2005-02-13 14:30 ` Jonas Maebe
  0 siblings, 2 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-12 20:47 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 350 bytes --]

Hello,

I've attached a patch to fix Mac OS X compilation (and some small 
cleanup of the Mach-O code in dyngen.c as well). Unfortunately, qemu 
"crashes" (i.e., reports a fatal error) almost immediately when trying 
to boot freedos, so it seems there are more errors. Hopefully, someone 
else can fix those now that compilation works again.


Jonas

[-- Attachment #2: qemu-macosx.patch --]
[-- Type: application/octet-stream, Size: 1409 bytes --]

Index: dyngen.c
===================================================================
RCS file: /cvsroot/qemu/qemu/dyngen.c,v
retrieving revision 1.37
diff -r1.37 dyngen.c
999c999
< 		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset & 0xffff);
---
> 		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset |= (other_half << 16);
1001c1001
< 		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
---
> 		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset |= ((uint16_t)(other_half & 0xffff));
1003c1003
< 		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
---
> 		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset += ((int16_t)(other_half & 0xffff));
1149d1148
<         const char *name;
1152d1150
<         name = find_str_by_index(sym->n_un.n_strx);
1155c1153
< 		if ( sym->n_type & N_STAB ) /* Debug symbols are skipped */
---
> 		if ( syment->n_type & N_STAB ) /* Debug symbols are skipped */
1840c1838
< 					if(!sym_name)
---
> 					if(!sym_name && (type != PPC_RELOC_BR24))
Index: osdep.c
===================================================================
RCS file: /cvsroot/qemu/qemu/osdep.c,v
retrieving revision 1.7
diff -r1.7 osdep.c
276c276
< #include <malloc.h>
---
> #include <stdlib.h>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-12 20:47 [Qemu-devel] Patch to fix Mac OS X compilation Jonas Maebe
@ 2005-02-13 13:30 ` Daniel Egger
  2005-02-13 14:36   ` Jonas Maebe
  2005-02-13 14:30 ` Jonas Maebe
  1 sibling, 1 reply; 13+ messages in thread
From: Daniel Egger @ 2005-02-13 13:30 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On 12.02.2005, at 21:47, Jonas Maebe wrote:

> I've attached a patch to fix Mac OS X compilation (and some small 
> cleanup of the Mach-O code in dyngen.c as well). Unfortunately, qemu 
> "crashes" (i.e., reports a fatal error) almost immediately when trying 
> to boot freedos, so it seems there are more errors. Hopefully, someone 
> else can fix those now that compilation works again.

Thanks for doing this.

It's pretty strange that the ABI has changed and such something
along this lines is necessary at all. The breakage seems to stem
from the fact that dyngen still gets the addresses wrong from the
instruction stream somewhere. I had a look into that but I
couldn't find anything obviously wrong in the first 3 or 4 hours.
Unfortunately I'm busy as hell so I cannot donate any more time
to this in the moment.

Servus,
       Daniel

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 478 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-12 20:47 [Qemu-devel] Patch to fix Mac OS X compilation Jonas Maebe
  2005-02-13 13:30 ` Daniel Egger
@ 2005-02-13 14:30 ` Jonas Maebe
  2005-02-19 20:00   ` Jonas Maebe
  1 sibling, 1 reply; 13+ messages in thread
From: Jonas Maebe @ 2005-02-13 14:30 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1387 bytes --]


On 12 feb 2005, at 21:47, Jonas Maebe wrote:

> I've attached a patch to fix Mac OS X compilation (and some small 
> cleanup of the Mach-O code in dyngen.c as well). Unfortunately, qemu 
> "crashes" (i.e., reports a fatal error) almost immediately when trying 
> to boot freedos, so it seems there are more errors. Hopefully, someone 
> else can fix those now that compilation works again.

Here's a better patch, there were still some errors in my fixes to the 
calculation of the relocations.

Now, qemu crashes with an "invalid instruction" error. The reason is 
that the __op_gen_label stuff does not get properly relocated. This is 
caused by the fact that for some reason the relocation of branches is 
done differently under Mac OS X and Linux: under Mac OS X, the original 
symbol is used in the calculations, while under Linux the relocated one 
is used (and the reason for that seems to be that looking up the 
relocated symbol of branch instructions often gives wrong results in 
the current Mach-O code).

It is possible to make a special case for __op_gen_label so that it 
does get replaced by the proper gen_labels[] expression, but even then 
I still get crashes (either because the branch points to a wrong 
address, or because the gen_labels array is not initialised properly).

I won't be working further on this in the near future, since I won't 
have time.


Jonas

[-- Attachment #2: qemu-macosx2.patch --]
[-- Type: application/octet-stream, Size: 2087 bytes --]

Index: dyngen.c
===================================================================
RCS file: /cvsroot/qemu/qemu/dyngen.c,v
retrieving revision 1.37
diff -u -r1.37 dyngen.c
--- dyngen.c	23 Jan 2005 20:42:06 -0000	1.37
+++ dyngen.c	13 Feb 2005 14:00:02 -0000
@@ -996,11 +996,11 @@
 
 	switch(rel->r_type)
 	{
-		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset & 0xffff);
+		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset |= (other_half << 16);
 			break;
-		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) | (uint16_t)(other_half & 0xffff);
 			break;
-		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) + (int16_t)(other_half & 0xffff);
 			break;
 		case PPC_RELOC_BR24:
 			sectoffset = ( *(uint32_t *)(text + rel->r_address) & 0x03fffffc );
@@ -1146,13 +1146,11 @@
 	
 	/* Now transform the symtab, to an extended version, with the sym size, and the C name */
 	for(i = 0, sym = symtab, syment = symtab_std; i < nb_syms; i++, sym++, syment++) {
-        const char *name;
         struct nlist *sym_follow, *sym_next = 0;
         unsigned int j;
-        name = find_str_by_index(sym->n_un.n_strx);
 		memset(sym, 0, sizeof(*sym));
 		
-		if ( sym->n_type & N_STAB ) /* Debug symbols are skipped */
+		if ( syment->n_type & N_STAB ) /* Debug symbols are skipped */
             continue;
 			
 		memcpy(sym, syment, sizeof(*syment));
Index: osdep.c
===================================================================
RCS file: /cvsroot/qemu/qemu/osdep.c,v
retrieving revision 1.7
diff -u -r1.7 osdep.c
--- osdep.c	10 Feb 2005 21:59:15 -0000	1.7
+++ osdep.c	13 Feb 2005 14:00:02 -0000
@@ -273,7 +273,7 @@
 
 #else
 
-#include <malloc.h>
+#include <stdlib.h>
 
 int qemu_write(int fd, const void *buf, size_t n)
 {

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-13 13:30 ` Daniel Egger
@ 2005-02-13 14:36   ` Jonas Maebe
  0 siblings, 0 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-13 14:36 UTC (permalink / raw)
  To: qemu-devel


On 13 feb 2005, at 14:30, Daniel Egger wrote:

> It's pretty strange that the ABI has changed and such something
> along this lines is necessary at all.

The reason is that the original code only took into account the lower 
16 bits of the addresses. Apparently, the generated op.o has grown so 
that there are now also addresses of which the higher bits are needed.

> The breakage seems to stem
> from the fact that dyngen still gets the addresses wrong from the
> instruction stream somewhere.

That is fixed in the new version of the patch I just sent, but now the 
branch relocation code needs to be fixed because of the gen_labels 
code.

> I had a look into that but I
> couldn't find anything obviously wrong in the first 3 or 4 hours.
> Unfortunately I'm busy as hell so I cannot donate any more time
> to this in the moment.

Same here...


Jonas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-13 14:30 ` Jonas Maebe
@ 2005-02-19 20:00   ` Jonas Maebe
  2005-02-20 14:29     ` Jonas Maebe
  0 siblings, 1 reply; 13+ messages in thread
From: Jonas Maebe @ 2005-02-19 20:00 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]


On 13 feb 2005, at 15:30, Jonas Maebe wrote:

> Here's a better patch, there were still some errors in my fixes to the 
> calculation of the relocations.

And again a more improved patch (I think). This one includes a special 
case/hack for Mac OS X for __op_gen_label. Now qemu doesn't crash 
anymore when trying to boot freedos, but it doesn't show anything 
either. I just get a black screen. The same goes for reactos, so I 
guess it's some endless loop while emulating the bios or so.


Jonas

PS: this patch also contains a (temporary) check to make sure the 
displacement of the generated jumps for the __op_gen_label stuff isn't 
 > 24 bits, but I haven't seen it getting triggered yet.

[-- Attachment #2: qemu-macosx3.patch --]
[-- Type: application/octet-stream, Size: 3657 bytes --]

Index: dyngen.c
===================================================================
RCS file: /cvsroot/qemu/qemu/dyngen.c,v
retrieving revision 1.37
diff -u -r1.37 dyngen.c
--- dyngen.c	23 Jan 2005 20:42:06 -0000	1.37
+++ dyngen.c	19 Feb 2005 19:54:26 -0000
@@ -996,11 +996,11 @@
 
 	switch(rel->r_type)
 	{
-		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset & 0xffff);
+		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset |= (other_half << 16);
 			break;
-		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) | (uint16_t)(other_half & 0xffff);
 			break;
-		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) + (int16_t)(other_half & 0xffff);
 			break;
 		case PPC_RELOC_BR24:
 			sectoffset = ( *(uint32_t *)(text + rel->r_address) & 0x03fffffc );
@@ -1146,13 +1146,11 @@
 	
 	/* Now transform the symtab, to an extended version, with the sym size, and the C name */
 	for(i = 0, sym = symtab, syment = symtab_std; i < nb_syms; i++, sym++, syment++) {
-        const char *name;
         struct nlist *sym_follow, *sym_next = 0;
         unsigned int j;
-        name = find_str_by_index(sym->n_un.n_strx);
 		memset(sym, 0, sizeof(*sym));
 		
-		if ( sym->n_type & N_STAB ) /* Debug symbols are skipped */
+		if ( syment->n_type & N_STAB ) /* Debug symbols are skipped */
             continue;
 			
 		memcpy(sym, syment, sizeof(*syment));
@@ -1848,11 +1846,17 @@
                                                        sym_name);
 					switch(type) {
 					case PPC_RELOC_BR24:
-						fprintf(outfile, "{\n");
-						fprintf(outfile, "    uint32_t imm = *(uint32_t *)(gen_code_ptr + %d) & 0x3fffffc;\n", slide);
-						fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | ((imm + ((long)%s - (long)gen_code_ptr) + %d) & 0x03fffffc);\n", 
+					    if (!strstart(sym_name,"__op_gen_label",&p)) {
+    						fprintf(outfile, "{\n");
+    						fprintf(outfile, "    uint32_t imm = *(uint32_t *)(gen_code_ptr + %d) & 0x3fffffc;\n", slide);
+    						fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | ((imm + ((long)%s - (long)gen_code_ptr) + %d) & 0x03fffffc);\n", 
 											slide, slide, name, sslide );
-						fprintf(outfile, "}\n");
+    						fprintf(outfile, "}\n");
+    					} else {
+							fprintf(outfile, " if ( ((((long)%s - (long)gen_code_ptr) + %d) < -(1<<24)) && ((((long)%s - (long)gen_code_ptr) + %d) > ((1<<24) - 1))) { printf(\"relocation > 24 bits\\n\"); }",final_sym_name,sslide,final_sym_name,sslide);
+							fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | ((((long)%s - (long)gen_code_ptr) + %d) & 0x03fffffc);\n",
+											slide, slide, final_sym_name, sslide );
+    					}
 						break;
 					case PPC_RELOC_HI16:
 						fprintf(outfile, "    *(uint16_t *)(gen_code_ptr + %d + 2) = (%s + %d) >> 16;\n", 
Index: osdep.c
===================================================================
RCS file: /cvsroot/qemu/qemu/osdep.c,v
retrieving revision 1.7
diff -u -r1.7 osdep.c
--- osdep.c	10 Feb 2005 21:59:15 -0000	1.7
+++ osdep.c	19 Feb 2005 19:54:26 -0000
@@ -273,7 +273,7 @@
 
 #else
 
-#include <malloc.h>
+#include <stdlib.h>
 
 int qemu_write(int fd, const void *buf, size_t n)
 {

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-19 20:00   ` Jonas Maebe
@ 2005-02-20 14:29     ` Jonas Maebe
  2005-02-21 18:58       ` Fabrice Bellard
  2005-02-21 20:27       ` Fabrice Bellard
  0 siblings, 2 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-20 14:29 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]


On 19 feb 2005, at 21:00, Jonas Maebe wrote:

> And again a more improved patch (I think). This one includes a special 
> case/hack for Mac OS X for __op_gen_label. Now qemu doesn't crash 
> anymore when trying to boot freedos, but it doesn't show anything 
> either. I just get a black screen. The same goes for reactos, so I 
> guess it's some endless loop while emulating the bios or so.

Here's the semi-final fix (stupid bug in my previous patch). FreeDos 
can now boot, so long as you do not load any drivers. The reason is 
that the emulation of the loope instruction goes wrong, and that 
instruction is used in himem.sys (but does not occur when booting 
without it). The problem is that this is the assembler code generated 
by gcc for op_loopzw:

0x74370 <op_loopzw>:    mflr    r0
0x74374 <op_loopzw+4>:  stw     r0,8(r1)
0x74378 <op_loopzw+8>:  stwu    r1,-64(r1)
0x7437c <op_loopzw+12>: lwz     r0,48(r27)
0x74380 <op_loopzw+16>: rlwinm  r0,r0,3,0,28
0x74384 <op_loopzw+20>: lis     r2,12
0x74388 <op_loopzw+24>: lwz     r2,-16636(r2)
0x7438c <op_loopzw+28>: lwzx    r12,r2,r0
0x74390 <op_loopzw+32>: mtctr   r12
0x74394 <op_loopzw+36>: bctrl
0x74398 <op_loopzw+40>: lhz     r0,6(r27)
0x7439c <op_loopzw+44>: cmpwi   cr7,r0,0
0x743a0 <op_loopzw+48>: beq-    cr7,0x743b0 <op_loopzw+64>
0x743a4 <op_loopzw+52>: andi.   r0,r3,64
0x743a8 <op_loopzw+56>: beq-    0x743b0 <op_loopzw+64>
0x743ac <op_loopzw+60>: b       0x296eb0 <__op_gen_label1>
0x743b0 <op_loopzw+64>: lwz     r0,72(r1)
0x743b4 <op_loopzw+68>: addi    r1,r1,64
0x743b8 <op_loopzw+72>: mtlr    r0


At address 0x743ac, the branch to __op_gen_label1 gets patched so it 
points to the basic block coming after the loopzw. In that case, LR 
does not get restored (it was destroyed at 0x74394 by the bctrl), so 
the next blr which gets executed returns to 0x74398. This causes an 
endless loop, obviously.

This does not seem to be Mac OS X-specific at all, and should also 
happen under Linux/ppc I think.


Jonas

[-- Attachment #2: qemu-macosx4.patch --]
[-- Type: application/octet-stream, Size: 3420 bytes --]

Index: dyngen.c
===================================================================
RCS file: /cvsroot/qemu/qemu/dyngen.c,v
retrieving revision 1.37
diff -u -r1.37 dyngen.c
--- dyngen.c	23 Jan 2005 20:42:06 -0000	1.37
+++ dyngen.c	20 Feb 2005 14:12:01 -0000
@@ -996,11 +996,11 @@
 
 	switch(rel->r_type)
 	{
-		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset & 0xffff);
+		case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, &other_half); sectoffset |= (other_half << 16);
 			break;
-		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) | (uint16_t)(other_half & 0xffff);
 			break;
-		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (other_half & 0xffff);
+		case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, &other_half); sectoffset = (sectoffset << 16) + (int16_t)(other_half & 0xffff);
 			break;
 		case PPC_RELOC_BR24:
 			sectoffset = ( *(uint32_t *)(text + rel->r_address) & 0x03fffffc );
@@ -1146,13 +1146,11 @@
 	
 	/* Now transform the symtab, to an extended version, with the sym size, and the C name */
 	for(i = 0, sym = symtab, syment = symtab_std; i < nb_syms; i++, sym++, syment++) {
-        const char *name;
         struct nlist *sym_follow, *sym_next = 0;
         unsigned int j;
-        name = find_str_by_index(sym->n_un.n_strx);
 		memset(sym, 0, sizeof(*sym));
 		
-		if ( sym->n_type & N_STAB ) /* Debug symbols are skipped */
+		if ( syment->n_type & N_STAB ) /* Debug symbols are skipped */
             continue;
 			
 		memcpy(sym, syment, sizeof(*syment));
@@ -1848,11 +1846,16 @@
                                                        sym_name);
 					switch(type) {
 					case PPC_RELOC_BR24:
-						fprintf(outfile, "{\n");
-						fprintf(outfile, "    uint32_t imm = *(uint32_t *)(gen_code_ptr + %d) & 0x3fffffc;\n", slide);
-						fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | ((imm + ((long)%s - (long)gen_code_ptr) + %d) & 0x03fffffc);\n", 
+					    if (!strstart(sym_name,"__op_gen_label",&p)) {
+    						fprintf(outfile, "{\n");
+    						fprintf(outfile, "    uint32_t imm = *(uint32_t *)(gen_code_ptr + %d) & 0x3fffffc;\n", slide);
+    						fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | ((imm + ((long)%s - (long)gen_code_ptr) + %d) & 0x03fffffc);\n", 
 											slide, slide, name, sslide );
-						fprintf(outfile, "}\n");
+    						fprintf(outfile, "}\n");
+    					} else {
+							fprintf(outfile, "    *(uint32_t *)(gen_code_ptr + %d) = (*(uint32_t *)(gen_code_ptr + %d) & ~0x03fffffc) | (((long)%s - (long)gen_code_ptr - %d) & 0x03fffffc);\n",
+											slide, slide, final_sym_name, slide);
+    					}
 						break;
 					case PPC_RELOC_HI16:
 						fprintf(outfile, "    *(uint16_t *)(gen_code_ptr + %d + 2) = (%s + %d) >> 16;\n", 
Index: osdep.c
===================================================================
RCS file: /cvsroot/qemu/qemu/osdep.c,v
retrieving revision 1.7
diff -u -r1.7 osdep.c
--- osdep.c	10 Feb 2005 21:59:15 -0000	1.7
+++ osdep.c	20 Feb 2005 14:12:01 -0000
@@ -273,7 +273,7 @@
 
 #else
 
-#include <malloc.h>
+#include <stdlib.h>
 
 int qemu_write(int fd, const void *buf, size_t n)
 {

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-20 14:29     ` Jonas Maebe
@ 2005-02-21 18:58       ` Fabrice Bellard
  2005-02-21 19:36         ` Jonas Maebe
  2005-02-21 20:27       ` Fabrice Bellard
  1 sibling, 1 reply; 13+ messages in thread
From: Fabrice Bellard @ 2005-02-21 18:58 UTC (permalink / raw)
  To: qemu-devel

Jonas Maebe wrote:
> 
> On 19 feb 2005, at 21:00, Jonas Maebe wrote:
> 
>> And again a more improved patch (I think). This one includes a special 
>> case/hack for Mac OS X for __op_gen_label. Now qemu doesn't crash 
>> anymore when trying to boot freedos, but it doesn't show anything 
>> either. I just get a black screen. The same goes for reactos, so I 
>> guess it's some endless loop while emulating the bios or so.
> 
> 
> Here's the semi-final fix (stupid bug in my previous patch). FreeDos can 
> now boot, so long as you do not load any drivers. The reason is that the 
> emulation of the loope instruction goes wrong, and that instruction is 
> used in himem.sys (but does not occur when booting without it). The 
> problem is that this is the assembler code generated by gcc for op_loopzw:
> 
> 0x74370 <op_loopzw>:    mflr    r0
> 0x74374 <op_loopzw+4>:  stw     r0,8(r1)
> 0x74378 <op_loopzw+8>:  stwu    r1,-64(r1)
> 0x7437c <op_loopzw+12>: lwz     r0,48(r27)
> 0x74380 <op_loopzw+16>: rlwinm  r0,r0,3,0,28
> 0x74384 <op_loopzw+20>: lis     r2,12
> 0x74388 <op_loopzw+24>: lwz     r2,-16636(r2)
> 0x7438c <op_loopzw+28>: lwzx    r12,r2,r0
> 0x74390 <op_loopzw+32>: mtctr   r12
> 0x74394 <op_loopzw+36>: bctrl
> 0x74398 <op_loopzw+40>: lhz     r0,6(r27)
> 0x7439c <op_loopzw+44>: cmpwi   cr7,r0,0
> 0x743a0 <op_loopzw+48>: beq-    cr7,0x743b0 <op_loopzw+64>
> 0x743a4 <op_loopzw+52>: andi.   r0,r3,64
> 0x743a8 <op_loopzw+56>: beq-    0x743b0 <op_loopzw+64>
> 0x743ac <op_loopzw+60>: b       0x296eb0 <__op_gen_label1>
> 0x743b0 <op_loopzw+64>: lwz     r0,72(r1)
> 0x743b4 <op_loopzw+68>: addi    r1,r1,64
> 0x743b8 <op_loopzw+72>: mtlr    r0
> 
> 
> At address 0x743ac, the branch to __op_gen_label1 gets patched so it 
> points to the basic block coming after the loopzw. In that case, LR does 
> not get restored (it was destroyed at 0x74394 by the bctrl), so the next 
> blr which gets executed returns to 0x74398. This causes an endless loop, 
> obviously.
> 
> This does not seem to be Mac OS X-specific at all, and should also 
> happen under Linux/ppc I think.

I do not see in your patch any fix for the PARAMn bug. The symptom is:

translate.c:x: error: too many arguments to function `gen_op_xxx'
translate.c:x: warning: initialization from incompatible pointer type

I am using gcc 3.3. The problem is related to the fact that 
'__op_paramN' is no longer exported in the symbol table.

Fabrice.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-21 18:58       ` Fabrice Bellard
@ 2005-02-21 19:36         ` Jonas Maebe
  2005-02-21 19:43           ` Jonas Maebe
  2005-02-21 20:08           ` Fabrice Bellard
  0 siblings, 2 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-21 19:36 UTC (permalink / raw)
  To: qemu-devel


On 21 feb 2005, at 19:58, Fabrice Bellard wrote:

> I do not see in your patch any fix for the PARAMn bug. The symptom is:
>
> translate.c:x: error: too many arguments to function `gen_op_xxx'
> translate.c:x: warning: initialization from incompatible pointer type

That is fixed. At least, I can compile Qemu with that patch under Mac 
OS X (and run the result, as long as I don't execute any loope/loopne 
instructions).

> I am using gcc 3.3. The problem is related to the fact that 
> '__op_paramN' is no longer exported in the symbol table.

No, the problem was that the wrong values were being searched for in 
the symbol table (as a result of more symbols appearing in it, causing 
offsets to get larger). As long as you keep __op_paramN declared as 
static when __APPLE__ is defined (dyngen-exec.h:201), then they are in 
the regular symbol table (as opposed to in the dynamic symbol table, 
which is currently not parsed by Qemu) and will be found.

The problem was that the relocation addresses weren't properly 
calculated. As a result, any relocation with an address >= 0x8000 could 
not be found in the symbol table (because only the lower signed 16 bits 
of the address were used, dyngen was searching for the wrong addresses 
in the symbol table)

The fixes to solve that problem are this:

-               case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset = (sectoffset & 0xffff);
+               case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset |= (other_half << 16);
                         break;
-               case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset = (other_half & 0xffff);
+               case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset = (sectoffset << 16) | (uint16_t)(other_half & 
0xffff);
                         break;
-               case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset = (other_half & 0xffff);
+               case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, 
&other_half); sectoffset = (sectoffset << 16) + (int16_t)(other_half & 
0xffff)

The original code only stored the lower 16 bits in sectoffset, the new 
code merges in the upper 16 bits as well (or, in the first case, 
doesn't overwrite them).

The removal of the "name" variable and related code a bit further on is 
merely because it isn't actually used.

This change:

-               if ( sym->n_type & N_STAB ) /* Debug symbols are 
skipped */
+               if ( syment->n_type & N_STAB ) /* Debug symbols are 
skipped */

is because at that point sym is not yet initialised, so the "if" would 
always fail and no debug symbols would be skipped.

The final change below the PPC_RELOC_BR24 switch statement is because 
the original implementors of the mach-o support wanted to keep using 
the call-stubs after relocating. The call stubs are quite similar to 
PLT entries under Linux, and I suppose the reason they want to keep 
using them is to prevent problems with the 32MB displacement limit of 
PPC branches.

For this reason, they disregard the result of the relocation lookup for 
branches (since those don't point to any stubs, but to fully resolved 
symbols) and simply change the offset in the copied branch so it keeps 
pointing at the stub inside the op.o object file (an optimization to 
try could be to check whether the final symbol lies within 32MB of the 
copy of the code and if so, use a direct branch instead of one via a 
stub).

Anyway, in case of the __op_gen_label stuff, the branch does have to 
changed so it uses gen_table[], so I added a strstart() check to 
account for this special situation.


Jonas

PS: symbols which are still in the dynamic symbol table and therefore 
not properly relocated under Mac OS X (also not in Qemu 0.61) are 
rclw_table, rclb_table and parity_table. The hack to declare them as 
"static" does not currently work for them, because then you get 
definition conflicts. A proper fix would be to add support for parsing 
the dynamic symbol table as well.

A hack would be to move some things to different include files so the 
"static" definition is the only one that appears when compiling op.o, 
and the "extern" definitions when compiling other files. Another 
solution is to parse the output of "otool -IV op.o", as these 
variables/symbols are shown by that one in the contents of the 
"(__DATA,__nl_symbol_ptr)" section.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-21 19:36         ` Jonas Maebe
@ 2005-02-21 19:43           ` Jonas Maebe
  2005-02-21 20:08           ` Fabrice Bellard
  1 sibling, 0 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-21 19:43 UTC (permalink / raw)
  To: qemu-devel


On 21 feb 2005, at 20:36, Jonas Maebe wrote:

> That is fixed. At least, I can compile Qemu with that patch under Mac 
> OS X

That is: Mac OS X 10.3.8 with XCode 1.2 (and associated cctools, i.e. 
gcc "3.3 20030304 (Apple Computer, Inc. build 1640)") installed.


Jonas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-21 19:36         ` Jonas Maebe
  2005-02-21 19:43           ` Jonas Maebe
@ 2005-02-21 20:08           ` Fabrice Bellard
  2005-02-21 20:29             ` Jonas Maebe
  1 sibling, 1 reply; 13+ messages in thread
From: Fabrice Bellard @ 2005-02-21 20:08 UTC (permalink / raw)
  To: qemu-devel

Jonas Maebe wrote:
> 
> On 21 feb 2005, at 19:58, Fabrice Bellard wrote:
> 
>> I do not see in your patch any fix for the PARAMn bug. The symptom is:
>>
>> translate.c:x: error: too many arguments to function `gen_op_xxx'
>> translate.c:x: warning: initialization from incompatible pointer type
> 
> 
> That is fixed. At least, I can compile Qemu with that patch under Mac OS 
> X (and run the result, as long as I don't execute any loope/loopne 
> instructions).
> 
>> I am using gcc 3.3. The problem is related to the fact that 
>> '__op_paramN' is no longer exported in the symbol table.
> 
> 
> No, the problem was that the wrong values were being searched for in the 
> symbol table (as a result of more symbols appearing in it, causing 
> offsets to get larger). As long as you keep __op_paramN declared as 
> static when __APPLE__ is defined (dyngen-exec.h:201), then they are in 
> the regular symbol table (as opposed to in the dynamic symbol table, 
> which is currently not parsed by Qemu) and will be found.
> 
> The problem was that the relocation addresses weren't properly 
> calculated. As a result, any relocation with an address >= 0x8000 could 
> not be found in the symbol table (because only the lower signed 16 bits 
> of the address were used, dyngen was searching for the wrong addresses 
> in the symbol table)
> 
> The fixes to solve that problem are this:
> 
> -               case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset = (sectoffset & 0xffff);
> +               case PPC_RELOC_LO16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset |= (other_half << 16);
>                         break;
> -               case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset = (other_half & 0xffff);
> +               case PPC_RELOC_HI16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset = (sectoffset << 16) | (uint16_t)(other_half & 
> 0xffff);
>                         break;
> -               case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset = (other_half & 0xffff);
> +               case PPC_RELOC_HA16: fetch_next_pair_value(rel+1, 
> &other_half); sectoffset = (sectoffset << 16) + (int16_t)(other_half & 
> 0xffff)
> 
> The original code only stored the lower 16 bits in sectoffset, the new 
> code merges in the upper 16 bits as well (or, in the first case, doesn't 
> overwrite them).
> 
> The removal of the "name" variable and related code a bit further on is 
> merely because it isn't actually used.
> 
> This change:
> 
> -               if ( sym->n_type & N_STAB ) /* Debug symbols are skipped */
> +               if ( syment->n_type & N_STAB ) /* Debug symbols are 
> skipped */
> 
> is because at that point sym is not yet initialised, so the "if" would 
> always fail and no debug symbols would be skipped.
> 
> The final change below the PPC_RELOC_BR24 switch statement is because 
> the original implementors of the mach-o support wanted to keep using the 
> call-stubs after relocating. The call stubs are quite similar to PLT 
> entries under Linux, and I suppose the reason they want to keep using 
> them is to prevent problems with the 32MB displacement limit of PPC 
> branches.
> 
> For this reason, they disregard the result of the relocation lookup for 
> branches (since those don't point to any stubs, but to fully resolved 
> symbols) and simply change the offset in the copied branch so it keeps 
> pointing at the stub inside the op.o object file (an optimization to try 
> could be to check whether the final symbol lies within 32MB of the copy 
> of the code and if so, use a direct branch instead of one via a stub).
> 
> Anyway, in case of the __op_gen_label stuff, the branch does have to 
> changed so it uses gen_table[], so I added a strstart() check to account 
> for this special situation.

Thank you for the explanation. I understand the issue now.

> PS: symbols which are still in the dynamic symbol table and therefore 
> not properly relocated under Mac OS X (also not in Qemu 0.61) are 
> rclw_table, rclb_table and parity_table. The hack to declare them as 
> "static" does not currently work for them, because then you get 
> definition conflicts. A proper fix would be to add support for parsing 
> the dynamic symbol table as well.
> 
> A hack would be to move some things to different include files so the 
> "static" definition is the only one that appears when compiling op.o, 
> and the "extern" definitions when compiling other files. Another 
> solution is to parse the output of "otool -IV op.o", as these 
> variables/symbols are shown by that one in the contents of the 
> "(__DATA,__nl_symbol_ptr)" section.

What not just keeping them as they are now ? Are you sure the code is 
badly relocated ? It seems that a reference to the equivalent of the ELF 
GOT is used to access these tables - I don't understand why a specific 
relocation is needed.

The real problem is that gcc for Mac OS X does not support the 
"visibility" attribute. Setting the visibility of the symbol to "hidden" 
would solve the problem !

Fabrice.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-20 14:29     ` Jonas Maebe
  2005-02-21 18:58       ` Fabrice Bellard
@ 2005-02-21 20:27       ` Fabrice Bellard
  2005-02-21 20:58         ` Jonas Maebe
  1 sibling, 1 reply; 13+ messages in thread
From: Fabrice Bellard @ 2005-02-21 20:27 UTC (permalink / raw)
  To: qemu-devel

Jonas Maebe wrote:

> At address 0x743ac, the branch to __op_gen_label1 gets patched so it 
> points to the basic block coming after the loopzw. In that case, LR does 
> not get restored (it was destroyed at 0x74394 by the bctrl), so the next 
> blr which gets executed returns to 0x74398. This causes an endless loop, 
> obviously.
> 
> This does not seem to be Mac OS X-specific at all, and should also 
> happen under Linux/ppc I think.

I just made a fix. Tell me if you see other problems...

Fabrice.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-21 20:08           ` Fabrice Bellard
@ 2005-02-21 20:29             ` Jonas Maebe
  0 siblings, 0 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-21 20:29 UTC (permalink / raw)
  To: qemu-devel


On 21 feb 2005, at 21:08, Fabrice Bellard wrote:

>> PS: symbols which are still in the dynamic symbol table and therefore 
>> not properly relocated under Mac OS X (also not in Qemu 0.61) are 
>> rclw_table, rclb_table and parity_table. The hack to declare them as 
>> "static" does not currently work for them, because then you get 
>> definition conflicts. A proper fix would be to add support for 
>> parsing the dynamic symbol table as well.
>> A hack would be to move some things to different include files so the 
>> "static" definition is the only one that appears when compiling op.o, 
>> and the "extern" definitions when compiling other files. Another 
>> solution is to parse the output of "otool -IV op.o", as these 
>> variables/symbols are shown by that one in the contents of the 
>> "(__DATA,__nl_symbol_ptr)" section.
>
> What not just keeping them as they are now ? Are you sure the code is 
> badly relocated ?

Yes. Just search for "warning" in the generated op.h (i.e., they are 
not relocated at all).

> It seems that a reference to the equivalent of the ELF GOT is used to 
> access these tables

That is correct.

> - I don't understand why a specific relocation is needed.

It's not a special relocation, but a different symbol table. Mach-O 
object files contain two symbol tables: one which contains all private 
symbols (called the "symbol table", and which is already fully parsed 
by dyngen), and one which contains all imported and exported symbols 
(the "dynamic symbol table").

This dynamic symbol table command is loaded by dyngen (search for 
"dysymtab"), but the table itself is not loaded nor parsed. As such, 
when you find a relocated symbol in an instruction (which only contains 
a section number, an index and things like that, but no name) and this 
is an external (or exported) symbol, then dyngen won't be able to find 
its name (since it will only search the regular symbol table).

When dyngen can't find the name of a relocation, it can't create a 
fixup for it since it needs to print the name in the generated op.h

> The real problem is that gcc for Mac OS X does not support the 
> "visibility" attribute. Setting the visibility of the symbol to 
> "hidden" would solve the problem !

That's possible. If this attribute would result in the symbols becoming 
private, they would indeed move from the dynamic symbol table to the 
regular symbol table.


Jonas

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] Patch to fix Mac OS X compilation
  2005-02-21 20:27       ` Fabrice Bellard
@ 2005-02-21 20:58         ` Jonas Maebe
  0 siblings, 0 replies; 13+ messages in thread
From: Jonas Maebe @ 2005-02-21 20:58 UTC (permalink / raw)
  To: qemu-devel


On 21 feb 2005, at 21:27, Fabrice Bellard wrote:

>> At address 0x743ac, the branch to __op_gen_label1 gets patched so it 
>> points to the basic block coming after the loopzw. In that case, LR 
>> does not get restored (it was destroyed at 0x74394 by the bctrl), so 
>> the next blr which gets executed returns to 0x74398. This causes an 
>> endless loop, obviously.
>> This does not seem to be Mac OS X-specific at all, and should also 
>> happen under Linux/ppc I think.
>
> I just made a fix. Tell me if you see other problems...

Thanks, it seems to work great now! There's still the problems of 
rclw_table, rclb_table and parity_table not being relocated properly, 
but that doesn't affect many programs (who uses daa? :) and was already 
the same in 0.61.


Jonas

PS: I wonder when we'll see an updated iEmulator <g>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2005-02-21 21:20 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-12 20:47 [Qemu-devel] Patch to fix Mac OS X compilation Jonas Maebe
2005-02-13 13:30 ` Daniel Egger
2005-02-13 14:36   ` Jonas Maebe
2005-02-13 14:30 ` Jonas Maebe
2005-02-19 20:00   ` Jonas Maebe
2005-02-20 14:29     ` Jonas Maebe
2005-02-21 18:58       ` Fabrice Bellard
2005-02-21 19:36         ` Jonas Maebe
2005-02-21 19:43           ` Jonas Maebe
2005-02-21 20:08           ` Fabrice Bellard
2005-02-21 20:29             ` Jonas Maebe
2005-02-21 20:27       ` Fabrice Bellard
2005-02-21 20:58         ` Jonas Maebe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).