| Message ID | CAFAwnGMTjFUMgbB_sWxZz2_3CwhrMkvjGmkGGpyeZ_4u=dtRhQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
2012/10/19 Julio Guerra <guerr@julio.in>: > This patch fixes bug 1031698 : > https://bugs.launchpad.net/qemu/+bug/1031698 > > If we look at the (truncated) translation of the conditional branch > instruction in the test submitted in the bug post, the call to the > exception helper is missing in the "bne-false" chunk of translated > code : > > IN: > bne- 0x1800278 > > OUT: > 0xb544236d: jne 0xb5442396 > > 0xb5442373: mov %ebp,(%esp) > 0xb5442376: mov $0x44,%ebx > 0xb544237b: mov %ebx,0x4(%esp) > 0xb544237f: mov $0x1800278,%ebx > 0xb5442384: mov %ebx,0x25c(%ebp) > 0xb544238a: call 0x827475a > ^^^^^^^^^^^^^^^^^^ > # OK : call the exception helper function > > 0xb5442396: mov %ebp,(%esp) > 0xb5442399: mov $0x44,%ebx > 0xb544239e: mov %ebx,0x4(%esp) > 0xb54423a2: mov $0x1800270,%ebx > 0xb54423a7: mov %ebx,0x25c(%ebp) > # KO : missing "call 0x827475a" > > > Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by > gen_bcond) changes ctx->exception's value to excp's : > > gen_bcond() > { > gen_goto_tb(ctx, 0, ctx->nip + li - 4); > /* ctx->exception value is POWERPC_EXCP_BRANCH */ > > gen_goto_tb(ctx, 1, ctx->nip); > /* ctx->exception now value is POWERPC_EXCP_TRACE */ > } > > > Making the following gen_goto_tb()'s test false during the second call : > > if ((ctx->singlestep_enabled & > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) { > target_ulong tmp = ctx->nip; > ctx->nip = dest; > /* ... and this is the missing call */ > gen_exception(ctx, POWERPC_EXCP_TRACE); > ctx->nip = tmp; > } > > So the patch simply adds the missing matching case, fixing our problem. > > Signed-off-by: Julio Guerra <guerr@julio.in> > --- > target-ppc/translate.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff -up a/target-ppc/translate.c b/target-ppc/translate.c > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont > if (unlikely(ctx->singlestep_enabled)) { > if ((ctx->singlestep_enabled & > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > - ctx->exception == POWERPC_EXCP_BRANCH) { > + (ctx->exception == POWERPC_EXCP_BRANCH || > + ctx->exception == POWERPC_EXCP_TRACE)) { > target_ulong tmp = ctx->nip; > ctx->nip = dest; > gen_exception(ctx, POWERPC_EXCP_TRACE); ping -- Julio Guerra
On 19.10.2012, at 12:17, Julio Guerra wrote: > This patch fixes bug 1031698 : > https://bugs.launchpad.net/qemu/+bug/1031698 > > If we look at the (truncated) translation of the conditional branch > instruction in the test submitted in the bug post, the call to the > exception helper is missing in the "bne-false" chunk of translated > code : > > IN: > bne- 0x1800278 > > OUT: > 0xb544236d: jne 0xb5442396 > > 0xb5442373: mov %ebp,(%esp) > 0xb5442376: mov $0x44,%ebx > 0xb544237b: mov %ebx,0x4(%esp) > 0xb544237f: mov $0x1800278,%ebx > 0xb5442384: mov %ebx,0x25c(%ebp) > 0xb544238a: call 0x827475a > ^^^^^^^^^^^^^^^^^^ > # OK : call the exception helper function > > 0xb5442396: mov %ebp,(%esp) > 0xb5442399: mov $0x44,%ebx > 0xb544239e: mov %ebx,0x4(%esp) > 0xb54423a2: mov $0x1800270,%ebx > 0xb54423a7: mov %ebx,0x25c(%ebp) > # KO : missing "call 0x827475a" > > > Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by > gen_bcond) changes ctx->exception's value to excp's : > > gen_bcond() > { > gen_goto_tb(ctx, 0, ctx->nip + li - 4); > /* ctx->exception value is POWERPC_EXCP_BRANCH */ > > gen_goto_tb(ctx, 1, ctx->nip); > /* ctx->exception now value is POWERPC_EXCP_TRACE */ > } > > > Making the following gen_goto_tb()'s test false during the second call : > > if ((ctx->singlestep_enabled & > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) { > target_ulong tmp = ctx->nip; > ctx->nip = dest; > /* ... and this is the missing call */ > gen_exception(ctx, POWERPC_EXCP_TRACE); > ctx->nip = tmp; > } > > So the patch simply adds the missing matching case, fixing our problem. > > Signed-off-by: Julio Guerra <guerr@julio.in> Thanks, applied to ppc-next :) Alex > --- > target-ppc/translate.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff -up a/target-ppc/translate.c b/target-ppc/translate.c > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont > if (unlikely(ctx->singlestep_enabled)) { > if ((ctx->singlestep_enabled & > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > - ctx->exception == POWERPC_EXCP_BRANCH) { > + (ctx->exception == POWERPC_EXCP_BRANCH || > + ctx->exception == POWERPC_EXCP_TRACE)) { > target_ulong tmp = ctx->nip; > ctx->nip = dest; > gen_exception(ctx, POWERPC_EXCP_TRACE);
Am 23.11.2012 17:06, schrieb Alexander Graf: > > On 19.10.2012, at 12:17, Julio Guerra wrote: > >> This patch fixes bug 1031698 : >> https://bugs.launchpad.net/qemu/+bug/1031698 >> >> If we look at the (truncated) translation of the conditional branch >> instruction in the test submitted in the bug post, the call to the >> exception helper is missing in the "bne-false" chunk of translated >> code : >> >> IN: >> bne- 0x1800278 >> >> OUT: >> 0xb544236d: jne 0xb5442396 >> >> 0xb5442373: mov %ebp,(%esp) >> 0xb5442376: mov $0x44,%ebx >> 0xb544237b: mov %ebx,0x4(%esp) >> 0xb544237f: mov $0x1800278,%ebx >> 0xb5442384: mov %ebx,0x25c(%ebp) >> 0xb544238a: call 0x827475a >> ^^^^^^^^^^^^^^^^^^ >> # OK : call the exception helper function >> >> 0xb5442396: mov %ebp,(%esp) >> 0xb5442399: mov $0x44,%ebx >> 0xb544239e: mov %ebx,0x4(%esp) >> 0xb54423a2: mov $0x1800270,%ebx >> 0xb54423a7: mov %ebx,0x25c(%ebp) >> # KO : missing "call 0x827475a" >> >> >> Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by >> gen_bcond) changes ctx->exception's value to excp's : >> >> gen_bcond() >> { >> gen_goto_tb(ctx, 0, ctx->nip + li - 4); >> /* ctx->exception value is POWERPC_EXCP_BRANCH */ >> >> gen_goto_tb(ctx, 1, ctx->nip); >> /* ctx->exception now value is POWERPC_EXCP_TRACE */ >> } >> >> >> Making the following gen_goto_tb()'s test false during the second call : >> >> if ((ctx->singlestep_enabled & >> (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && >> ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) { >> target_ulong tmp = ctx->nip; >> ctx->nip = dest; >> /* ... and this is the missing call */ >> gen_exception(ctx, POWERPC_EXCP_TRACE); >> ctx->nip = tmp; >> } >> >> So the patch simply adds the missing matching case, fixing our problem. >> >> Signed-off-by: Julio Guerra <guerr@julio.in> > > Thanks, applied to ppc-next :) Please don't forget to add the missing "target-ppc: ". :) Andreas
diff -up a/target-ppc/translate.c b/target-ppc/translate.c --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -3466,7 +3466,8 @@ static inline void gen_goto_tb(DisasCont if (unlikely(ctx->singlestep_enabled)) { if ((ctx->singlestep_enabled & (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && - ctx->exception == POWERPC_EXCP_BRANCH) { + (ctx->exception == POWERPC_EXCP_BRANCH || + ctx->exception == POWERPC_EXCP_TRACE)) { target_ulong tmp = ctx->nip; ctx->nip = dest; gen_exception(ctx, POWERPC_EXCP_TRACE);
This patch fixes bug 1031698 : https://bugs.launchpad.net/qemu/+bug/1031698 If we look at the (truncated) translation of the conditional branch instruction in the test submitted in the bug post, the call to the exception helper is missing in the "bne-false" chunk of translated code : IN: bne- 0x1800278 OUT: 0xb544236d: jne 0xb5442396 0xb5442373: mov %ebp,(%esp) 0xb5442376: mov $0x44,%ebx 0xb544237b: mov %ebx,0x4(%esp) 0xb544237f: mov $0x1800278,%ebx 0xb5442384: mov %ebx,0x25c(%ebp) 0xb544238a: call 0x827475a ^^^^^^^^^^^^^^^^^^ # OK : call the exception helper function 0xb5442396: mov %ebp,(%esp) 0xb5442399: mov $0x44,%ebx 0xb544239e: mov %ebx,0x4(%esp) 0xb54423a2: mov $0x1800270,%ebx 0xb54423a7: mov %ebx,0x25c(%ebp) # KO : missing "call 0x827475a" Indeed, gen_exception(ctx, excp) called by gen_goto_tb (called by gen_bcond) changes ctx->exception's value to excp's : gen_bcond() { gen_goto_tb(ctx, 0, ctx->nip + li - 4); /* ctx->exception value is POWERPC_EXCP_BRANCH */ gen_goto_tb(ctx, 1, ctx->nip); /* ctx->exception now value is POWERPC_EXCP_TRACE */ } Making the following gen_goto_tb()'s test false during the second call : if ((ctx->singlestep_enabled & (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && ctx->exception == POWERPC_EXCP_BRANCH /* false...*/) { target_ulong tmp = ctx->nip; ctx->nip = dest; /* ... and this is the missing call */ gen_exception(ctx, POWERPC_EXCP_TRACE); ctx->nip = tmp; } So the patch simply adds the missing matching case, fixing our problem. Signed-off-by: Julio Guerra <guerr@julio.in> --- target-ppc/translate.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)