bugmake - Bugs: bug #63315, loadapi test / user-defined...

 
 

bug #63315: loadapi test / user-defined function reload SEGV failures

Submitter:  Brad Smith <brad0>
Submitted:  Fri 04 Nov 2022 03:06:58 AM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.4 Operating System:  POSIX-Based
Fixed Release:  4.4.1 Triage Status:  Small Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 13 Nov 2022 02:49:33 PM UTC, comment #8: 

Me too, on OmniOS. Presumably Linux will map the shared object back into the same address as before, or has that just been luck? It's definitely not the case on all operating systems.

Please consider changing the title of this bug so it's easier to find by others hitting this.

Anonymous
Sun 13 Nov 2022 08:52:42 AM UTC, comment #7: 

I also hit this on illumos, and just reached the same conclusion - that the memory containing the function name in the hash table entry becomes unmapped when testapi.so is unloaded and loaded again. I'll apply the patch, thanks!

Anonymous
Sun 06 Nov 2022 07:54:45 PM UTC, comment #6: 

I'm not sure what the problem actually is here.  It's something weird about the shell; it's doing something strange with SIGTERM.  However I modified the helper app to be able to send TERM then replaced the test recipe "kill -TERM && sleep" with a reference to that and with these two changes on the OpenBSD system everything passes:


1213 Tests in 133 Categories Complete ... No Failures :-)


FYI:


diff --git a/tests/scripts/features/output-sync b/tests/scripts/features/output-sync
index 13a54ca0..40546994 100644
--- a/tests/scripts/features/output-sync
+++ b/tests/scripts/features/output-sync
@@ -360,7 +360,7 @@ use POSIX ();
 # file.
 run_make_test(q!
 pid:=$(shell echo $$PPID)
-all:; @kill -TERM $(pid) && sleep 16
+all:; @#HELPER# term $(pid) sleep 10
 !, '-O -j2', '/#MAKE#: \*\*\* \[#MAKEFILE#:3: all] Terminated/', POSIX::SIGTERM);
 }

diff --git a/tests/scripts/features/temp_stdin b/tests/scripts/features/temp_stdin
index b06df53e..c01d627c 100644
--- a/tests/scripts/features/temp_stdin
+++ b/tests/scripts/features/temp_stdin
@@ -71,7 +71,7 @@ run_make_test(q!
 include bye.mk
 pid:=$(shell echo $$PPID)
 all:;
-bye.mk: force; @kill -TERM $(pid) && sleep 16
+bye.mk: force; @#HELPER# term $(pid) sleep 10
 force:
 !, '-f-', '/#MAKE#: \*\*\* \[#MAKEFILE#:5: bye.mk] Terminated/', POSIX::SIGTERM);
 }
diff --git a/tests/thelp.pl b/tests/thelp.pl
index 993339cb..c243bcb8 100755
--- a/tests/thelp.pl
+++ b/tests/thelp.pl
@@ -16,6 +16,7 @@
 #  wait <word>  : wait for a file named <word> to exist
 #  tmout <secs> : Change the timeout for waiting.  Default is 4 seconds.
 #  sleep <secs> : Sleep for <secs> seconds then echo <secs>
+#  term <pid>   : send SIGTERM to PID <pid>
 #  fail <err>   : echo <err> to stdout then exit with error code err
 #
 # If given -q only the "out" command generates output.
@@ -95,6 +96,12 @@ sub op {
         return 1;
     }

+    if ($op eq 'term') {
+        print "term $nm\n";
+        kill('TERM', $nm);
+        return 1;
+    }
+
     if ($op eq 'fail') {
         print "fail $nm\n";
         exit($nm);


Paul D. Smith <psmith>
Group administrator
Sun 06 Nov 2022 02:14:45 PM UTC, comment #5: 

I was looking at it yesterday.  Both the kill / cleanup tests (temp_stdin and the one in output-sync) are failing because they are showing the exit code, not the Terminated output.

It's not a big problem because the temporary files are cleaned up properly, it's just that we get different output when make exits.

It's a bit strange so I'll look further today hopefully.

Paul D. Smith <psmith>
Group administrator
Sun 06 Nov 2022 03:57:14 AM UTC, comment #4: 

Thanks.

What about the temp_stdin test?

Brad Smith <brad0>
Sat 05 Nov 2022 04:59:39 PM UTC, comment #3: 

Thank you for the assist Brad; I have found the problem and will fix it in the next release.

FYI:


diff --git a/src/function.c b/src/function.c
index f0ef3434..893e583d 100644
--- a/src/function.c
+++ b/src/function.c
@@ -2801,7 +2801,7 @@ define_new_function (const floc *flocp, const char *name,
          _("Invalid maximum argument count (%u) for function %s"), max, name);

   ent = xmalloc (sizeof (struct function_table_entry));
-  ent->name = name;
+  ent->name = xstrdup (name);
   ent->len = (unsigned char) len;
   ent->minimum_args = (unsigned char) min;
   ent->maximum_args = (unsigned char) max;
@@ -2812,7 +2812,11 @@ define_new_function (const floc *flocp, const char *name,
   ent->fptr.alloc_func_ptr = func;

   ent = hash_insert (&function_table, ent);
-  free (ent);
+  if (ent)
+    {
+      free ((void*)ent->name);
+      free (ent);
+    }
 }

 void


Paul D. Smith <psmith>
Group administrator
Fri 04 Nov 2022 07:41:30 PM UTC, comment #2: 

comment #1:

> For the loadapi issues I'll need someone to debug that, or let me onto a system I can debug with.  There's clearly something different about the OpenBSD settings related to dynamic loading/unloading which is cause coredumps, because we don't see this on the Linux or MacOS platform.
>
> If your makefiles don't use this feature it should be OK.
>
> For the output-sync error that's just an issue with the test framework; I will fix it.  It's not a problem with GNU make itself.


I can give you access to a system to test with.

Brad Smith <brad0>
Fri 04 Nov 2022 07:29:04 PM UTC, comment #1: 

For the loadapi issues I'll need someone to debug that, or let me onto a system I can debug with.  There's clearly something different about the OpenBSD settings related to dynamic loading/unloading which is cause coredumps, because we don't see this on the Linux or MacOS platform.

If your makefiles don't use this feature it should be OK.

For the output-sync error that's just an issue with the test framework; I will fix it.  It's not a problem with GNU make itself.

Paul D. Smith <psmith>
Group administrator
Fri 04 Nov 2022 03:06:58 AM UTC, original submission:  

After updating to 4.4 we're now seeing some test failures where as with 4.3 everything passes fine.


features/loadapi ........................................ Error running /home/ports/pobj/gmake-4.4/build-amd64/tests/../make (expected 0; got 139)
/home/ports/pobj/gmake-4.4/build-amd64/tests/scripts/features/loadapi:163: /home/ports/pobj/gmake-4.4/build-amd64/tests/../make -f work/features/loadapi.mk.3

Caught signal 11!
Error running /home/ports/pobj/gmake-4.4/build-amd64/tests/../make (expected 0; got 139)
/home/ports/pobj/gmake-4.4/build-amd64/tests/scripts/features/loadapi:215: /home/ports/pobj/gmake-4.4/build-amd64/tests/../make -f work/features/loadapi.mk.7

Caught signal 11!
Error running /home/ports/pobj/gmake-4.4/build-amd64/tests/../make (expected 0; got 139)
/home/ports/pobj/gmake-4.4/build-amd64/tests/scripts/features/loadapi:163: /home/ports/pobj/gmake-4.4/build-amd64/tests/../make -f work/features/loadapi.mk.8

Caught signal 11!
Error running /home/ports/pobj/gmake-4.4/build-amd64/tests/../make (expected 0; got 139)
/home/ports/pobj/gmake-4.4/build-amd64/tests/scripts/features/loadapi:215: /home/ports/pobj/gmake-4.4/build-amd64/tests/../make -f work/features/loadapi.mk.12

Caught signal 11!
FAILED (9/13 passed)


features/output-sync ....................................
*** Test died (features/output-sync): Invalid [] range "t-s" in regex; marked by <-- HERE in m/make: /*/*/* /[work/features/output-s <-- HERE ync.mk.15:3: all] Terminated/ at /home/ports/pobj/gmake-4.4/build-amd64/tests/test_driver.pl line 979.

FAILED (15/16 passed)


features/temp_stdin ..................................... FAILED (6/7 passed)


Brad Smith <brad0>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by psmith (Posted a comment)
  • -email is unavailable- added by brad0 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-11-13 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4.1
        Triage StatusNone Small Effort
    2022-11-13 psmith Operating SystemNone POSIX-Based
        SummaryTest failures with 4.4 on OpenBSD loadapi test / user-defined function reload SEGV failures

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code