rr refactorings

Robert O'Callahan robert at ocallahan.org
Mon Apr 21 20:40:10 PDT 2014

On Tue, Apr 22, 2014 at 3:20 PM, Chris Jones <jones.chris.g at gmail.com>wrote:

> Do you mind if I add these to the x86-64 wiki?

That's fine of course.

> On Mon, Apr 21, 2014 at 6:32 PM, Robert O'Callahan <robert at ocallahan.org>wrote:
>> I've got a few ideas about things I'd like to do, to clean up the code a
>> bit and work towards x86-64 support. Not planning to do it all right away,
>> but I'd like some feedback.
>> 1) Currently pointers to tracee memory have the same type as pointers to
>> rr memory. I find this makes code harder to read. I suggest introducing a
>> TaskPtr<T> type which is a pointer to T in tracee memory. This would be a
>> struct wrapping a uintptr_t. We'd use it for all tracee pointers.
> ​Sounds fine to me as long as it's easily convertible to and constructible
> from T* / void*.

I think we'd do that initially but we might be able to remove those
conversions for extra safety later.

> (I might bikeshed |remote<T>| for brevity and ​avoiding potential
> ambiguity around "task".)

Sounds good. remote or Remote? What are our naming conventions? :-)

>> * Then we can have a Task::record_remote(TaskPtr<T> p) variant that
>> records sizeof(T) bytes.
> ​(Or ​|record(remote<T>)| and |record(const T&)|.  "_remote" and
> "TaskPtr<" are redundant.)


>  ​Was there a "2)"?​

No :-)

> 4) Many kernel ABI structs have different layouts for 32-bit vs 64-bit,
>> e.g. iovec, stat64, getrusage. I don't think we can pull the definitions of
>> these structs from #include files in a way that makes both the 32-bit
>> definition and the 64-bit definition visible to rr. It would also be a
>> little painful to use two separate definitions. So how about:
> ​If I follow correctly, you're proposing to have rr declare local versions
> of the "common-case" arch-varying structs like iovec?  And you want both
> the 32-bit and 64-bit variants to be explicitly name-able to support
> mixed-arch tracee trees in the future?

Correct. And nameable via a template parameter so we can easily write
architecture-generic code which wants architecture-dependent types.

​That sounds fine, but one issue I'd point out is that even for the same
> arch, both 32-bit and 64-bit variants of structs can be used.  For example,
> on x86, |fcntl64(GETLK, struct flock)| works, and so does
> |fcntl64(GETLK64(, struct flock64)|.  There's really not an elegant
> solution for that, but it feels a little awkward to name those variants by
> architecture when they're just different off_t widths.  I'm not proposing
> anything, just pointing that out :).

I think the best solution depends on whether, on x86-64, the kernel's
struct flock uses 32-bit or 64-bit offsets. If it uses 64-bit the kernel's
struct flock is Arch-dependent so we should give it an Arch parameter. If
it uses 32-bit then flock doesn't need to be parameterized. flock64 doesn't
need to be parameterized since it's the same in both architectures.

Jtehsauts  tshaei dS,o n" Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.r"t sS?o  Whhei csha iids  teoa
stiheer :p atroa lsyazye,d  'mYaonu,r  "sGients  uapr,e  tfaokreg iyvoeunr,
'm aotr  atnod  sgaoy ,h o'mGee.t"  uTph eann dt hwea lmka'n?  gBoutt  uIp
waanndt  wyeonut  thoo mken.o w
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/rr-dev/attachments/20140422/d1d2e8ab/attachment-0001.html>

More information about the rr-dev mailing list