mozjs17 @17.0.0_5 won't build (on PPC Tiger, Mac OS X 10.4.11) because sys/sysctl.h does not provide _SC_NPROCESSORS_ONLN but HW_NCPU

[ballapete (Peter "Pete" Dyballa)]

jsgc.cpp
/opt/local/bin/g++-apple-4.2 -o jsgc.o -c  -I./dist/system_wrappers_js -include ./config/gcc_hidden.h -DIMPL_MFBT -DEXPORT_JS_API -DNO_NSPR_10_SUPPORT -DUSE_ZLIB -I./../../mfbt/double-conversion -I. -I. -I./dist/include  -I/opt/local/include/nspr      -I. -I./assembler -I./yarr  -fPIC -I/opt/local/include -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wempty-body -Wno-ctor-dtor-privacy -Wno-overlength-strings -Wno-invalid-offsetof -Wno-variadic-macros -Wcast-align -pipe -Os -arch ppc -fno-common -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -pthread -pipe  -DNDEBUG -DTRIMMED -g -O3 -fno-stack-protector -fomit-frame-pointer -DUSE_SYSTEM_MALLOC=1 -DENABLE_ASSEMBLER=1  -I/opt/local/include -DMOZILLA_CLIENT -include ./js-confdefs.h -MD -MF .deps/jsgc.o.pp /opt/local/var/macports/build/_opt_local_var_macports_sources_lil.fr.rsync.macports.org_release_tarballs_ports_lang_mozjs17/mozjs17/work/mozjs17.0.0/js/src/jsgc.cpp
/opt/local/var/macports/build/_opt_local_var_macports_sources_lil.fr.rsync.macports.org_release_tarballs_ports_lang_mozjs17/mozjs17/work/mozjs17.0.0/js/src/jsgc.cpp: In function 'unsigned int js::GetCPUCount()':
/opt/local/var/macports/build/_opt_local_var_macports_sources_lil.fr.rsync.macports.org_release_tarballs_ports_lang_mozjs17/mozjs17/work/mozjs17.0.0/js/src/jsgc.cpp:2868: error: '_SC_NPROCESSORS_ONLN' was not declared in this scope
gmake[1]: *** [config/rules.mk:1019: jsgc.o] Error 1
gmake[1]: Leaving directory '/opt/local/var/macports/build/_opt_local_var_macports_sources_lil.fr.rsync.macports.org_release_tarballs_ports_lang_mozjs17/mozjs17/work/mozjs17.0.0/js/src'

The bug is here:

 2858	static unsigned
 2859	GetCPUCount()
 2860	{
 2861	    static unsigned ncpus = 0;
 2862	    if (ncpus == 0) {
 2863	# ifdef XP_WIN
 2864	        SYSTEM_INFO sysinfo;
 2865	        GetSystemInfo(&sysinfo);
 2866	        ncpus = unsigned(sysinfo.dwNumberOfProcessors);
 2867	# else
 2868	        long n = sysconf(_SC_NPROCESSORS_ONLN);
 2869	        ncpus = (n > 0) ? unsigned(n) : 1;
 2870	# endif
 2871	    }
 2872	    return ncpus;
 2873	}
 2874	#endif /* JS_THREADSAFE */

[ballapete (Peter "Pete" Dyballa)]

main.log from PPC Tiger with failure

[ballapete (Peter "Pete" Dyballa)]

Changing the block to

static unsigned
GetCPUCount()
{
    static unsigned ncpus = 0;
    if (ncpus == 0) {
# ifdef XP_WIN
        SYSTEM_INFO sysinfo;
        GetSystemInfo(&sysinfo);
        ncpus = unsigned(sysinfo.dwNumberOfProcessors);
# elif __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 1050
        long n = sysconf(HW_NCPU);
        ncpus = (n > 0) ? unsigned(n) : 1;
# else
        long n = sysconf(_SC_NPROCESSORS_ONLN);
        ncpus = (n > 0) ? unsigned(n) : 1;
# endif
    }
    return ncpus;
}
#endif /* JS_THREADSAFE */

should work – but sys/sysctl.h is never included! Maybe sys/sysctl.h could be included here:

   96	#ifdef XP_WIN
   97	# include "jswin.h"
   98	#else
   99	# include <unistd.h>
  100	#endif

[ballapete (Peter "Pete" Dyballa)]

Mac OS X 10.5.8, Leopard, also has HW_NCPU instead of _SC_NPROCESSORS_ONLN – in sys/sysctl.h, but if has #define _SC_NPROCESSORS_ONLN 5 in unistd.h. So it must be something like:

#ifdef XP_WIN
# include "jswin.h"
#else
# include <unistd.h>
# if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ < 1050
# include <sys/sysctl.h>
# endif
#endif

The attached patch, patch_js_src_jsgc.cpp.diff, allows compilation of js/src/jsgc.cpp.

[ballapete (Peter "Pete" Dyballa)]

Proposed patch for Tiger to allow determination of # of CPUs

[kencu (Ken)]

Good work .. see also <​https://github.com/macports/macports-ports/commit/4678a5ef46617d3d8ba1ab4f82c7fe1182f9f9df>

[ballapete (Peter "Pete" Dyballa)]

Replying to kencu:

Good work .. see also <​https://github.com/macports/macports-ports/commit/4678a5ef46617d3d8ba1ab4f82c7fe1182f9f9df>

OK! So your recommendation is to first include AvailabilityMacros.h (or Availability.h) in order to have a means to determine the macOS or Mac OS X version which is independently defined of the C compiler used? (Because some C compilers might not define that macro?) Of course a "globally" valid patch should start with #ifdef __APPLE__.

So a good patch should look like this?

#ifdef __APPLE__                                                        # this block only for Macs
# ifndef __MAC_OS_X_VERSION_MIN_REQUIRED                                # are AvailabilityMacros.h or Availability.h not yet included?
#  if __ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__ >= 1050             # then for Leopard and later…
#   include <Availability.h>                                            # …either include this…
#  else
#   include <AvailabilityMacros.h>                                      # …or include that
#  endif
# endif
# if __MAC_OS_X_VERSION_MIN_REQUIRED < 1050                             # and for some OS versions do this…

/* whatever */

# else                                                                  # …for others do that

/* whatever else*/

# endif                                                                 # finish OS version discrimination
#endif                                                                  # finish Apple case

It would mean some effort if someone else would need to perform the quality control *I* put into my work…

[ryandesign (Ryan Carsten Schmidt)]

We had this issue reported before in #46567; not sure what happened.

[kencu (Ken)]

Replying to ballapete:

It would mean some effort if someone else would need to perform the quality control *I* put into my work…

Indeed so. Just noticed this was copied to Dave Evans, and remembered that this was how he'd fixed the exact same problem in a different port six months ago, so figured he might want to fix it similarly again.

As you know, there are a certain set of almost identical recurring issues on 10.4, 10.5, and 10.6 that keep coming up over and over in ports. We might as well fix them similarly -- almost should make a cheat sheet for them.

[kencu (Ken)]

I tried sysconf(HW_NCPU) on my Tiger machine with 2 processors, but sysconf(HW_NCPU) returns 100 processors, so although it does compile, it does not return proper values, which is no surprise because sysconf does not understand HW_NCPU as a selector to return the number of CPUs, even though it does compile it. Previously, most people who fixed this function for Tiger just returned 1 processor and forgot about it, which is practical enough, if inaccurate. But I had a few minutes today, so I came up with something that might be useful.

let's say you have a generic program using the sysconf function which fails on Tiger, like this:

#include <unistd.h>
#include <stdio.h>

int main() {
   printf("number of cpus %d \n\n", (int)sysconf(_SC_NPROCESSORS_ONLN));
} 

A simple enough replacement function, which returns a long (as sysconf does) but with the proper number of processors and also defines _SC_NPROCESSORS_ONLN to fix the define error on Tiger, makes the editing reasonably easily:

/*  Tiger replacement function */
#include <sys/sysctl.h>
#define _SC_NPROCESSORS_ONLN 58

long tigersysconf(int name){
  if (name == _SC_NPROCESSORS_ONLN) {
    int nm[2];
    size_t len = 4;
    uint32_t count;
		
    nm[0] = CTL_HW; nm[1] = HW_AVAILCPU;
    sysctl(nm, 2, &count, &len, NULL, 0);
		
    if (count < 1) {
      nm[1] = HW_NCPU;
      sysctl(nm, 2, &count, &len, NULL, 0);
      if (count < 1) { count = 1; }
    }
    return (long)count;
  }
  return -1;
} /*  end of Tiger replacement function */

so you would paste that Tiger replacement function up near the top of the source file, and change the function call from this

sysconf(_SC_NPROCESSORS_ONLN)

to this - just one single simple edit in the source:

tigersysconf(_SC_NPROCESSORS_ONLN)

and this works correctly.

tigerg5:~/testnumproc cunningh$ gcc -o testsysnumproc testsysnumproc.c
tigerg5:~/testnumproc cunningh$ ./testsysnumproc
number of cpus 2 

This could either be set up as a simple patch to be included only on Tiger, or wrapped up in #ifdef __APPLE__ etc preprocessor guards for permanent inclusion in the source file. I didn't paste that version in here, as it looks like spaghetti code with all the ifdefs, and I didn't want to distract from how simple this fix really is.

I just used this on a fix to repair a broken build of gcc6 on Tiger Intel i386, and it seems to work OK.

[kencu (Ken)]

Although this extra function is pretty simple, it would be quite elegant to just override the sysconf function on Tiger for certain selectors, with a function that falls through to the library sysconf. That would make this really quite transparent.

<​https://github.com/rentzsch/mach_override>. <​https://papers.put.as/papers/macosx/2003/overridingMacOSX.pdf><​http://stackoverflow.com/questions/617554/override-a-function-call-in-c>

[ballapete (Peter "Pete" Dyballa)]

It built here.

[kencu (Ken)]

I have since incorporated this functionality into LegacySupport so this is transparently handled now on Tiger.

It'll "just work" and you will never even be aware it was fixed for you.

[mascguy (Christopher Nielsen)]

Replying to kencu:

I have since incorporated this functionality into LegacySupport so this is transparently handled now on Tiger.

It'll "just work" and you will never even be aware it was fixed for you.

Beautiful, closing as fixed.