[Ltrace-devel] ltrace: Try to fix MIPS arch (tested against git-fcf256c)

[Petr Machata]

Sedat Dilek <sedat.dilek at gmail.com> writes:

> On Sat, Sep 1, 2012 at 2:15 AM, Petr Machata <pmachata at redhat.com> wrote:
>> Sedat Dilek <sedat.dilek at gmail.com> writes:
>>> [ handle_event.c ]
>>>
>>> Fix "pred" uninitialized in pending_new_remove().
>>
>> That should go into a separate patch.  It's a good catch, but
>> conceptually has nothing to do with cleaning up MIPS.

This should be one patch.

>>> [ breakpoints.c ]
>>>
>>> MIPS arch has no own "breakpoints.c".
>>> IIRC sth, was wrong with "list_of_symbols" in
>>> enable_all_breakpoints()... "Process" has no member "list_of_symbols",
>>> so cut off the mips-ifdef part.
>>> Just testing compiles or not.
>>
>> Cutting this actually seems reasonable.  There was a similar ifdef for
>> PPC, and that is now gone as well (and PPC works fine).  I believe
>> ltrace now handles delaying breakpoint enablement.
>>
>>> [ handle_event.c ]
>>>
>>> Same as for breakpoints.c cut off the mips-ifdef, seen
>>> "list_of_symbols" errors in handle_breakpoint().
>>
>> Hmm, that seems like an implementation of the delayed start.  I think
>> this can be removed as well, with the same rationale as above.

These two should probably be in a patch by themselves.

>>> [ ltrace-elf.h ]
>>>
>>> Unfortunately, the ARCH_HAVE_LTELF_DATA defined in
>>> "sysdeps/linux-gnu/mipsel/arch.h" is somehow not recognized.
>>> Restore the mips-ifdef part thrown out in struct ltelf {}.
>>> See commit e67635d6dcec ("Move arch-specific bits from ltrace-elf.c to
>>> PPC and MIPS back ends") for more details.
>>
>> This is not acceptable.  If what you are seeing is redefinition error,
>> then arch.h might be included twice.  Note that it has no inclusion
>> guard.  The proper way to fix this is to put inclusion guards to that
>> file.

And this too.  (That's three already, so yesterday I miscounted.)

>>> As a bonbon I added syscallent.h.diff (signalent.h.diff was empty).
>>
>> That should also go to a separate patch.
>
> That highly depends on the target's Linux kernel version (see attached
> linux-2.6.13.1_syscallent.h.diff, the previous one was linux-2.6.32).

Yes, it does, but newer kernel versions never change meaning of older
system call numbers, so it's always safe to apply an update.  I
regenerated syscallent.h from latest kernel tree, that has a couple more
system calls still, and that is now on master.

I'm looking into this a bit, and it appears that the whole MIPS
situation is a fair deal of mess.  MIPS is endian switchable.  There is
currently one piece of code in ltrace that asks about endian, and that
is in lens_enum.c.  It would be best if we could rewrite it to work
either way.  Then we could drop the #define ARCH_ENDIAN_* stuff that I
put in some time ago.  Unless we want to support tracing MIPSel binaries
with MIPSeb ltrace (or the other way around), I don't believe we need to
burden ourselves with the endian business.  If we want to, then current
measures are insufficient anyway.

Then there are three ABI's (o32, n32, 64), I thought only Irix did this.
That means we need syscallent1.h as well, for the 64-bit cases (n32,
64).  But because syscall numbering is disjoint, it would lead to a
sparse array, and while it might work, it would be not very efficient.
We will be in pretty much the same situation with the new x86 ABI "x32",
and some support for this will have to land one way or another.

It is apparent that current MIPS back end only cares about o32.  I
suppose that's the more common case, as most "small" devices (routers et
al.) will just have 32-bit CPU.  Personally I would like to see proper
support of 64-bit (n32, 64) ABI's as well.

So, that's for sometimes.

Thank you,
PM