Finally: Mac Intel patches

[Joel Reymont]

Hi Mikael!

Thank you for your comments. See my response below.

On Aug 23, 2006, at 5:42 PM, Mikael Pettersson wrote:

> I don't think there's any guarantee that Darwin only runs on
> SSE2-capable x86-32 processors, so omitting the test seems dangerous.
> The runtime test should work on all x86 processors, so I'm going to
> let Darwin on x86-32 test at runtime.

Fair enough.

>> +#elif defined(__DARWIN__) && defined(__i386__)
>> +    mcontext_t mc = uc->uc_mcontext;
>> +    i386_float_state_t fpstate = mc->fs;
>> +    fpstate.fpu_mxcsr = 0x1F80;
>> +    *((unsigned short *)&fpstate.fpu_fsw) &= ~0xFF;
>> #elif defined(__DARWIN__)
>>     mcontext_t mc = uc->uc_mcontext;
>>     mc->ss.srr0 += 4;
>
> You're copying the float state into fpstate and updating the copy
> without flushing the changes back to the original. This cannot be
> right. fpstate should be a pointer to mc->fs.

I'm updating the structure and the changes are flushed back by the  
kernel when the signal handler terminates.

> What is the concrete type of the fpu_fsw field? I really don't
> want to update it via a pointer cast like this.

Here's the full structure (well, almost):

/* For legacy reasons we need to leave the hw_state as char bytes */
struct i386_float_state {
         int                     fpu_reserved[2];
         fp_control_t            fpu_fcw;                        /*  
x87 FPU control word */
         fp_status_t             fpu_fsw;                        /*  
x87 FPU status word */
         uint8_t                 fpu_ftw;                        /*  
x87 FPU tag word */
         uint8_t                 fpu_rsrv1;                      /*  
reserved */
         uint16_t                fpu_fop;                        /*  
x87 FPU Opcode */
         uint32_t                fpu_ip;                         /*  
x87 FPU Instruction Pointer offset */
         uint16_t                fpu_cs;                         /*  
x87 FPU Instruction Pointer Selector */
         uint16_t                fpu_rsrv2;                      /*  
reserved */
         uint32_t                fpu_dp;                         /*  
x87 FPU Instruction Operand(Data) Pointer offset */
         uint16_t                fpu_ds;                         /*  
x87 FPU Instruction Operand(Data) Pointer Selector */
         uint16_t                fpu_rsrv3;                      /*  
reserved */
         uint32_t                fpu_mxcsr;                      /*  
MXCSR Register state */
         uint32_t                fpu_mxcsrmask;          /* MXCSR  
mask */

fpu_sw has type of fp_status_t which is a union.

/*
* Status word.
*/

typedef struct fp_status {
     unsigned short              invalid :1,
                                 denorm  :1,
                                 zdiv    :1,
                                 ovrfl   :1,
                                 undfl   :1,
                                 precis  :1,
                                 stkflt  :1,
                                 errsumm :1,
                                 c0      :1,
                                 c1      :1,
                                 c2      :1,
                                 tos     :3,
                                 c3      :1,
                                 busy    :1;
} fp_status_t;


> The current x86 assembly code uses .global not .globl.

Apple's assembler uses .globl. Don't ask me!

> Adding mach_override.o needs to be conditional on OPSYS=darwin.
> I introduced an auxiliary make variable to express that.

Thanks!

> You don't use dlsym() etc, so why include dlfcn.h?

An oversight.

> I rearranged the hipe_x86_signal.c changes to make them smaller.

Thanks again. I'll apply your patches and will let you know.

--
http://wagerlabs.com/