
Betrayed by a bitfield (2012) - ingve
https://lwn.net/Articles/478657/
======
nohawp
It was fixed in 4.8
[https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080)

------
jacquesm
"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.

~~~
maguirre
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?

~~~
jacquesm
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.

------
revelation
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).

~~~
mschuster91
> 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.

[https://lwn.net/Articles/479402/](https://lwn.net/Articles/479402/)

~~~
mikeash
Right, but there's only one here. Is it just an attempt to future proof in
case they add a second?

~~~
arcticbull
Likely their standard practice so the addition of a second field is
immaterial. Does look funky though.

------
kazinator
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.

------
Const-me
I wonder why people still use Itanium CPUs? And Intel is still producing them,
they even going to release the next one, named Kittson?

Hasn’t x86-64 architecture basically won?

Or there’re some specific niches where Itaniums work better for some reason?

~~~
throwaway2048
the niche of "software written/licensed for itanium"

------
r1ch
Curious how this was solved, anyone have a link to a followup?

~~~
arcticbull
This was resolved in GCC 4.8 here:
[https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080)

"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.

------
pkaye
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.

------
cesarbs
Compiler and compilation-related bugs are quite interesting to come across,
since they can produce truly mind-boggling behavior.

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](https://github.com/dotnet/corefx/issues/8825)

------
jhallenworld
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.

~~~
jjnoakes
Volatile is never the answer if your question is about locking.

~~~
jhallenworld
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?

~~~
jjnoakes
Not only is it not enough, using volatile for locking primitives is a bad idea
and should not be done. It slows down the program with no benefits.

A correct multi-thrraded program is correct with or without volatile, and so
removing volatile can only speed things up and should be done.

------
Bromskloss
Shouldn't they file a bug report with the C standard instead of with GCC?

~~~
xorblurb
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.

~~~
Bromskloss
> 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.

------
RantyDave
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.

