Critical section ringbuffer_t.read

[Ghostbird]

src/ringbuffer.c:195–202 read the value of the read index, and update it. This operation is not atomic:

if (rbptr->read != (uint32_t)upcast_read)
{
  /* Overwrite happened. */
  fprintf(stderr, "Buffer overwrite detected.");
  return NULL;
}
/* Update read index. */
rbptr->read = (uint32_t)((upcast_read + count) % rbptr->size);

src/ringbuffer.c:223–334 read the value of the read index, and update it. This operation is not atomic:

  if (rbptr->write == rbptr->read)
    rbptr->read += WRITE_DATACOUNT;

[AlixAbbasi]

Ops. Thats a major issue. Why we did not talk about it today? I just saw it now.

EDIT: Now I remember that we agreed that for now accept the possibility of race condition. But let this issue open so in future we might be able to address it.

[Ghostbird]

I have thought about this somewhat more, and the race condition does not really break anything. All systems continue to work, and no assumptions are broken if the race condition happens, and the read wins. However, we lose up to one entire buffer of data, because the read thinks that it has caught up to the write, even though in reality it is one entire buffer behind. In practice this means that our coverage of the application is reduced, but nothing is violated. This means that as long the application still provides useful coverage, we need not implement any explicit solutions (So far I haven't found any explicit solutions that do not violate the real-time requirements.)

So far I haven't been able to think of a scenario where other issues can occur. However, that does not mean they don't exist.

It would be really interesting to get a test version working and let it check a 100% CFG. Then we can evaluate how much of the CFG it can cover under different loads.

[AlixAbbasi]

Yeap, but with the last discussion we had you were assuming it will corrupt the data since we are not reading bits but bytes. Obviously it is not going the stop the read function, it just cause a false positive. Almost all existing CFIs are violating the real time requirements (both soft and hard real-time).

Regarding the testing I am working on its assembly now. We then move to generate a graph probably with GCC at first step.

[Ghostbird]

What I meant was that it probably does not corrupt data. I don't know what you mean by reading bytes instead of bits.