Discussion of mzbz#1533969 Fix build error with newer glibc

[bgstack15]

From https://bugzilla.mozilla.org/show_bug.cgi?id=1533969, here is my successful patch for Pale Moon.
patch on my gitlab

The affected files in Pale Moon (not sure about all UXP) are:

• tools/profiler/tasktracer/GeckoTaskTracer.cpp
• js/src/jsnativestack.cpp

Code: Select all

Source: https://bugzilla.mozilla.org/show_bug.cgi?id=1533969
From: Emilio Cobos Álvarez (:emilio) https://bugzilla.mozilla.org/user_profile?user_id=546716
Date: 2019-03-12 15:08 PDT
Modified for Pale Moon by: B Stack <bgstack15@gmail.com>
This patch is applicable for all versions of Fedora GNU/Linux, but it really only affects gcc >= 9.1.1 in fc == 31
--- tools/profiler/tasktracer/GeckoTaskTracer.cpp.orig	2019-06-04 18:16:01.000000000 -0400
+++ tools/profiler/tasktracer/GeckoTaskTracer.cpp	2019-06-13 13:16:29.160643732 -0400
@@ -20,19 +20,19 @@
 
 #include <stdarg.h>
 
-// We need a definition of gettid(), but glibc doesn't provide a
+// We need a definition of gettid(), but old glibc versions don't provide a
 // wrapper for it.
 #if defined(__GLIBC__)
 #include <unistd.h>
 #include <sys/syscall.h>
-static inline pid_t gettid()
+#define gettid() static_cast<pid_t>(syscall(SYS_gettid))
 {
   return (pid_t) syscall(SYS_gettid);
 }
 #elif defined(XP_MACOSX)
 #include <unistd.h>
 #include <sys/syscall.h>
-static inline pid_t gettid()
+#define gettid() static_cast<pid_t>(syscall(SYS_thread_selfid))
 {
   return (pid_t) syscall(SYS_thread_selfid);
 }
--- js/src/jsnativestack.cpp.orig	2019-06-04 18:16:01.000000000 -0400
+++ js/src/jsnativestack.cpp	2019-06-14 08:13:04.080567510 -0400
@@ -26,11 +26,7 @@
 #  include <sys/syscall.h>
 #  include <sys/types.h>
 #  include <unistd.h>
-static pid_t
-gettid()
-{
-    return syscall(__NR_gettid);
-}
+#  define gettid() static_cast<pid_t>(syscall(SYS_gettid))
 # endif
 
 #else

If this looks suitable, would you like me to submit a merge request to UXP on github?

[vannilla]

Does it actually compile?
A very cursory look tells me there are some syntax errors, but I'm not a c++ expert so there might be some subtleties.
Also, wouldn't it be better to conditionally compile this new code so that it's used only when a newer glibc is used? I don't know what's Moonchild's policy on this matter is, but, you know, better safe than sorry.

[Moonchild]

Your patch is a bad port of the Mozilla changeset. As-is it's certainly not acceptable.
I'd be more comfortable if Travis looks at this; I'm not familiar enough with these defines to make a proper call on if this would cause issues on gcc <9 and/or other versions of glibc/O.S.es

[trava90]

Can GCC 9 compile Pale Moon without this patch? Does GCC <9 still compile Pale Moon with this patch? What version(s) of glibc are we talking about and how prevalent is it? How would this affect non Fedora systems?

I'd also feel better if this code was conditionally compiled depending on the version of GCC used and version of glibc found (at the very least until it could be better tested with older versions).

[bgstack15]

My description is incorrect in the above post. It is related to the glibc version and not the gcc version. Yes, gcc < 9.1.1 were able to compile Pale Moon without this patch.

I don't have the skills for providing nifty #defines for just Fedora and just Fedora >= 31. I guess this patch could be targeted at sufficiently high versions of glibc, which is a #ifdef GLIBC_228 or something, right?

Thank you for reading this. I don't need to upstream this or anything. I just thought it might be helpful because eventually other platforms will have newer glibc versions that are affected.

[trava90]

My description is incorrect in the above post. It is related to the glibc version and not the gcc version.

Ok, in that case my question becomes how does this affect systems with older versions of glibc?

I guess this patch could be targeted at sufficiently high versions of glibc, which is a #ifdef GLIBC_228 or something, right?

This would be the better option. Search through the code for other glibc conditionals as a reference for the #ifdef.