[Bug ports/11291] potential deadlock in sem_*wait and sem_post for MIPS architectures

["mischa dot jonker at viragelogic dot com" <sourceware-bugzilla at sourceware dot org>]

------- Additional Comments From mischa dot jonker at viragelogic dot com  2010-02-17 16:37 -------
It looks like the problem is not in the macro, the "m" (*mem) is actually there:

#define __arch_compare_and_exchange_xxx_32_int(mem, newval, oldval, rel, acq) \
     __asm__ __volatile__ (                                                   \
     ".set      push\n\t"                                                     \
     MIPS_PUSH_MIPS2                                                          \
     rel        "\n"                                                          \
     "1:\t"                                                                   \
     "ll        %0,%4\n\t"                                                    \
     "move      %1,$0\n\t"                                                    \
     "bne       %0,%2,2f\n\t"                                                 \
     "move      %1,%3\n\t"                                                    \
     "sc        %1,%4\n\t"                                                    \
     "beqz      %1,1b\n"                                                      \
     acq        "\n\t"                                                        \
     ".set      pop\n"                                                        \
     "2:\n\t"                                                                 \
              : "=&r" (__prev), "=&r" (__cmp)                                 \
              : "r" (oldval), "r" (newval), "m" (*mem)                        \
              : "memory")


If we look at the disassembled output of gcc, we see that in below loop (inside __new_sem_post), the "cur = isem->value" 
is taken out of the loop, as isem->value does not seem to change. 

The relevant code from sem_post:
 do
    {
      cur = isem->value; // <-- taken out of the loop
      if (isem->value == SEM_VALUE_MAX) //<--- taken out of the loop as well
        {
          __set_errno (EOVERFLOW);
          return -1;
        }
    }
  while (atomic_compare_and_exchange_bool_acq (&isem->value, cur + 1, cur));

And here the disassembly, with explanation:
0000e530 <sem_post>:
    e530:       3c1c0002        lui     gp,0x2
    e534:       279ceb00        addiu   gp,gp,-5376
    e538:       0399e021        addu    gp,gp,t9
    e53c:       8c860000        lw      a2,0(a0)        // a2 (=cur) = isem->value
    e540:       3c037fff        lui     v1,0x7fff
    e544:       3462ffff        ori     v0,v1,0xffff    // v0 = SEM_VALUE_MAX
    e548:       14c20007        bne     a2,v0,e568 <sem_post+0x38>  //check for overflow
    e54c:       24c50001        addiu   a1,a2,1         // a1 = cur + 1 (note: branch delay slot!!)

<-- code to return error in case of overflow
    e550:       8f84833c        lw      a0,-31940(gp)
    e554:       7c03e83b        rdhwr   v1,$29
    e558:       00831021        addu    v0,a0,v1
    e55c:       2404ffff        li      a0,-1
    e560:       10000021        b       e5e8 <sem_post+0xb8>
    e564:       2403004f        li      v1,79 -->

// Execution continues here after overflow check. Please note that the ll/sc combination
// is used, which is in fact atomic. Furthermore, it fetches 0(a0) again, which is isem->value.
    e568:       c0830000        ll      v1,0(a0)        

// And here is the problem!! It compares the just fetched value with a2, which is initialised
// at the beginning of the loop, but never loaded again. The fact that 'll' is used for the load
// above (instead of lw), makes me believe that the atomic_compare_and_exchange_bool_acq macro
// is in fact correct (the ll belongs to the macro, since it's a special instruction for
// atomic operations and not generated by gcc). 
    e56c:       14660006        bne     v1,a2,e588 <sem_post+0x58>
    e570:       00003821        move    a3,zero
    e574:       00a03821        move    a3,a1
    e578:       e0870000        sc      a3,0(a0)
    e57c:       10e0fffa        beqz    a3,e568 <sem_post+0x38>
    e580:       00000000        nop
    e584:       0000000f        sync
    ....



-- 


http://sourceware.org/bugzilla/show_bug.cgi?id=11291

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.