"The chances of good things resulting from this sequence of events are quite small."
That's putting it very mildly.
In a way they were lucky, in this context there was a possible fix, if you're assembling a packet to be sent across the network then you won't be able to solve it that easily.
For more fun like this look up all the caveats around packed structs.
I took your advice and went looking for it. I came away with the feeling that some people think they are useful some people thin they are evil. Did you have something specific in mind that you thought packed structures should be avoided for?
Packed structures seem to bring out compiler bugs like not much else does, no idea why but they have caused me many nights of lost sleep over the years to the point where if I see packed structures, there is a bug and the data does not get sent over the network my first instinct is to disable the packing to see if the problem goes away.
Eh, that use of a bitfield is asking for it. What could you possibly gain from using a single bitfield in the entire struct?
I also thought it was well known that using bitfields you are just pushing the burden of generating the bit shifting code on the compiler, and that implies accessing other fields. I don't think there is a processor out there that has single bit writes for general purpose memory (some do for config registers).
> Eh, that use of a bitfield is asking for it. What could you possibly gain from using a single bitfield in the entire struct?
A (buried way down) comment explains this:
> if you can even replace two int variables with one bitfield, you may shrink your datastructure enough to have it fit within a CPU cache line. The memory access that can then be avoided can be enough to make a noticeable difference.
if you can even replace two int variables with one bitfield, you may shrink your datastructure enough to have it fit within a CPU cache line.
But the way you should do that is by declaring an integer variable of the exact size you anticipate requiring to store your bits, and set/clear/test those bits explicitly with Boolean operators.
C bitfield semantics have always been far too hand-wavy to use for anything important. This is a perfect example (not that a separate variable will necessarily save you from a compiler stupid enough to write to adjacent structure members for no reason).
I think the very reasonable expectation is that you can only have issues with respect to accessing different parts of adjacent bitfield members, since in those cases the compiler generally really needs to access other members of the structure.
If you turn that bit-field into a plain unsigned int, the problem goes away, right?
If an unsigned int can be accessed all by itself without involving the adjacent lock.
This strongly suggests that the compiler should be using "units" for bit-field allocation which are no larger than unsigned int.
It's odd for for a bit-field derived from, the type unsigned int to cause an access which is wider than unsigned int.
Also, nowhere in the standard does it say that the bit-field "units" can be shared with adjacent non-members. In C99 the wording is, "If enough space remains, a bit-field that immediately follows another bit-field in a structure shall be packed into adjacent bits of the same unit." I.e. unit sharing takes place when a bit-field follows a bit-field. I can find no other text which specifies or permits unit sharing between a non-bit-field and a following bit-field.
"Here is a candidate patch (it ICEs one Ada testcase, see PR52134). It enforces
the C++11 memory model (as far as bitfields are concerned) upon everyone and
builds on the machinery that was implemented for it (changing the implementation
for when we compute "an underlying object" and permanently store it, to make
it possible to use this information for optimization purposes)."
If I'm not mistaken they could have ensured 64-bit alignment when a bitfield immediately follows a lock, although that likely wouldn't have been fun.
Bitfields seem to be a perpetual problem. We've used them a lot in firmware on ARM processors. Most compilers (even x86) generate inefficient code sequences compared to what I can do in assembly though Clang/llvm seems to be the best. The ARM official compilers had some bitfield bugs in some particular releases that caused us headaches.
Packing should be fine.. I think the real bug is that the lock is not being handled properly. It should be marked with volatile, and this should signal the compiler that it can not be part of a read-modify-write to an unrelated item.
Well you are right in the sense that volatile should not be used in place of locking primitives, and that even when implementing locking primitives, it's not enough (you need memory barriers, probably in an assembly language snippet market as __volatile__).
That being said, if the lock variable itself if marked as volatile, the compiler should take the hint based on its simple definition "value could change at any time", and not try to combine in with unrelated read-modify-write accesses.
Otherwise why wouldn't you want the compiler to speed up or compress bitfields?
The argument that it was once a bug of the C standard makes absolutely no sense. It is not allowed by C11. C99 does not talk about that because C99 does not officially support multithreading, but that has not prevented largely C99 compliant compilers from supporting multithreading, while this kind of "optimization" intrinsically prevents multithreading.
So the situation is just that, at one point, "optimizations" have been added by people who did not know what the where doing, and tried to hide, as usual for modern compiler authors, behind the literal wording of the C standard to justify their incompetence and the risks they force on the world.
I see. Does GCC not support C11, or does it behave differently in this matter depending on whether it is in C11 mode or not?
> tried to hide, as usual for modern compiler authors, behind the literal wording of the C standard to justify their incompetence and the risks they force on the world.
I think that it's even worse to rely upon something that is not guaranteed by the letter of the standard.
Ummm, if it's going to be hit by more than one thread, you should lock it. I don't see how this is GCC's fault, wait till you learn about write barriers.