pnkfelix / krabcake

Umbrella repository for Krabcake experiments
Other
46 stars 1 forks source link

todo: double-check asm for valgrind_do_client_request_expr and so on #18

Open pnkfelix opened 1 year ago

pnkfelix commented 1 year ago

Look at:

https://github.com/pnkfelix/krabcake-vg/blob/075a18ae43c29b5e12f9292867857aacb4faf1f1/include/valgrind.h.in#L432-L438

namely

    __asm__ volatile(__SPECIAL_INSTRUCTION_PREAMBLE               \
                     /* %RDX = client_request ( %RAX ) */         \
                     "xchgq %%rbx,%%rbx"                          \
                     : "=d" (_zzq_result)                         \
                     : "a" (&_zzq_args[0]), "0" (_zzq_default)    \
                     : "cc", "memory"                             \
                    );      

then look at

https://github.com/pnkfelix/krabcake/blob/e9fb135ea005b8888c15d2055c6dc9dfab3cbea1/kc/test_dependencies/src/lib.rs#L66-L74

We need to be convinced the top and the bottom have the same semantics. (As part of this, I probably need to write more of our initial tests in C, rather than moving them all into Rust at the outset like we had planned.)

In particular, I am currently worried that some state corruption I am observing, where the guest rdx is written before a client request, and read after the client request, but the client request itself is allowed to overwrite that guest rdx, is a sign that something in our asm block is not properly specifying to the compiler that the asm may itself overwrite rdx and therefore that the compiler must save and restore rdx around the asm block, or allocate the local state in some other way.

pnkfelix commented 1 year ago

Hmm.

Isn't the primary thing I'm noting, the overwrite of rdx, isn't that indeed handled by the inout("di") in the given spec?

Curiouser and curiouser.

Update: wait, di is not the same as dx... that's probably the bug here

bryangarza commented 1 year ago

@pnkfelix I guess we can close this?