Betrayed by a bitfield (2012) | Hacker News Reader

Betrayed by a bitfield (2012)

lwn.net

62 pointsingve26 comments

It was fixed in 4.8 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52080

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

https://lwn.net/Articles/479402/

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

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

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.

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.

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?

the niche of "software written/licensed for itanium"

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

This was resolved in GCC 4.8 here: 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.

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.

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

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.

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

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?

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.

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

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.

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

> C11. C99

2k-30+70 weirds things.

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.

Reply on news.ycombinator.com