* [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-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-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 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-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-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: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).