| Submitter | Vitaly Chipounov |
|---|---|
| Date | July 2, 2012, 10:20 p.m. |
| Message ID | <1341267649-18231-1-git-send-email-vitaly.chipounov@epfl.ch> |
| Download | mbox | patch |
| Permalink | /patch/168651/ |
| State | New |
| Headers | show |
Comments
On Tue, Jul 3, 2012 at 2:20 AM, Vitaly Chipounov <vitaly.chipounov@epfl.ch> wrote: > According to the Intel manual > "Intel® 64 and IA-32 Architectures Software Developer’s Manual > Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": > > "When in compatibility mode, FS and GS overrides operate as defined by > 32-bit mode behavior regardless of the value loaded into the upper 32 > linear-address bits of the hidden descriptor register base field. > Compatibility mode ignores the upper 32 bits when calculating an effective address." > > However, the code misses the 64-bit mode case, where an instruction with > address and segment size override would be translated incorrectly. For example, > inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: > > (uint32_t)(gs.base + ebx * 4 + 0x260) > instead of > gs.base + (uint32_t)(ebx * 4 + 0x260) > > Signed-off-by: Vitaly Chipounov <vitaly.chipounov@epfl.ch> Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
Thanks, applied. On Mon, Jul 2, 2012 at 10:20 PM, Vitaly Chipounov <vitaly.chipounov@epfl.ch> wrote: > According to the Intel manual > "Intel® 64 and IA-32 Architectures Software Developer’s Manual > Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": > > "When in compatibility mode, FS and GS overrides operate as defined by > 32-bit mode behavior regardless of the value loaded into the upper 32 > linear-address bits of the hidden descriptor register base field. > Compatibility mode ignores the upper 32 bits when calculating an effective address." > > However, the code misses the 64-bit mode case, where an instruction with > address and segment size override would be translated incorrectly. For example, > inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: > > (uint32_t)(gs.base + ebx * 4 + 0x260) > instead of > gs.base + (uint32_t)(ebx * 4 + 0x260) > > Signed-off-by: Vitaly Chipounov <vitaly.chipounov@epfl.ch> > --- > target-i386/translate.c | 43 +++++++++++++++++++++++++------------------ > 1 files changed, 25 insertions(+), 18 deletions(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index a902f4a..dfec735 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -456,12 +456,19 @@ static inline void gen_op_movl_A0_seg(int reg) > tcg_gen_ld32u_tl(cpu_A0, cpu_env, offsetof(CPUX86State, segs[reg].base) + REG_L_OFFSET); > } > > -static inline void gen_op_addl_A0_seg(int reg) > +static inline void gen_op_addl_A0_seg(DisasContext *s, int reg) > { > tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base)); > - tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > #ifdef TARGET_X86_64 > - tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); > + if (CODE64(s)) { > + tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); > + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > + } else { > + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > + tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); > + } > +#else > + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); > #endif > } > > @@ -617,7 +624,7 @@ static inline void gen_string_movl_A0_ESI(DisasContext *s) > override = R_DS; > gen_op_movl_A0_reg(R_ESI); > gen_op_andl_A0_ffff(); > - gen_op_addl_A0_seg(override); > + gen_op_addl_A0_seg(s, override); > } > } > > @@ -638,7 +645,7 @@ static inline void gen_string_movl_A0_EDI(DisasContext *s) > } else { > gen_op_movl_A0_reg(R_EDI); > gen_op_andl_A0_ffff(); > - gen_op_addl_A0_seg(R_ES); > + gen_op_addl_A0_seg(s, R_ES); > } > } > > @@ -2063,7 +2070,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ > } else > #endif > { > - gen_op_addl_A0_seg(override); > + gen_op_addl_A0_seg(s, override); > } > } > } else { > @@ -2130,7 +2137,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ > else > override = R_DS; > } > - gen_op_addl_A0_seg(override); > + gen_op_addl_A0_seg(s, override); > } > } > > @@ -2207,7 +2214,7 @@ static void gen_add_A0_ds_seg(DisasContext *s) > } else > #endif > { > - gen_op_addl_A0_seg(override); > + gen_op_addl_A0_seg(s, override); > } > } > } > @@ -2460,12 +2467,12 @@ static void gen_push_T0(DisasContext *s) > if (s->ss32) { > if (s->addseg) { > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > } else { > gen_op_andl_A0_ffff(); > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > gen_op_st_T0_A0(s->dflag + 1 + s->mem_index); > if (s->ss32 && !s->addseg) > @@ -2500,11 +2507,11 @@ static void gen_push_T1(DisasContext *s) > gen_op_addl_A0_im(-4); > if (s->ss32) { > if (s->addseg) { > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > } else { > gen_op_andl_A0_ffff(); > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > gen_op_st_T1_A0(s->dflag + 1 + s->mem_index); > > @@ -2528,10 +2535,10 @@ static void gen_pop_T0(DisasContext *s) > gen_op_movl_A0_reg(R_ESP); > if (s->ss32) { > if (s->addseg) > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } else { > gen_op_andl_A0_ffff(); > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > gen_op_ld_T0_A0(s->dflag + 1 + s->mem_index); > } > @@ -2556,7 +2563,7 @@ static void gen_stack_A0(DisasContext *s) > gen_op_andl_A0_ffff(); > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > if (s->addseg) > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > } > > /* NOTE: wrap around in 16 bit not fully handled */ > @@ -2569,7 +2576,7 @@ static void gen_pusha(DisasContext *s) > gen_op_andl_A0_ffff(); > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > if (s->addseg) > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > for(i = 0;i < 8; i++) { > gen_op_mov_TN_reg(OT_LONG, 0, 7 - i); > gen_op_st_T0_A0(OT_WORD + s->dflag + s->mem_index); > @@ -2588,7 +2595,7 @@ static void gen_popa(DisasContext *s) > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > tcg_gen_addi_tl(cpu_T[1], cpu_T[1], 16 << s->dflag); > if (s->addseg) > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > for(i = 0;i < 8; i++) { > /* ESP is not reloaded */ > if (i != 3) { > @@ -2638,7 +2645,7 @@ static void gen_enter(DisasContext *s, int esp_addend, int level) > gen_op_andl_A0_ffff(); > tcg_gen_mov_tl(cpu_T[1], cpu_A0); > if (s->addseg) > - gen_op_addl_A0_seg(R_SS); > + gen_op_addl_A0_seg(s, R_SS); > /* push bp */ > gen_op_mov_TN_reg(OT_LONG, 0, R_EBP); > gen_op_st_T0_A0(ot + s->mem_index); > -- > 1.7.4.1 > >
Patch
diff --git a/target-i386/translate.c b/target-i386/translate.c index a902f4a..dfec735 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -456,12 +456,19 @@ static inline void gen_op_movl_A0_seg(int reg) tcg_gen_ld32u_tl(cpu_A0, cpu_env, offsetof(CPUX86State, segs[reg].base) + REG_L_OFFSET); } -static inline void gen_op_addl_A0_seg(int reg) +static inline void gen_op_addl_A0_seg(DisasContext *s, int reg) { tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base)); - tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); #ifdef TARGET_X86_64 - tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); + if (CODE64(s)) { + tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); + } else { + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); + tcg_gen_andi_tl(cpu_A0, cpu_A0, 0xffffffff); + } +#else + tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); #endif } @@ -617,7 +624,7 @@ static inline void gen_string_movl_A0_ESI(DisasContext *s) override = R_DS; gen_op_movl_A0_reg(R_ESI); gen_op_andl_A0_ffff(); - gen_op_addl_A0_seg(override); + gen_op_addl_A0_seg(s, override); } } @@ -638,7 +645,7 @@ static inline void gen_string_movl_A0_EDI(DisasContext *s) } else { gen_op_movl_A0_reg(R_EDI); gen_op_andl_A0_ffff(); - gen_op_addl_A0_seg(R_ES); + gen_op_addl_A0_seg(s, R_ES); } } @@ -2063,7 +2070,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ } else #endif { - gen_op_addl_A0_seg(override); + gen_op_addl_A0_seg(s, override); } } } else { @@ -2130,7 +2137,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ else override = R_DS; } - gen_op_addl_A0_seg(override); + gen_op_addl_A0_seg(s, override); } } @@ -2207,7 +2214,7 @@ static void gen_add_A0_ds_seg(DisasContext *s) } else #endif { - gen_op_addl_A0_seg(override); + gen_op_addl_A0_seg(s, override); } } } @@ -2460,12 +2467,12 @@ static void gen_push_T0(DisasContext *s) if (s->ss32) { if (s->addseg) { tcg_gen_mov_tl(cpu_T[1], cpu_A0); - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } } else { gen_op_andl_A0_ffff(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } gen_op_st_T0_A0(s->dflag + 1 + s->mem_index); if (s->ss32 && !s->addseg) @@ -2500,11 +2507,11 @@ static void gen_push_T1(DisasContext *s) gen_op_addl_A0_im(-4); if (s->ss32) { if (s->addseg) { - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } } else { gen_op_andl_A0_ffff(); - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } gen_op_st_T1_A0(s->dflag + 1 + s->mem_index); @@ -2528,10 +2535,10 @@ static void gen_pop_T0(DisasContext *s) gen_op_movl_A0_reg(R_ESP); if (s->ss32) { if (s->addseg) - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } else { gen_op_andl_A0_ffff(); - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } gen_op_ld_T0_A0(s->dflag + 1 + s->mem_index); } @@ -2556,7 +2563,7 @@ static void gen_stack_A0(DisasContext *s) gen_op_andl_A0_ffff(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); if (s->addseg) - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); } /* NOTE: wrap around in 16 bit not fully handled */ @@ -2569,7 +2576,7 @@ static void gen_pusha(DisasContext *s) gen_op_andl_A0_ffff(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); if (s->addseg) - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); for(i = 0;i < 8; i++) { gen_op_mov_TN_reg(OT_LONG, 0, 7 - i); gen_op_st_T0_A0(OT_WORD + s->dflag + s->mem_index); @@ -2588,7 +2595,7 @@ static void gen_popa(DisasContext *s) tcg_gen_mov_tl(cpu_T[1], cpu_A0); tcg_gen_addi_tl(cpu_T[1], cpu_T[1], 16 << s->dflag); if (s->addseg) - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); for(i = 0;i < 8; i++) { /* ESP is not reloaded */ if (i != 3) { @@ -2638,7 +2645,7 @@ static void gen_enter(DisasContext *s, int esp_addend, int level) gen_op_andl_A0_ffff(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); if (s->addseg) - gen_op_addl_A0_seg(R_SS); + gen_op_addl_A0_seg(s, R_SS); /* push bp */ gen_op_mov_TN_reg(OT_LONG, 0, R_EBP); gen_op_st_T0_A0(ot + s->mem_index);
According to the Intel manual "Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": "When in compatibility mode, FS and GS overrides operate as defined by 32-bit mode behavior regardless of the value loaded into the upper 32 linear-address bits of the hidden descriptor register base field. Compatibility mode ignores the upper 32 bits when calculating an effective address." However, the code misses the 64-bit mode case, where an instruction with address and segment size override would be translated incorrectly. For example, inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: (uint32_t)(gs.base + ebx * 4 + 0x260) instead of gs.base + (uint32_t)(ebx * 4 + 0x260) Signed-off-by: Vitaly Chipounov <vitaly.chipounov@epfl.ch> --- target-i386/translate.c | 43 +++++++++++++++++++++++++------------------ 1 files changed, 25 insertions(+), 18 deletions(-)