Re: [Qemu-devel] Patch to fix Mac OS X compilation

[Fabrice Bellard <fabrice@bellard.org>]

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.