[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [microblaze-uclinux] [patch] errno corruption



Hi David,

At Thu, 18 Mar 2004 20:50:27 +1000,
David McCullough wrote:
> 
> 
> Jivin Yasushi SHOJI lays it down ...
> > At Thu, 18 Mar 2004 15:32:25 +0900,
> > I wrote:
> > [...]
> > > ok, here is my version of _sycall2
> > > 
> > > #define SYSCALL_CLOBBERS	"r3", "r4", "r11", "r12", "r17"
> > > 
> > > #define _syscall2(type, name, type1, arg1, type2, arg2)                 \
> > > type name (type1 arg1, type2 arg2)                                      \
> > > {                                                                       \
> > >         register long __r5 __asm__ ("r5") = arg1;                       \
> > >         register long __r6 __asm__ ("r6") = arg2;                       \
> 
> Originally I mentioned that you should avoid the above.  Here is the
> comment from the gcc info pages:

Sorry, I misread your comment.

>    Defining such a register variable does not reserve the register; it
>    remains available for other uses in places where flow control determines
>    the variable's value is not live.  However, these registers are made
>    unavailable for use in the reload pass; excessive use of this feature
>    leaves the compiler too few available registers to compile certain
>    functions.
> 
>    This option does not guarantee that GCC will generate code that has
>    this variable in the register you specify at all times.  You may not
>    code an explicit reference to this register in an `asm' statement and
>    assume it will always refer to this variable.

hmm... 

I hate to say this but most arch in 2.4 and 2.6 implements _syscall#
in this way.  is there any discussion about this in lkml?

if this cause hard-to-find bug, we should raise this in the list.

> The real trouble with unistd.h is that the syscalls are done as macros
> and not inline functions.  This means the can be included into the
> middle of some C code,  and at the point the optimiser may move your
> declarations away from the asm,  leaving the possibility for the value
> in the register to not be preserved and break your system call.

Sorry for my ignorance, but I thought _syscall# is helper macro to
generate actual syscall _functions_.

other use of these macro should be avoided.

> That said,  99% of archs get away with it.  m68k still does.  The
> trouble is the newer the compiler the more likely it is to break, and
> I have see this problem happen under m68k,  especially in code trying to
> save a few bytes on system calls.  When it happens,  expect a very long
> debug session and "Doh!" at the end ;-)

just to be curious, can you detail the problem, if possible? like the
version of gcc and how you used the macro.

> I know if may not be as efficient (m68k has the same problem) but I
> believe it is the only way to guarantee it is correct,

ok, here is the revised version of _syscall2 and chmod() which is
expanded from _syscall2:

#define _syscall2(type, name, type1, arg1, type2, arg2)                 \
type name (type1 arg1, type2 arg2)                                      \
{                                                                       \
        long __ret;                                                     \
        __asm__ __volatile__ ("addk	r5, r0, %2	\n\t"           \
                              "addk	r6, r0, %3	\n\t"           \
                              "bralid	r17, 0x8	\n\t"           \
                              "addik	r12, r0, %1	\n\t"           \
                              "addk	%0, r3, r0	\n\t"           \
                              : "=r" (__ret)                            \
                              : "i" (__NR_##name),                      \
                                "r" ((long)arg1),                       \
                                "r" ((long)arg2)                        \
                              : SYSCALL_CLOBBERS);                      \
        __syscall_return (type, __ret);                                 \
}



   2e790:       3021ffd8        addik   r1, r1, -40
   2e794:       fac10024        swi     r22, r1, 36
   2e798:       d9e00800        sw      r15, r0, r1
   2e79c:       10a02800        addk    r5, r0, r5
   2e7a0:       10c03000        addk    r6, r0, r6
   2e7a4:       ba3c0008        bralid  r17, 8
   2e7a8:       3180000f        addik   r12, r0, 15
   2e7ac:       12c30000        addk    r22, r3, r0
   2e7b0:       aa56ff82        xori    r18, r22, -126
   2e7b4:       bcb2000c        bgei    r18, 12         // 2e7c0
   2e7b8:       bcb60028        bgei    r22, 40         // 2e7e0
   2e7bc:       b800000c        bri     12              // 2e7c8
   2e7c0:       3656ff82        rsubik  r18, r22, -126
   2e7c4:       bcb2001c        bgei    r18, 28         // 2e7e0
   2e7c8:       b000ffff        imm     -1
   2e7cc:       b9f46500        brlid   r15, 25856      // 24ccc <__errno_location>
   2e7d0:       80000000        or      r0, r0, r0
   2e7d4:       14960000        rsubk   r4, r22, r0
   2e7d8:       d8801800        sw      r4, r0, r3
   2e7dc:       32c0ffff        addik   r22, r0, -1
   2e7e0:       10760000        addk    r3, r22, r0
   2e7e4:       eac10024        lwi     r22, r1, 36
   2e7e8:       c9e00800        lw      r15, r0, r1
   2e7ec:       b60f0008        rtsd    r15, 8
   2e7f0:       30210028        addik   r1, r1, 40


btw, I'm not using SYSCALL_ARG# macro in the patch, are you ok with
it, John?

patch appended.

regards,
--
          yashi

Attachment: unistd.h-1.3-explicit2.patch
Description: Binary data