|
From: Florian K. <br...@ac...> - 2012-02-19 01:03:42
|
On 02/18/2012 05:23 PM, Julian Seward wrote:
>
> If I had to guess, I'd say you've fallen into the swamp of complexity
> that is the difference between "redir" and "no-redir" translations.
> I tried but failed to find in the sources, a comment explaining about
> this, so before I write a long explaination, can I ask:
>
> The places where the PC appears to be wrong: are these exactly the
> first basic blocks of redirected functions (in the sense of the
> functions defined in eg memcheck/mc_replace_strmem.c) ?
>
I just reran with --trace-redir=yes and in all cases where the PC
appeared to be wrong it was a redir translation.
For the failing regtest here is the redirect:
--24258-- REDIR: 0x49b60ab398 (index) redirected to 0x400933c (index)
GUEST_IA 49B60AB398 CURR_IA 400933C PUT_IP == 0
GUEST_IA 49B60AB398 PREV_IA 400933C CURR_IA 4009342
GUEST_IA 49B60AB398 PREV_IA 4009342 CURR_IA 4009348
.....
Shouldn't the PC in the guest state be updated here as well?
Florian
>
> On Saturday, February 18, 2012, Florian Krohm wrote:
>> Hi,
>>
>> I was attempting an optimization on s390x where updating the instruction
>> address in the guest state is an expensive operation. Piecing together
>> the 64-bit immediate value takes 4 (or 2) insns depending on model. So
>> I thought I could do the following (and save 2% - 5% of insns):
>>
>> if (put_IP) {
>> // If upper half is the same just update the lower half
>> if ((previous_IA >> 32) == (current_IA >> 32))
>> update lower 4 byte of instruction address in guest state
>> else
>> update full 8 byte of instruction address in guest state
>> }
>> previous_IA = current_IA;
>>
>> But I was proven wrong. memcheck/tests/strchr was failing by telling me:
>>
>> ==23763== Conditional jump or move depends on uninitialised value(s)
>> ==23763== at 0x4904009366: ??? <------<<
>> ==23763== by 0x8000067B: main (strchr.c:15)
>> ==23763==
>>
>> instead of
>>
>> ==22402== Conditional jump or move depends on uninitialised value(s)
>> ==22402== at 0x4009366: index (mc_replace_strmem.c:210)
>> ==22402== by 0x8000067B: main (strchr.c:15)
>> ==22402==
>>
>> I believe that the reason for this failure is that the instruction
>> address in the guest state is not set as expected when some brandnew
>> jitted code is executed for the first time. It is expected in
>> guest_generic_bb_to_IR:
>>
>> /* for the first insn, the dispatch loop will have set
>> %IP, but for all the others we have to do it ourselves. */
>> need_to_put_IP = toBool(n_instrs > 0);
>>
>> /* Finally, actually disassemble an instruction. */
>> dres = dis_instr_fn ( irsb,
>> need_to_put_IP,
>>
>> With some debug printf I see this happening:
>> (GUEST_IA is the contents of the instruction address in the guest state)
>>
>> In the disassembler when translating:
>>
>> GUEST_IA 49B60AB398 CURR_IA 400933C PUT_IP == 0
>> GUEST_IA 49B60AB398 PREV_IA 400933C CURR_IA 4009342
>> GUEST_IA 49B60AB398 PREV_IA 4009342 CURR_IA 4009348
>> GUEST_IA 49B60AB398 PREV_IA 4009348 CURR_IA 400934C
>> GUEST_IA 49B60AB398 PREV_IA 400934C CURR_IA 4009350
>> GUEST_IA 49B60AB398 PREV_IA 4009350 CURR_IA 400935E
>> GUEST_IA 49B60AB398 PREV_IA 400935E CURR_IA 4009364
>> GUEST_IA 49B60AB398 PREV_IA 4009364 CURR_IA 4009366
>> GUEST_IA 49B60AB398 PREV_IA 4009366 CURR_IA 400936A
>> GUEST_IA 49B60AB398 PREV_IA 400936A CURR_IA 4009370
>>
>> Right before entering VG_(run_innerloop) for execution it reports:
>>
>> CALLING run_innerloop: GUEST_IA is 49b60ab398
>>
>> I think that the GUEST_IA should have the value 400933C here.
>>
>> This is not due to a bug in the s390x dispatch loop. I see the same
>> thing (guest_EIP not having the expected value) also happening on x86.
>>
>> To summarize, I think there is a latent bug here which surfaced through
>> my optimization. Still, it should be corrected. Perhaps it is easiest to
>> just update the guest state all the time. That would (a) be correct and
>> (b) not hurt because IR optimization will remove the PUT anyhow if it is
>> not needed. I can cook up a patch if you think this is the right approach.
>>
>> Florian
|