Mysterious "value computed is not used" warnings on __atomic_exchange_n

Question

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.

The whole source file can be found [here](https://raw.githubusercontent.com/HorvathAkosPeter/pipenet/master/staging/ss.c). — peterh, Sep 06 '15 at 12:43
You should not use the atomic-builtins directly, but use C11 `stdatomic.h`. Why make your code depend on a compiler without actual need. — too honest for this site, Sep 06 '15 at 13:17
@Olaf I tried to use the possible lowest level, because of the lack of trust. But now that my code produces good asm, there is no more reason to not use the standard-compliant way, and this is what I will do. — peterh, Sep 06 '15 at 13:21
"lack of trust"... Well, as this is a header-only feature, you can easily verify this. So you verify all assember your compiler generates? What if the assembler is mapped to the wrong machine-codes? Where do you stop? — too honest for this site, Sep 06 '15 at 13:23
@Olaf Not all, only which does hardware-near things, for example atomic operations. Verify the asm of an atomic instruction is a very simple thing, and in my previous tries I've found multiple times that the atomic builtins produced suspicious code (f.e. if it reads the variable from the stack, does the swap in regs, and puts it back, it is already _not_ atomic). Another reason was that I wanted to see, how gcc does this. — peterh, Sep 06 '15 at 13:27
Depends on the memory order you specify and the architecture. Some architecture like ARM use a lock-free, non-blocking restart mechanism where you **do** use such operations. Not sure about x86. — too honest for this site, Sep 06 '15 at 13:36
@Olaf Another reason was that I simply didn't found the C11 stdatomic.h, only c++ things which I also didn't trust. Now I found, thank you! — peterh, Sep 06 '15 at 14:20

Ben Voigt · Accepted Answer · 2015-09-07T02:48:37.083

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"

You have right, this is really the only way to make the asm perfect... maybe I misunderstood the gcc builtin docs, maybe not. :-( — peterh, Sep 07 '15 at 10:26

peterh · Answer 2 · 2015-09-07T10:54:45.243

-1

@Olaf answered my question in a comment: although declare a dummy variable with __attribute__((unused)) and giving him the return values avoids the warnings, but there is also a much simpler way to do that: casting the functions to (void), so:

(void)__atomic_exchange_n(...);

Extension: although this solution also eliminates the warning, it still doesn't use the __atomic_exchange_n gcc builtin correctly (although sometimes it compiled correct code). The accepted answer solves finally the problem.

edited Sep 07 '15 at 10:54

answered Sep 07 '15 at 01:59

peterh

11,875
18
85
108

This doesn't solve the problem at all. Yes, it shows how to discard the result of the expression, but reading the entire function, it is clear that the result is needed. – Ben Voigt Sep 07 '15 at 02:47
@BenVoigt You have right, this is really the only way to make the asm perfect... maybe I misunderstood the gcc builtin docs, maybe not. :-( – peterh Sep 07 '15 at 10:13

Mysterious "value computed is not used" warnings on __atomic_exchange_n

2 Answers2