brianwatling / libfiber

A User Space Threading Library Supporting Multi-Core Systems
MIT License
137 stars 32 forks source link

Compilation failure because of union alignment #4

Closed ayekat closed 5 years ago

ayekat commented 5 years ago

Hello,

I am not sure if this was introduced with some update of gcc at some point, but it fails to compile on 3 occasions with alignment 1 of ‘union <anonymous>’ is less than 16.

The following patch makes the compilation pass again, however I am not sure if it breaks something else down the road:

--- a/include/dist_fifo.h
+++ b/include/dist_fifo.h
@@ -49,7 +49,7 @@ typedef struct dist_fifo_pointer

 typedef union
 {
-    dist_fifo_pointer_t pointer;
+    dist_fifo_pointer_t pointer __attribute__((__aligned__(2 * sizeof(void *))));
     pointer_pair_t blob;
 } __attribute__ ((__packed__)) dist_fifo_pointer_wrapper_t;

diff --git a/include/fiber_signal.h b/include/fiber_signal.h
index aedfb37..a494db9 100644
--- a/include/fiber_signal.h
+++ b/include/fiber_signal.h
@@ -96,7 +96,7 @@ typedef union fiber_multi_signal
     struct {
         uintptr_t volatile counter;
         mpsc_fifo_node_t* volatile head;
-    } data;
+    } data __attribute__ ((__aligned__(2 * sizeof(void *))));
     pointer_pair_t blob;
 } __attribute__ ((__packed__)) fiber_multi_signal_t;

diff --git a/include/mpmc_lifo.h b/include/mpmc_lifo.h
index e881ae0..1754970 100644
--- a/include/mpmc_lifo.h
+++ b/include/mpmc_lifo.h
@@ -28,7 +28,7 @@ typedef union
     struct {
         uintptr_t volatile counter;
         mpmc_lifo_node_t* volatile head;
-    } data;
+    } data __attribute__ ((__aligned__(2 * sizeof(void *))));
     pointer_pair_t blob;
 } __attribute__ ((__packed__)) mpmc_lifo_t;
brianwatling commented 5 years ago

Hi Tinu, Apologies for the slow response and thanks for reporting the issue. Could give me more details about which version of gcc you're using and the full error message including line numbers? I cloned and built a fresh copy with no problems on Ubuntu 18.04 and gcc 7.3.0. Your suggested fix looks fine so long as the size of the unions don't change. Ideally the various assert()'s on struct sizes would be converted to compile time asserts to prevent such a problem.

ayekat commented 5 years ago

Hi Brian, thanks for replying (and sorry for that missing info)

It's under GCC 8.2.1+20180831-1, on Arch Linux.

Your suggested fix looks fine so long as the size of the unions don't change.

Yes, simply copied how the other structures (e.g. pointer_pair_t) are defined in the codebase. But I admit I am a bit surprised (and confused) that gcc would not deduce the alignment automatically (given that it should know that dist_fifo_pointer_t is a structure of two pointers).

brianwatling commented 5 years ago

Fixed by https://github.com/brianwatling/libfiber/commit/111db17c675bac1e8ab024cb444715036828c97d

ayekat commented 5 years ago

Thank you very much!