Fix potential overflow in decrement when computing GC_markers_m1

Also, call WARN if a non-positive value is specified in GC_MARKERS.

* pthread_support.c [PARALLEL_MARK] (GC_thr_init): Replace markers_m1
local variable with markers one; keep real number of markers in
"markers" variable (not a decremented one); treat invalid (i.e.
non-positive) markers value (obtained from GC_MARKERS environment
variable) the same way as too big ones (i.e. set to maximum number of
markers in this case); adjust WARN message accordingly; report invalid
or too big markers value in WARN.
* win32_threads.c [PARALLEL_MARK] (GC_thr_init): Likewise.
* pthread_support.c [PARALLEL_MARK] (GC_thr_init): Adjust code
indentation.

diff --git a/pthread_support.c b/pthread_support.c
index fe89790..702fe25 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -1175,30 +1175,31 @@ GC_INNER void GC_thr_init(void)
       available_markers_m1 = 0; /* but use only one marker */
 #   endif
   } else {
-#  ifdef PARALLEL_MARK
-     {
-       char * markers_string = GETENV("GC_MARKERS");
-       int markers_m1;
-
-       if (markers_string != NULL) {
-         markers_m1 = atoi(markers_string) - 1;
-         if (markers_m1 >= MAX_MARKERS) {
-           WARN("Limiting number of mark threads\n", 0);
-           markers_m1 = MAX_MARKERS - 1;
-         }
-       } else {
-         markers_m1 = GC_nprocs - 1;
-#        ifdef GC_MIN_MARKERS
-           /* This is primarily for targets without getenv().   */
-           if (markers_m1 < GC_MIN_MARKERS - 1)
-             markers_m1 = GC_MIN_MARKERS - 1;
-#        endif
-         if (markers_m1 >= MAX_MARKERS)
-           markers_m1 = MAX_MARKERS - 1; /* silently limit the value */
-       }
-       available_markers_m1 = markers_m1;
-     }
-#  endif
+#   ifdef PARALLEL_MARK
+      {
+        char * markers_string = GETENV("GC_MARKERS");
+        int markers;
+
+        if (markers_string != NULL) {
+          markers = atoi(markers_string);
+          if (markers <= 0 || markers > MAX_MARKERS) {
+            WARN("Too big or invalid number of mark threads: %" WARN_PRIdPTR
+                 "; using maximum threads\n", (signed_word)markers);
+            markers = MAX_MARKERS;
+          }
+        } else {
+          markers = GC_nprocs;
+#         ifdef GC_MIN_MARKERS
+            /* This is primarily for targets without getenv().  */
+            if (markers < GC_MIN_MARKERS)
+              markers = GC_MIN_MARKERS;
+#         endif
+          if (markers > MAX_MARKERS)
+            markers = MAX_MARKERS; /* silently limit the value */
+        }
+        available_markers_m1 = markers - 1;
+      }
+#   endif
   }
   GC_COND_LOG_PRINTF("Number of processors = %d\n", GC_nprocs);
 # ifdef PARALLEL_MARK

diff --git a/win32_threads.c b/win32_threads.c
index 8a4f7ca..124f4de 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -2413,19 +2413,20 @@ GC_INNER void GC_thr_init(void)
 # if defined(PARALLEL_MARK)
     {
       char * markers_string = GETENV("GC_MARKERS");
-      int markers_m1;
+      int markers;
 
       if (markers_string != NULL) {
-        markers_m1 = atoi(markers_string) - 1;
-        if (markers_m1 >= MAX_MARKERS) {
-          WARN("Limiting number of mark threads\n", 0);
-          markers_m1 = MAX_MARKERS - 1;
+        markers = atoi(markers_string);
+        if (markers <= 0 || markers > MAX_MARKERS) {
+          WARN("Too big or invalid number of mark threads: %" WARN_PRIdPTR
+               "; using maximum threads\n", (signed_word)markers);
+          markers = MAX_MARKERS;
         }
       } else {
 #       ifdef MSWINCE
           /* There is no GetProcessAffinityMask() in WinCE.     */
           /* GC_sysinfo is already initialized.                 */
-          markers_m1 = (int)GC_sysinfo.dwNumberOfProcessors - 1;
+          markers = (int)GC_sysinfo.dwNumberOfProcessors;
 #       else
 #         ifdef _WIN64
             DWORD_PTR procMask = 0;
@@ -2442,17 +2443,17 @@ GC_INNER void GC_thr_init(void)
               ncpu++;
             } while ((procMask &= procMask - 1) != 0);
           }
-          markers_m1 = ncpu - 1;
+          markers = ncpu;
 #       endif
 #       ifdef GC_MIN_MARKERS
           /* This is primarily for testing on systems without getenv(). */
-          if (markers_m1 < GC_MIN_MARKERS - 1)
-            markers_m1 = GC_MIN_MARKERS - 1;
+          if (markers < GC_MIN_MARKERS)
+            markers = GC_MIN_MARKERS;
 #       endif
-        if (markers_m1 >= MAX_MARKERS)
-          markers_m1 = MAX_MARKERS - 1; /* silently limit the value */
+        if (markers > MAX_MARKERS)
+          markers = MAX_MARKERS; /* silently limit the value */
       }
-      available_markers_m1 = markers_m1;
+      available_markers_m1 = markers - 1;
     }
 
     /* Check whether parallel mode could be enabled.    */