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.)
>

Right.


>  ​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.

Rob
-- 
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