Search code examples
cgccatomicgcc4.9reentrancy

Mysterious "value computed is not used" warnings on __atomic_exchange_n

I developed a reentrant function based on the atomic builtins of the gcc. Unfortunately, I get mysterious warnings about "computed but not used" values:

$ gcc -c -Wall ss.c
ss.c: In function ‘ss_wrapper’:
ss.c:87:3: warning: value computed is not used [-Wunused-value]
   __atomic_exchange_n(&ss_top, hit, __ATOMIC_SEQ_CST);
   ^
ss.c:91:5: warning: value computed is not used [-Wunused-value]
     __atomic_exchange_n(&ss_top, bkp->next, __ATOMIC_SEQ_CST); // release the lock, find out if there is new element
     ^

This is my function:

static void ss_wrapper(int signum, siginfo_t* siginfo, void *ucontext) {
  // currently top element on the signal stack
  static struct ss_hit* ss_top = NULL;


  struct ss_hit* hit = ss_newhit(signum, siginfo, (ucontext_t*)ucontext);
  struct ss_hit* bkp;

  again:

  bkp = hit;
  __atomic_exchange_n(&ss_top, hit, __ATOMIC_SEQ_CST);
  if (!hit) { // we got the lock, we are the master
    ss_fire(bkp);

    // release the lock, find out if there is new element
    __atomic_exchange_n(&ss_top, bkp->next, __ATOMIC_SEQ_CST);
    if (bkp->next) { // there IS

      hit = bkp;
      free(bkp);
      goto again;

    } else
      free(bkp);
  } else { // we didn't got the lock, but we got the top in hit
    __atomic_store(&hit->next, &bkp, __ATOMIC_SEQ_CST);
  }
}

Why does it happen? __atomic_exchange_n shouldn't compute anything, it only swaps the content of two variables.

Solution

  • it only swaps the content of two variables

    No, it does not. It swaps the content of one variable with the content of a register. The second variable is never changed. (Even without consulting the documentation, this is apparent from the different convention for passing the two parameters -- the memory address which is atomically exchanged is passed as a pointer, the other value is copied, not accessed in place)

    As a result of this misunderstanding, your logic is broken. You meant to do:

    hit = __atomic_exchange_n(&ss_top, hit, __ATOMIC_SEQ_CST);
if (!hit) { // we got the lock, we are the master

    which writes the new values of the register back into the second variable. The accesses to hit are non-atomic, but that's ok because it is a local variable, not shared with any other thread.

    Do not just throw away the return value, if you do, you will never enter the branch for "we are the master"