ufrisk / MemProcFS

MemProcFS
GNU Affero General Public License v3.0
2.8k stars 352 forks source link

Linux C11 atomics compatibility #267

Closed ajkhoury closed 4 months ago

ajkhoury commented 4 months ago

When compiling with Clang targeting C11 or later, the following errors occur when compiling vmm/oscompatibility.c regarding the atomic operations:

/usr/bin/clang -DLINUX -DSQLITE_CORE -DSQLITE_THREADSAFE=2 -D_GNU_SOURCE -I./MemProcFS/vmm -I./MemProcFS/includes -g -fPIC -fvisibility=hidden -Wno-deprecated-declarations -g -O0 -Wextra -Wno-unused-parameter -Wno-cast-function-type -Wall -Wno-format-truncation -Wno-enum-compare -Wno-pointer-sign -Wno-multichar -Wno-unused-variable -Wno-unused-value -Wno-unused-but-set-variable -Wno-pointer-to-int-cast -Wno-int-to-pointer-cast -Wno-pointer-bool-conversion -std=gnu11 -MD -MT CMakeFiles/vmm.dir/contrib/MemProcFS/vmm/oscompatibility.c.o -MF CMakeFiles/vmm.dir/contrib/MemProcFS/vmm/oscompatibility.c.o.d -o CMakeFiles/vmm.dir/contrib/MemProcFS/vmm/oscompatibility.c.o -c /Source/MemProcFS/vmm/oscompatibility.c
/Source/MemProcFS/vmm/oscompatibility.c:454:8: error: address argument to atomic operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int *') invalid)
  454 |     if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
      |        ^                              ~~~~~~~~~~~~~~
/usr/lib/llvm-19/lib/clang/19/include/stdatomic.h:144:67: note: expanded from macro 'atomic_compare_exchange_strong'
  144 | #define atomic_compare_exchange_strong(object, expected, desired) __c11_atomic_compare_exchange_strong(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)
      |                                                                   ^                                    ~~~~~~
/Source/MemProcFS/vmm/oscompatibility.c:467:12: error: address argument to atomic operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int *') invalid)
  467 |         if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
      |            ^                              ~~~~~~~~~~~~~~
/usr/lib/llvm-19/lib/clang/19/include/stdatomic.h:144:67: note: expanded from macro 'atomic_compare_exchange_strong'
  144 | #define atomic_compare_exchange_strong(object, expected, desired) __c11_atomic_compare_exchange_strong(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)
      |                                                                   ^                                    ~~~~~~
/Source/MemProcFS/vmm/oscompatibility.c:482:12: error: address argument to atomic operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int *') invalid)
  482 |         if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
      |            ^                              ~~~~~~~~~~~~~~
/usr/lib/llvm-19/lib/clang/19/include/stdatomic.h:144:67: note: expanded from macro 'atomic_compare_exchange_strong'
  144 | #define atomic_compare_exchange_strong(object, expected, desired) __c11_atomic_compare_exchange_strong(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)
      |                                                                   ^                                    ~~~~~~
/Source/MemProcFS/vmm/oscompatibility.c:504:8: error: address argument to atomic operation must be a pointer to _Atomic type ('uint32_t *' (aka 'unsigned int *') invalid)
  504 |     if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwOne, 0)) {
      |        ^                              ~~~~~~~~~~~~~~
/usr/lib/llvm-19/lib/clang/19/include/stdatomic.h:144:67: note: expanded from macro 'atomic_compare_exchange_strong'
  144 | #define atomic_compare_exchange_strong(object, expected, desired) __c11_atomic_compare_exchange_strong(object, expected, desired, __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)
      |                                                                   ^                                    ~~~~~~
4 errors generated.

Please consider using or casting to a required _Atomic type, i.e. the atomic_uint type defined in stdatomic.h. Below is a quick patch:

diff --git a/vmm/oscompatibility.c b/vmm/oscompatibility.c
index ab23614..98cc42f 100644
--- a/vmm/oscompatibility.c
+++ b/vmm/oscompatibility.c
@@ -451,7 +451,7 @@ BOOL AcquireSRWLockExclusive_Try(_Inout_ PSRWLOCK SRWLock)
 {
     DWORD dwZero = 0;
     __sync_fetch_and_add_4(&SRWLock->c, 1);
-    if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
+    if(atomic_compare_exchange_strong((atomic_uint *)&SRWLock->xchg, &dwZero, 1)) {
         return TRUE;
     }
     __sync_sub_and_fetch_4(&SRWLock->c, 1);
@@ -464,7 +464,7 @@ VOID AcquireSRWLockExclusive(_Inout_ PSRWLOCK SRWLock)
     __sync_fetch_and_add_4(&SRWLock->c, 1);
     while(TRUE) {
         dwZero = 0;
-        if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
+        if(atomic_compare_exchange_strong((atomic_uint *)&SRWLock->xchg, &dwZero, 1)) {
             return;
         }
         futex(&SRWLock->xchg, FUTEX_WAIT, 1, NULL, NULL, 0);
@@ -479,7 +479,7 @@ BOOL AcquireSRWLockExclusive_Timeout(_Inout_ PSRWLOCK SRWLock, _In_ DWORD dwMill
     __sync_fetch_and_add_4(&SRWLock->c, 1);
     while(TRUE) {
         dwZero = 0;
-        if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwZero, 1)) {
+        if(atomic_compare_exchange_strong((atomic_uint *)&SRWLock->xchg, &dwZero, 1)) {
             return TRUE;
         }
         if((dwMilliseconds != 0) && (dwMilliseconds != 0xffffffff)) {
@@ -501,7 +501,7 @@ BOOL AcquireSRWLockExclusive_Timeout(_Inout_ PSRWLOCK SRWLock, _In_ DWORD dwMill
 VOID ReleaseSRWLockExclusive(_Inout_ PSRWLOCK SRWLock)
 {
     DWORD dwOne = 1;
-    if(atomic_compare_exchange_strong(&SRWLock->xchg, &dwOne, 0)) {
+    if(atomic_compare_exchange_strong((atomic_uint *)&SRWLock->xchg, &dwOne, 0)) {
         if(__sync_sub_and_fetch_4(&SRWLock->c, 1)) {
             futex(&SRWLock->xchg, FUTEX_WAKE, 1, NULL, NULL, 0);
         }
ufrisk commented 4 months ago

Many thanks for this. The project doesn't support clang right now, but I have plans to get it to work with it since I'm also interested in moving it over to mac. This will help in that work. Thank You :)

I'll look into it hopefully some time next week.

ajkhoury commented 4 months ago

This and the other issue are the only two changes I had to make to support clang :)

ufrisk commented 4 months ago

I'll see to it that it's added as soon as possible. For now I've updated the LeechCore with the atomics since I had another change in there anyway.

ufrisk commented 4 months ago

Can you check if the latest release just published fixes this issue?

ajkhoury commented 4 months ago

Yes all fixed, thanks!