Random crash

[diorcety]

I'm currently having random crash (depending on which library is loaded or not), in the function gotoblas_init.
Valgrind detect an issue on

3600 »·······==7953== Syscall param sched_getaffinity(mask) points to unaddressable byte(s)
3601 ==7953==    at 0x5831F92: sched_getaffinity@@GLIBC_2.3.4 (sched_getaffinity.c:36)
3602 ==7953==    by 0x16EB6ED3: ??? (in /tmp/aa/lib/libopenblas.so.0.3)
3603 ==7953==  Address 0x8030 is not stack'd, malloc'd or (recently) free'd

I started to think to a false positive but when I looked to the code:

https://github.com/xianyi/OpenBLAS/blob/develop/driver/others/init.c#L893

In this IFDEF cpusetp is not initialized ... is it normal?

[diorcety]

@martin-frbg is it normal?

[brada4]

I think 878 879 has to be swapped and _FREE duplicated in both sides.

[brada4]

affinity code is off by default , it has other problems too - like once early at init affinity is set before main() and no other multithreading can escape that one CPU core.

[martin-frbg]

@diorcety yes this looks decidedly unhealthy, sorry. Probably one code "cleanup" too many and one test too few when I last tackled that. And no, moving the CPU_ALLOC up one line will not do as CPU_ALLOC was not available before glibc 2.7

[diorcety]

Thanks for the reply. Used with 0 as first argument we can use a single cpu_set_t allocation, no?

[diorcety]

even a stack allocation... I will try a fix tomorrow and check if it resolves my issue.

[martin-frbg]

I'd do something like this, but there may be a more elegant way ? (Note also that the number of arguments in the final CPU_COUNT() call is/was wrong in the original - using two arguments like for CPU_COUNT_S so this would probably not even have compiled on a glibc 2.6 system )

--- a/driver/others/init.c
+++ b/driver/others/init.c
@@ -857,7 +857,14 @@ void gotoblas_affinity_init(void) {
   common -> shmid = pshmid;
 
   if (common -> magic != SH_MAGIC) {
+
+#if defined(__GLIBC_PREREQ)
+#if __GLIBC_PREREQ(2, 7)
     cpu_set_t *cpusetp;
+#else
+    cpu_set_t cpuset;
+#endif
+#endif    
     int nums;
     int ret;
 
@@ -890,7 +897,7 @@ void gotoblas_affinity_init(void) {
     }
     CPU_FREE(cpusetp);
 #else
-    ret = sched_getaffinity(0,sizeof(cpu_set_t), cpusetp);
+    ret = sched_getaffinity(0,sizeof(cpu_set_t), &cpuset);
     if (ret!=0) {
         common->num_procs = nums;
     } else {
@@ -898,11 +905,11 @@ void gotoblas_affinity_init(void) {
     int i;
     int n = 0;
     for (i=0;i<nums;i++)
-        if (CPU_ISSET(i,cpusetp)) n++;
+        if (CPU_ISSET(i,&cpuset)) n++;
     common->num_procs = n;
     }
 #else
-    common->num_procs = CPU_COUNT(sizeof(cpu_set_t),cpusetp);
+    common->num_procs = CPU_COUNT(&cpuset);
     }
 #endif
 

[diorcety]

@martin-frbg I'm not blaming you, just to trying to understand if I'm not missing something. Yes I'm building against a old glibc (docker of a custom manylinux which is a centos 5.11 with glibc 2.5). That why I may triggered this issue.

[brada4]

Just do not enable non-default custom option and it works?

[martin-frbg]

@brada4 that is probably not too helpful... while removing the default NO_AFFINITY=1 option can cause problems (like locking every process launched from the same context onto the same core, as mentioned in the faq), there are probably some legitimate uses for it. The code should certainly compile and run successfully with either setting.

[diorcety]

I'm only using "-DBUILD_WITHOUT_LAPACK=ON -DUSE_OPENMP=ON -DTARGET=GENERIC" as cmake flags.
Your patch fixes my issue

[martin-frbg]

The default options in cmake actually differed from what is in the plain Makefiles. While a library with NO_AFFINITY not set will not automatically create problems, there are probably more drawbacks to having this as the default , e.g. #1591 (especially if one were to distribute the prebuilt library to a wider audience with varying applications). #2127 aligns the CMakelists.txt setting with Makefile.rule now.