Re:Betrayed by a bitfield (2012)
by jacquesm on Sunday July 03, 2016 @12:03PM
(#12026447)
"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.
Re:Betrayed by a bitfield (2012)
by maguirre on Sunday July 03, 2016 @12:07PM
(#12026458)
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?
Re:Betrayed by a bitfield (2012)
by jacquesm on Sunday July 03, 2016 @12:15PM
(#12026491)
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.
Re:Betrayed by a bitfield (2012)
by revelation on Sunday July 03, 2016 @12:54PM
(#12026667)
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).
Re:Betrayed by a bitfield (2012)
by mschuster91 on Sunday July 03, 2016 @01:17PM
(#12026773)
> 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.
Re:Betrayed by a bitfield (2012)
by CamperBob2 on Sunday July 03, 2016 @06:33PM
(#12027933)
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).
Re:Betrayed by a bitfield (2012)
by ambrop7 on Sunday July 03, 2016 @01:49PM
(#12026909)
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.
Re:Betrayed by a bitfield (2012)
by kazinator on Sunday July 03, 2016 @01:56PM
(#12026946)
This looks like a compiler problem.
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.
Re:Betrayed by a bitfield (2012)
by pkaye on Sunday July 03, 2016 @02:29PM
(#12027075)
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.
Re:Betrayed by a bitfield (2012)
by jhallenworld on Sunday July 03, 2016 @08:42PM
(#12028341)
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.
Re:Betrayed by a bitfield (2012)
by jhallenworld on Monday July 04, 2016 @09:06AM
(#12030481)
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?
Re:Betrayed by a bitfield (2012)
by xorblurb on Sunday July 03, 2016 @04:17PM
(#12027442)
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.
Re:Betrayed by a bitfield (2012)
by Bromskloss on Sunday July 03, 2016 @05:08PM
(#12027621)
> It is not allowed by C11.
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.
Re:Betrayed by a bitfield (2012)
by RantyDave on Monday July 04, 2016 @02:02AM
(#12029092)
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.
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 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).
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.
https://lwn.net/Articles/479402/
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).
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.
Hasn’t x86-64 architecture basically won?
Or there’re some specific niches where Itaniums work better for some reason?
"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.
A while back we ran into this one in the .NET jitter while working on ASP.NET Core: https://github.com/dotnet/corefx/issues/8825
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?
A correct multi-thrraded program is correct with or without volatile, and so removing volatile can only speed things up and should be done.
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.
2k-30+70 weirds things.