The bundled bullet code fails to compile with strict-aliasing violations

[eli-schwartz]

Relevant: #5

Essentially bulletphysics/bullet3#4590

tl;dr

Gentoo is trying to get all distro packages to be LTO-clean, and as part of that we are trying to compile with -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing to check for cases where the compiler can try to optimize based on assuming UB cannot happen, and miscompile code that has UB in it. strict-aliasing issues are always bad but LTO can make them even worse.

We already track bullet as an issue -- and we have a workaround, in that we can tell the compiler "well just don't optimize this code, it's too risky". The problem here is that since bullet is part of the supertuxcart source build, we can't just build supertuxcart with optimizations, but link to an unoptimized bullet (until bullet is fixed).

You should probably track the bullet ticket, but it might also be good to somehow consider unbundling bullet if possible.

I have no idea if you still need a patched copy -- it would be great if that need is obsolete. The other reason mentioned in #5 was about using the correct version of bullet:

Updating bullet to a newer version is a major undertaking, since (in the past) the physics behaviour can change drastically from one version to the next (which is why bullet recommends not to use a system library).

I think it would be quite reasonable honestly to just... require an exact version (or range of versions) of bullet that are known to work, and allow people to configure against a system copy of that and otherwise force them to use the bundled copy.

I think "it changes drastically from version to version" is overall a weak argument against the use of system libraries.

...

Reported downstream at https://bugs.gentoo.org/458894

[qwertychouskie]

Un-bundling Bullet is not going to happen. Even the smallest differences between versions can create massive de-synchronizations in networked multiplayer.

Although upgrading Bullet is technically possible between 1.5 (the last planned release in the 1.x series) and 2.0, it is extraordinarily unlikely to happen, given how much it could affect the feel of the game, and how much validation (playtesting, network testing across a variety of platforms and architectures, etc) would be required. Such a major undertaking would only happen if a newer Bullet provided some feature or improvement considered important to how the game plays, not just to enable a small compile optimization.

Patches to make the bundled Bullet LTO-clean could be merged if provided, but only if they can be proven to not affect physics in any way.

[Alayan-stk-2]

I think it would be quite reasonable honestly to just... require an exact version (or range of versions) of bullet that are known to work, and allow people to configure against a system copy of that and otherwise force them to use the bundled copy.

I think "it changes drastically from version to version" is overall a weak argument against the use of system libraries.

It is an extremely strong argument.

The point of system libraries is twofold:

• Save some disk space from duplication.
• Ensure the library can be updated in a timely fashion even if the software making use of it is not actively maintained. This matters mostly for libraries that can present security risks.

That's it.

With modern computers, the disk space argument has become worthless in most situations, being only relevant for big libraries used by a lot of software. It is certainly worthless when it comes to bullet.

As for the ease of update, it is completely negated in situations where software behavior requires a specific version of the library, and using an updated version is at risk of breaking it. This is very much the case here.

But using system libraries opens us to:

• People using incompatible versions, having their game broken and a bad experience, and us wasting our time when we receive related bug reports until it comes out that a system library has been used and caused the issue.
• Issues when different software need different versions of the same library (I don't know if handling this has gotten better, but this used to be a real issue)
• Making future compilation of past versions much more difficult. Bundling libraries is very good for the preservation of software history, whereas system libraries are quite bad in this regard. Right now, old STK versions don't compile properly because of sysctl.h being deprecated, but that's very easy to fix compared to not having the right library version available.

SuperTuxKart comes with bundled libraries precisely because of all the serious bugs that were reported over the years from people using system libraries. There are also some custom changes that were made by our previous maintainer after the decision to bundle bullet.

I looked at the PR meant to fix the strict-aliasing violations, and only this commit appears directly applicable: bulletphysics/bullet3@fd2ec79

Other commits are either not applicable at all or not applicable directly because some of the surrounding code changed.

But there might be other strict-aliasing errors in our bundled lib that don't exist in more recent versions of bullet.

[eli-schwartz]

The point of system libraries is twofold:

• Save some disk space from duplication.

• Ensure the library can be updated in a timely fashion even if the software making use of it is not actively maintained. This matters mostly for libraries that can present security risks.

3. Ensure the library can be bugfixed once and fixed everywhere.
4. If the library is particularly large, make the applications using it spend a lot less time compiling unchanged code after updates to the application.
5. Save some network space from duplication.

...

Point 3 is a generalization of your sysctl.h comment.

With modern computers, the disk space argument has become worthless in most situations, being only relevant for big libraries used by a lot of software. It is certainly worthless when it comes to bullet.

Your point of view here is not universally held. However, it's also something that doesn't even slightly apply when considering network bandwidth utilization, a thing that has NOT scaled over the years as well as disk space.

[eli-schwartz]

Either way, I will be very happy to see fixes for the aliasing issue. :)

[Alayan-stk-2]

Either way, I will be very happy to see fixes for the aliasing issue. :)

I merged the commit referenced above as it doesn't cause any compatibility concerns, but I would assume there are some strict-aliasing violations remaining.

Do you have a list of all the strict-aliasing violations present in our version?