|
From: Julian S. <js...@ac...> - 2008-03-23 03:04:57
|
On Sunday 23 March 2008 02:01, Nicholas Nethercote wrote:
> > ------ IMark(0x80483D5, 7) ------
> > PUT(60) = 0x80483D5:I32
> > t137 = Left32(t130)
> > t26 = Add32(t25,0xFFFFFFF8:I32)
> > t139 = CmpNEZ32(t137)
> > DIRTY t139 RdFX-gst(16,4) RdFX-gst(60,4) :::
> > MC_(helperc_value_check4_fail){0x38006920}() DIRTY 1:I1 RdFX-gst(16,4)
> > RdFX-gst(60,4) ::: MC_(helperc_STOREV32le)[rp=2]{0x38006d40}(t26,0x0:I32)
> > STle(t26) = 0x4:I32
> > At first glance I would say that the first PUT(60) could be removed,
> > right? It is basically a dead store (assuming it can only be read/write
> > with GET/PUT). I didn't check further why the redudant removal
> > optimization doesn't pick this, so I'm checking here first if this is
> > sane.
>
> It's not removed because there's another statement (the STle) between it
> and the subsequent PUT(60) that could cause a memory exception. A signal
> handler for such an exception might inspect the %eip value, so it has to be
> up-to-date. Annoying, because it's so unlikely, but necessary.
Yes. But not only for the STle, also for the call to
MC_(helperc_value_check4_fail).
If you look at struct VexGuestX86State in libvex_guest_x86.h, you can
see that offset 60 is guest_EIP (the program counter). So the PUT is
updating the simulated program counter, so it is up to date should the
store generate an exception, but also (importantly) so that if
MC_(helperc_value_check4_fail) needs to make a stack backtrace, it will
start from the right point.
MC_(helperc_value_check4_fail) is the function that will, or at least
may, eventually lead to memcheck giving an error "Use of uninitialised
value of size 4".
If you look at priv/guest-x86/ghelpers.c, function
guest_x86_state_requires_precise_mem_exns tells the ir optimiser
(iropt.c) which parts of the guest state must be kept up to date at
all times, basically so that helper functions can get correct
stack backtraces. So it forces EBP, ESP and EIP to always be up
to date; all other guest registers can get completely out of date
and are only sync'd with reality at superblock transitions.
> > Second question is: can the last memcheck check be removed? i.e. the last
> > call to helperc_LOADV32le() is redudant, since it has already done a
> > store to that location and thus it knows that the operation is safe. Can
> > this call be removed? And what value do I assign to t144 then?
> > VA_BITS32_DEFINED? (I'm just considering the simpler case where the
> > #store bits >= #load bits).
>
> I think it is redundant. Since 0x0 (which is VA_BITS32_DEFINED) was the
> shadow store value, you'd assign 0x0 to t144. And you're right that you
> could do likewise if t144 was smaller than 32 bits.
>
> > Allow me just a last question: is it safe to replace the 't30 =
> > LDle:I32(t26)' statement with 't30 = 0x4:I32'? Well in general I would
> > say it is safe, but I dunno about memory-mapped I/O nor if/how valgrind
> > handles it. Maybe this can be done in only certain architectures?
>
> If we're optimising away the shadow load, maybe it's reasonable to optimise
> away the real load, but I'm really not sure about that one. Julian might
> have more to add.
Difficult question! I would comment first that it might be worth looking
at the original code to figure out why the compiler put a store and then
a load from the same location in the next insn. It might be that the
second instruction (0x80483DC) is a branch target, maybe the top of a
loop.
So is it safe to remove the real and shadow load and forward values
directly to it from the real and shadow store? Well, that would probably
work _most_ of the time, but sounds dangerous if multiple threads are
doing spinlocks or something like that - then it might be that changing
the number of memory references might have some bad effect.
J
|