luxonis / XLink

A cross-platform library for communicating with devices over various physical links.
Apache License 2.0
11 stars 17 forks source link

mvLog header kills system `__attribute__()` macro; causes chaos and mass failures #67

Open diablodale opened 1 year ago

diablodale commented 1 year ago

I discovered that the XLinkLog.h header redefines the critical system macro __attribute__. The rule is...don't use, change, or redefine things (e.g. macros) with double underscores. Period no exceptions. https://devblogs.microsoft.com/oldnewthing/20230109-00/?p=107685

XLink is working around this issue by careful ordering of header includes. Not a good solution. One header that is broken by this issue is libusb.h. Other code/headers may be affected silently.

Setup

Repro

  1. Setup a Windows machine to compile with mingw
  2. Create any .cpp file in the XLink project, e.g. src\pc\protocols\myfile.cpp
  3. Edit your cpp file to have the following
    
    #define MVLOG_UNIT_NAME xLinkUsb

ifdef XLINK_LIBUSB_LOCAL

include

else

include <libusb-1.0/libusb.h>

endif

include "XLink/XLinkLog.h"

int myvar = 1;


4. config, build

Your build should be successful
Now...change the order so that your file has this code...
```c++
#define MVLOG_UNIT_NAME xLinkUsb
#include "XLink/XLinkLog.h"

#ifdef XLINK_LIBUSB_LOCAL
#include <libusb.h>
#else
#include <libusb-1.0/libusb.h>
#endif

int myvar = 1;

Result

uncountable number of errors, deep within the system headers, usually related to intrinsics. For example...

[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h: In function '__m64 _mm_cvtsi32_si64(int)':
[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h:79:54: error: cannot convert a vector of type '__vector(2) int' to type '__m64' {aka 'int'} which has different size
[build]    79 |   return (__m64) __builtin_ia32_vec_init_v2si (__i, 0);
[build]       |                                                      ^
[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h: In function 'int _mm_cvtsi64_si32(__m64)':
[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h:122:39: error: cannot convert '__m64' {aka 'int'} to '__vector(2) int'
[build]   122 |   return __builtin_ia32_vec_ext_v2si ((__v2si)__i, 0);
[build]       |                                       ^~~~~~~~~~~
[build]       |                                       |
[build]       |                                       __m64 {aka int}
[build] <built-in>: note:   initializing argument 1 of 'int __builtin_ia32_vec_ext_v2si(__vector(2) int, int)'
[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h: In function '__m64 _mm_packs_pi16(__m64, __m64)':
[build] C:/Users/WDAGUtilityAccount/Desktop/mingw64/lib/gcc/x86_64-w64-mingw32/12.2.0/include/mmintrin.h:161:43: error: cannot convert '__v4hi' {aka 'short int'} to '__vector(4) short int'
[build]   161 |   return (__m64) __builtin_ia32_packsswb ((__v4hi)__m1, (__v4hi)__m2);
[build]       |                                           ^~~~~~~~~~~~
[build]       |                                           |
[build]       |                                           __v4hi {aka short int}
[build] <built-in>: note:   initializing argument 1 of '__vector(8) char __builtin_ia32_packsswb(__vector(4) short int, __vector(4) short int)'

Notes

The bug is in include\XLink\XLinkLog.h. It redefines the system macro __attribute__. That is forbidden. And causes chaos like libusb header failures and potentially silent failures elsewhere. For example, in usb_host.cpp 4 local headers and 5+ system headers will no longer have a functioning __attribute__ macro.

https://github.com/luxonis/XLink/blob/d209b7eeb8ac68435df5097e57e1dfd8ea1d0e83/include/XLink/XLinkLog.h#L48-L53

diablodale commented 1 year ago

commit linked above. needs wider testing to see if it compiles everywhere including the share hardware itself.