Fix -Wstrict-aliasing

[SoapGentoo]

• Casting a uint64_t* to double* invokes undefined behavior, since it violates the strict aliasing rules of ISO C. Instead of casting pointers, let's read through a union which is supported by C and yields the same performant assembly code.

Closes: https://bugs.gentoo.org/924864

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.37%. Comparing base (8380a9d) to head (808c083).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2528   +/-   ##
=======================================
  Coverage   84.37%   84.37%           
=======================================
  Files         375      376    +1     
  Lines       61595    61633   +38     
  Branches    12052    12060    +8     
=======================================
+ Hits        51970    52004   +34     
- Misses       9625     9629    +4     

Files	Coverage Δ
src/random/random.c	87.22% <100.00%> (+0.01%)	⬆️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b57427c...808c083. Read the comment docs.

[szhorvat]

I'll note that recent versions of MSVC generate different code for this version on x64 than the plan pointer cast. There's one extra instruction, and as far as I can tell it's writing the value to the main memory and reading it back instead of using only registers. https://godbolt.org/z/WEWPsc3cK

A similar difference persists with the actual igraph use case as well.

@SoapGentoo Can you comment on the difference between the two versions? Is there any reason why the current version wouldn't work?

[SoapGentoo]

The current code invokes undefined behavior, and just because it hasn't failed catastrophically yet, doesn't mean you should continue using it. Correct, MSVC seems to be storing it to the stack and loading it back from the stack, mainly because its codegen is terrible. I strongly doubt this will have an impact on runtime performance, since modern CPUs partially virtualize the stack anyways.

If you want a simple example how violating strict aliasing breaks code, just look at https://godbolt.org/z/rE4sW5K53. bar() return 0.0 even though the last store to the pointer wrote (uint64_t)1 to the memory location, and the compiler never read it back from memory, since casting double* to uint64_t* and dereferencing the pointer invokes UB. People think UB is some kind of esoteric concept, and code that works is "fine" since it has been tested. Invoking UB is never fine, and this code will break at some point in the future.

[eli-schwartz]

Correct, MSVC seems to be storing it to the stack and loading it back from the stack, mainly because its codegen is terrible. I strongly doubt this will have an impact on runtime performance, since modern CPUs partially virtualize the stack anyways.

Also mingw GCC and mingw clang both exist, and both don't have this problem...