Switch from old sync* built-ins to modern atomic* ones: `Undefined symbols for architecture ppc: "___sync_add_and_fetch_8"`

[barracuda156]

Current code is broken on 32-bit, since it uses old sync_* built-ins, not supported by libatomic:

/opt/local/bin/gcc-mp-14 -O2 -DSYSCONFDIR=\"/opt/local/etc\" -Wall -Wextra -Wnested-externs -Wformat=2 -g -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations -Wwrite-strings -Wshadow -Wpointer-arith -Wsign-compare -Wbad-function-cast -Wcast-align -Wdeclaration-after-statement -Wshadow -Wold-style-definition  -pipe -I/opt/local/libexec/openssl3/include -Os -arch ppc -pthread  -L/opt/local/libexec/openssl3/lib -Wl,-headerpad_max_install_names -L/opt/local/lib -lMacportsLegacySupport -arch ppc -o goaccess src/base64.o src/browsers.o src/color.o src/commons.o src/csv.o src/error.o src/gdashboard.o src/gdns.o src/gholder.o src/gkhash.o src/gkmhash.o src/gmenu.o src/goaccess.o src/gslist.o src/gstorage.o src/gwsocket.o src/json.o src/opesys.o src/options.o src/output.o src/parser.o src/persistence.o src/pdjson.o src/settings.o src/sort.o src/tpl.o src/ui.o src/util.o src/websocket.o src/xmalloc.o src/wsauth.o src/sha1.o   src/geoip2.o  -lncurses -lmaxminddb -lcrypto -lssl -lpthread -lintl -lintl 
Undefined symbols for architecture ppc:
  "___sync_add_and_fetch_8", referenced from:
      _count_process in gstorage.o
  "___sync_bool_compare_and_swap_8", referenced from:
      _parse_line in parser.o
ld: symbol(s) not found for architecture ppc
collect2: error: ld returned 1 exit status

While 32-bit platforms may not be a high concern, it is preferable to use modern atomic_* built-ins anyway.

[allinurl]

Thanks for reporting this. To confirm, gcc 14 and i386?

[barracuda156]

Thanks for reporting this. To confirm, gcc 14 and i386?

gcc 14 and ppc (ppc32). Version of gcc won’t make difference, since there is no hardware support for 8-byte atomics.

The same issue should be observed on arm and mips. No idea about i386.

[sertonix]

I am uncertain which memory model should be used but something like this should fix the issue (it at least compiled):

--- a/src/gkhash.c
+++ b/src/gkhash.c
@@ -1017,7 +1017,7 @@
     kh_val (hash, k) = 0;
   }

-  return __sync_add_and_fetch (&kh_val (hash, k), inc);
+  return __atomic_add_fetch (&kh_val (hash, k), inc, __ATOMIC_RELAXED);
 }

 /* Increase a uint64_t value given a string key.
@@ -1110,7 +1110,7 @@
     kh_val (hash, k) = 0;
   }

-  return __sync_add_and_fetch (&kh_val (hash, k), inc);
+  return __atomic_add_fetch (&kh_val (hash, k), inc, __ATOMIC_RELAXED);
 }

 /* Insert a string key and auto increment int value.
@@ -1202,7 +1202,7 @@
   k = kh_get (si32, hash, key);
   /* key found, return current value */
   if (k != kh_end (hash))
-    return __sync_add_and_fetch (&kh_val (hash, k), 0);
+    return __atomic_add_fetch (&kh_val (hash, k), 0, __ATOMIC_RELAXED);

   return 0;
 }
@@ -1300,7 +1300,7 @@
   k = kh_get (ii32, hash, key);
   /* key found, return current value */
   if (k != kh_end (hash))
-    return __sync_add_and_fetch (&kh_val (hash, k), 0);
+    return __atomic_add_fetch (&kh_val (hash, k), 0, __ATOMIC_RELAXED);

   return 0;
 }
--- a/src/gstorage.c
+++ b/src/gstorage.c
@@ -610,7 +610,7 @@
 /* Keep track of all valid and processed log strings. */
 void
 count_process (GLog *glog) {
-  __sync_add_and_fetch (&glog->processed, 1);
+  __atomic_add_fetch (&glog->processed, 1, __ATOMIC_RELAXED);
   lock_spinner ();
   ht_inc_cnt_overall ("total_requests", 1);
   unlock_spinner ();
--- a/src/parser.c
+++ b/src/parser.c
@@ -1195,7 +1195,7 @@
     if (tkn == bEnd || *bEnd != '\0' || errno == ERANGE)
       bandw = 0;
     logitem->resp_size = bandw;
-    __sync_bool_compare_and_swap (&conf.bandwidth, 0, 1); /* set flag */
+    __atomic_compare_exchange_n (&conf.bandwidth, &(int){0}, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); /* set flag */
     free (tkn);
     break;
     /* referrer */
@@ -1264,7 +1264,7 @@
     logitem->serve_time = (serve_secs > 0) ? serve_secs * MILS : 0;

     /* Determine if time-served data was stored on-disk. */
-    __sync_bool_compare_and_swap (&conf.serve_usecs, 0, 1); /* set flag */
+    __atomic_compare_exchange_n (&conf.serve_usecs, &(int){0}, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); /* set flag */
     free (tkn);
     break;
     /* time taken to serve the request, in seconds with a milliseconds
@@ -1287,7 +1287,7 @@
     logitem->serve_time = (serve_secs > 0) ? serve_secs * SECS : 0;

     /* Determine if time-served data was stored on-disk. */
-    __sync_bool_compare_and_swap (&conf.serve_usecs, 0, 1); /* set flag */
+    __atomic_compare_exchange_n (&conf.serve_usecs, &(int){0}, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); /* set flag */
     free (tkn);
     break;
     /* time taken to serve the request, in microseconds */
@@ -1304,7 +1304,7 @@
     logitem->serve_time = serve_time;

     /* Determine if time-served data was stored on-disk. */
-    __sync_bool_compare_and_swap (&conf.serve_usecs, 0, 1); /* set flag */
+    __atomic_compare_exchange_n (&conf.serve_usecs, &(int){0}, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); /* set flag */
     free (tkn);
     break;
     /* time taken to serve the request, in nanoseconds */
@@ -1323,7 +1323,7 @@
     logitem->serve_time = (serve_time > 0) ? serve_time / MILS : 0;

     /* Determine if time-served data was stored on-disk. */
-    __sync_bool_compare_and_swap (&conf.serve_usecs, 0, 1); /* set flag */
+    __atomic_compare_exchange_n (&conf.serve_usecs, &(int){0}, 1, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED); /* set flag */
     free (tkn);
     break;
     /* UMS: Krypto (TLS) "ECDHE-RSA-AES128-GCM-SHA256" */
@@ -1934,7 +1934,7 @@
   int64_t oldts = 0, newts = 0;
   /* atomic update loop */
   newts = mktime (&logitem->dt); // Get timestamp from logitem->dt
-  while (!__sync_bool_compare_and_swap (&glog->lp.ts, oldts, newts)) {
+  while (!__atomic_compare_exchange_n (&glog->lp.ts, &oldts, newts, false, __ATOMIC_RELAXED, __ATOMIC_RELAXED)) {
     oldts = glog->lp.ts; /* Reread glog->lp.ts if CAS failed */
     if (oldts >= newts) {
       break;    /* No need to update if oldts is already greater */

[allinurl]

@sertonix Thank you. Can you try building from upstream and see if that solves the issue from your end. Let me know. Thanks