From 1e371e7c9814f255325b42f1bb091e80b639d09b Mon Sep 17 00:00:00 2001
From: "B. Scott Michel" <scooter.phd@gmail.com>
Date: Mon, 10 Jul 2023 09:07:01 -0700
Subject: [PATCH] Memory sanitizer fixes

Fixes suggested by running the Clang memory santizer on the Github CI/CD
pipeline:

- sim_exp_check: Use calloc() to allocate and initialize the PCRE
  captured string offset vector. Also check the offsets to ensure they
  are captured substrings:

  -- If a substring wasn't captured, the offset pair is { -1, -1 }.
     (Of course, this would never happen in SIMH. :-)

  -- Remove the corresponding "_EXPECT_MATCH_GROUP_{pat}" variable
     from the environment when its substring isn't captured.  unsetenv()
     is locally implemented in scp.c, which sets the environment
     variable to an empty string; it isn't actually removed.

  Failure occurs in multiple tests.

- _sim_debug_write_flush: Ensure that debug_line_buf_last's underlying
  buffer is initialized when realloc-ed. (i650 failure.)

- vid_version: Initialize local variables "compiled" and "running". MSan
  claims that these aren't initialized. Reading the SDL  source, the
  patch level might not be initialized on some platforms. (multiple
  sim_video tests.)

NOTE: Do not attempt to run interactive memory-santized simulators on
Linux platforms where ncurses <= 6.2+20201114-2. MSan detects
uninitialized variables within ncurses; newer versions of ncurses fix
them.
---
 scp.c       | 26 +++++++++++++++++++-------
 sim_video.c |  2 +-
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/scp.c b/scp.c
index 7c735806..a8583c4a 100644
--- a/scp.c
+++ b/scp.c
@@ -13241,6 +13241,7 @@ for (i=0; i < exp->size; i++) {
     if (ep->switches & EXP_TYP_REGEX) {
 #if defined (USE_REGEX)
         int *ovector = NULL;
+        int ovector_elts;
         int rc;
         char *cbuf = (char *)exp->buf;
         static size_t sim_exp_match_sub_count = 0;
@@ -13259,27 +13260,38 @@ for (i=0; i < exp->size; i++) {
                 }
             }
         ++regex_checks;
-        ovector = (int *)malloc (3 * (ep->re_nsub + 1) * sizeof (*ovector));
+        ovector_elts = 3 * (ep->re_nsub + 1);
+        ovector = (int *)calloc ((size_t) ovector_elts, sizeof(*ovector));
         if (sim_deb && exp->dptr && (exp->dptr->dctrl & exp->dbit)) {
             char *estr = sim_encode_quoted_string (exp->buf, exp->buf_ins);
             sim_debug (exp->dbit, exp->dptr, "Checking String: %s\n", estr);
             sim_debug (exp->dbit, exp->dptr, "Against RegEx Match Rule: %s\n", ep->match_pattern);
             free (estr);
             }
-        rc = pcre_exec (ep->regex, NULL, cbuf, exp->buf_ins, 0, PCRE_NOTBOL, ovector, 3 * (ep->re_nsub + 1));
+        rc = pcre_exec (ep->regex, NULL, cbuf, exp->buf_ins, 0, PCRE_NOTBOL, ovector, ovector_elts);
         if (rc >= 0) {
             size_t j;
             char *buf = (char *)malloc (1 + exp->buf_ins);
 
             for (j=0; j < (size_t)rc; j++) {
                 char env_name[32];
+                int end_offs = ovector[2 * j + 1], start_offs = ovector[2 * j];
 
                 sprintf (env_name, "_EXPECT_MATCH_GROUP_%d", (int)j);
-                memcpy (buf, &cbuf[ovector[2 * j]], ovector[2 * j + 1] - ovector[2 * j]);
-                buf[ovector[2 * j + 1] - ovector[2 * j]] = '\0';
-                setenv (env_name, buf, 1);      /* Make the match and substrings available as environment variables */
-                sim_debug (exp->dbit, exp->dptr, "%s=%s\n", env_name, buf);
-                }
+                if (start_offs >= 0 && end_offs >= start_offs) {
+                    memcpy (buf, &cbuf[start_offs], end_offs - start_offs);
+                    buf[end_offs - start_offs] = '\0';
+                    setenv (env_name, buf, 1);      /* Make the match and substrings available as environment variables */
+                    sim_debug (exp->dbit, exp->dptr, "%s=%s\n", env_name, buf);
+                    }
+                else {
+                    /* Substring was not captured by regexp: remove from the environment
+                     * (unsetenv is local static -- doesn't actually remove the variable from
+                     * the environment, sets it to an empty string.) */
+                    sim_debug (exp->dbit, exp->dptr, "unsetenv %s\n", env_name);
+                    unsetenv(env_name);
+                    }
+                } 
             for (; j<sim_exp_match_sub_count; j++) {
                 char env_name[32];
 
diff --git a/sim_video.c b/sim_video.c
index d6220e57..05a8794c 100644
--- a/sim_video.c
+++ b/sim_video.c
@@ -2235,7 +2235,7 @@ return 0;
 const char *vid_version(void)
 {
 static char SDLVersion[160];
-SDL_version compiled, running;
+SDL_version compiled = { 0, }, running = { 0, };
 
 SDL_GetVersion(&running);