Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUILD][UNIX] Build fails with strict-aliasing violations #458

Open
eli-schwartz opened this issue Mar 5, 2024 · 3 comments
Open

[BUILD][UNIX] Build fails with strict-aliasing violations #458

eli-schwartz opened this issue Mar 5, 2024 · 3 comments
Labels
compatibility? reported as compatibility issue; triage pending OS: Unix specific to POV-Ray for Unix

Comments

@eli-schwartz
Copy link

Summary

Tried to build povray with *FLAGS: -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing

The following error resulted:

x86_64-pc-linux-gnu-g++ -DHAVE_CONFIG_H -I. -I..  -I.. -I../source/backend -I../source/base -I../source/frontend -I../unix -I../vfe -I../vfe/unix -I/usr/include/SDL -D_GNU_SOURCE=1 -D_REENTRANT -I/usr/lib64/../include  -DPOVLIBDIR=\"/usr/share/povray\" -DPOVCONFDIR=\"/etc/povray\" -pthread -I/usr/include  -I/usr/include  -Wno-multichar -Wno-write-strings -fno-enforce-eh-specs -march=native -fstack-protector-all -O2 -pipe -fdiagnostics-color=always -frecord-gcc-switches -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-clash-protection -flto=4 -Werror=odr -Werror=lto-type-mismatch -Werror=strict-aliasing -Wformat -Werror=format-security -pthread -c -o base/povms.o base/povms.cpp
In file included from base/configbase.h:151,
                 from base/povms.cpp:42:
base/povms.cpp: In function ‘void POVMSStream_Init()’:
./base/povms.h:97:53: error: dereferencing type-punned pointer will break strict-aliasing rules [-Werror=strict-aliasing]
   97 |                 #define HexToPOVMSIEEEFloat(h, f) *((int *)(&f)) = h
      |                                                    ~^~~~~~~~~~~~
base/povms.cpp:1038:9: note: in expansion of macro ‘HexToPOVMSIEEEFloat’
 1038 |         HexToPOVMSIEEEFloat(0x44663355, data_ieeefloat); // 0x44663355 equals 920.802063
      |         ^~~~~~~~~~~~~~~~~~~
cc1plus: some warnings being treated as errors

POV-Ray Version

  • Incarnation: POV-Ray for Unix
  • Affected source version: 3.7.0.0

Build Environment

  • Operating system: Gentoo Linux
  • Hardware architecture: amd64
  • Compiler: GCC 13
@eli-schwartz eli-schwartz added compatibility? reported as compatibility issue; triage pending OS: Unix specific to POV-Ray for Unix labels Mar 5, 2024
@wfpokorny
Copy link
Contributor

Thank you for reporting the issue.

I'm just a user with my own fork. Plus, my version of the messaging system is passing around doubles rather than floats but, that said, I think changing the code to that below a valid solution for official versions of POV-Ray.

//  See: https://github.com/POV-Ray/povray/issues/458
//  for why HexToPOVMSIEEEFloat macro replaced with code below.
//  HexToPOVMSIEEEFloat(0x44663355u, data_ieeefloat);

    static_assert(sizeof(POVMSIEEEFloat) == 4);
    const uint32_t ByOrdTest = 0x44663355u;
    union uC {
       uint32_t buffer;
       float    value;
    } u;
    u.buffer       = ByOrdTest;
    data_ieeefloat = u.value;

Strictly, this is still type-punning, but gcc / g++ guarantee the through a union form works properly.

Other rambling. It looks to me like the code here builds up methods to unscramble bytes arbitrary scrambled. Today, on a single machine this very likely only big endian vs little endian differences which can be tested for in other ways. The messaging code was set up to unscramble what might be happening with multiple machines of differing types as well as any scrambling due network transmission. POV-Ray on the whole was never completely fleshed out to run in this many machines across a network manner - as far as I know. Do we still need the arbitrary byte unscrambling?

@tribad
Copy link

tribad commented Mar 17, 2024

Hi,
you can use memcpy for that.

If the optimization level is somewhat higher, I checked it with -O2, it will be reduced to an assignment.

#include <stdio.h>
#include <memory.h>
#include <stdint.h>

int main(void) {
   int32_t i =0xdeadbeaf;
   float   f;

   memcpy(&f, &i, sizeof(f));
   printf("%f\n", f);
   return 0;
}

will be optimized to :

0000000000000000 <main>:
   0:	48 83 ec 08          	sub    $0x8,%rsp
   4:	48 8d 3d 00 00 00 00 	lea    0x0(%rip),%rdi        # b <main+0xb>
   b:	b8 01 00 00 00       	mov    $0x1,%eax
  10:	f2 0f 10 05 00 00 00 	movsd  0x0(%rip),%xmm0        # 18 <main+0x18>
  17:	00 
  18:	e8 00 00 00 00       	call   1d <main+0x1d>
  1d:	31 c0                	xor    %eax,%eax
  1f:	48 83 c4 08          	add    $0x8,%rsp
  23:	c3                   	ret

At offset

  • 0 and 4 you have the stackframe creation.
  • b the copy operation.
  • 10,17,18 is the printf call
  • 1d the return value creation
  • 1f stackframe cleanup
  • 23 return.

Only a hint.
But write a comment that it will be optimized, if you let the compiler do so.

@eli-schwartz
Copy link
Author

Yes, both unions and memcpy are the officially sanctioned way to do this without invoking UB. Both will be optimized by the compiler to the exact machine code that aliasing violations were hoping to produce.

I will be honest: I mostly submitted this bug report out of a sense of obligation and for tracking purposes. Given recent activity I did not have high hopes of it being fixed. :D Still, stranger things have happened... the original authors might come back...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility? reported as compatibility issue; triage pending OS: Unix specific to POV-Ray for Unix
Projects
None yet
Development

No branches or pull requests

3 participants