#881 XSPICE output queue and failed DC operating point

[ronnymilleo]

Hello there, I'm building a new user interface (https://github.com/ronnymilleo/imcsim) to work with electronic circuits and I found two leaks with LeakSanitizer while using ngspice 47 as a shared library. Both are still in master. The attached repro.c shows each one (build with: cc -g -fsanitize=address repro.c -o repro -lngspice). I used AI to help me find out where the leaks are coming from and here is the result:

1. remcirc leaks the values of pending XSPICE output events

When EVTsetup_queues() in src/xspice/evt/evtsetup.c clears the output queue, it frees each event but not its value. EVTdest() and Evt_purge_free_outputs() free both. remcirc calls this code through EVTunsetup(), so a transient with digital parts leaks a few bytes per pending event. "./repro events" runs an inverter and then remcirc.

--- a/src/xspice/evt/evtsetup.c
+++ b/src/xspice/evt/evtsetup.c
@@ -252,6 +252,7 @@
while(output_event) {
ptr = output_event;
output_event = output_event->next;

• FREE(((Evt_Output_Event_t *) ptr)->value);
  FREE(ptr);
  }

• A failed operating point does not close its plot

In src/spicelib/analysis/dcop.c, DCop() opens a plot with OUTpBeginPlot(), but returns without OUTendPlot() when the operating point fails ("DC solution failed"), so the run descriptor leaks (about 1 KB here). "./repro dcop" runs an SR latch of d_nand gates with both inputs held high, whose operating point fails.

--- a/src/spicelib/analysis/dcop.c
+++ b/src/spicelib/analysis/dcop.c
@@ -81,6 +81,7 @@
if(converged != 0) {
fprintf(stdout,"\nDC solution failed -\n");
CKTncDump(ckt);

• SPfrontEnd->OUTendPlot (plot);
  return(converged);
  }

With both changes applied to ngspice 47, both repro cases and the test suite of my application (437 test cases, run under AddressSanitizer) show no leaks.

[ronnymilleo]

The diffs in the description lost their formatting:

--- a/src/xspice/evt/evtsetup.c
+++ b/src/xspice/evt/evtsetup.c
@@ -252,6 +252,7 @@
         while(output_event) {
             ptr = output_event;
             output_event = output_event->next;

+            FREE(((Evt_Output_Event_t *) ptr)->value);
             FREE(ptr);
         }
         output_queue->head[i] = NULL;
--- a/src/spicelib/analysis/dcop.c
+++ b/src/spicelib/analysis/dcop.c
@@ -81,6 +81,7 @@
     if(converged != 0) {
         fprintf(stdout,"\nDC solution failed -\n");
         CKTncDump(ckt);
+        SPfrontEnd->OUTendPlot (plot);
         return(converged);
     }

[Holger Vogt]

Thanks for the patch, which seems to be reasonable.

There are several other places where CKTncDump(ckt); is used without being followed by SPfrontEnd->OUTendPlot (plot);

Could you hae a look at these?

[ronnymilleo]

yes sure

[ronnymilleo]

I checked the other places (line numbers from pre-master-48, 940bb50fa).

Of the nine CKTncDump() calls, four leak: the operating point that is recomputed with 'hertz' inside the frequency loop (acan.c:287, noisean.c:405, span.c:717) and the return after dcpss.c:3667. The others are before OUTpBeginPlot() (acan.c:141, noisean.c:203, span.c:472), already end the plot (dctran.c:316), or are the one from my first report (dcop.c:83).

Since the leak comes from returning between OUTpBeginPlot() and OUTendPlot(), I also read every return between them in these analyses. That found the same problem in DCtrCurv(), which has no CKTncDump(), and in other error exits of the AC and SP loops.

The attached patch replaces the one from my first report, so it still has the dcop.c and evtsetup.c changes. It is against pre-master-48 and does not apply to master as is (dcpss.c differs there). It changes:

• dctrcurv.c: ends the plot on the four "return(converged)" in the sweep loop.
• acan.c: ends the plot on the 'hertz' operating point, CKTload(), NIacIter() and CKTacDump() errors.
• noisean.c: on the 'hertz' operating point and CKTload() errors, calls CKTnoise(N_CLOSE) and frees data.
• span.c: does the endsweep cleanup on every error exit in the frequency loop.
• dcpss.c: does the "PSS analysis aborted" cleanup when the operating point fails, and frees RHS_derivative and pred,
  which no exit freed, not even the successful one.
• dcop.c and evtsetup.c: unchanged from my first report.

I reproduced each case with LeakSanitizer (shared library with --enable-xspice --enable-cider). Each one leaks without the patch and none of them do with it. The other span.c exits, and NIacIter() and CKTacDump() in acan.c, are fixed from reading the code only. repro.c loads the netlist, runs the commands, then "destroy all" and "remcirc". DCtrCurv(), ACan(), NOISEan() and SPan() keep the plot in a static pointer, so the leak only shows on the next run:

./repro snap_sweep.cir "dc i1 0 2m 0.5m" "dc i1 0 2m 0.5m"
./repro latch.cir "dc v1 5 0 -1" "dc v1 5 0 -1"
./repro snap_op.cir "pss 1k 1m a 128 10"
./repro latch.cir "op"
./repro events.cir "tran 100n 30u"

second=("destroy all" "remcirc" "circbyline second" "circbyline V1 a 0 dc 0 ac 1" "circbyline R1 a 0 1k"
        "circbyline .end")
./repro hertz.cir "ac lin 4 1 2000" "${second[@]}" "ac lin 2 1 10"
./repro hertz.cir "noise v(a) i1 lin 4 1 2000" "${second[@]}" "noise v(a) v1 lin 2 1 10"
second[3]="circbyline V1 a 0 dc 0 ac 1 portnum 1 z0 50"
./repro hertz_sp.cir "sp lin 4 1 40000" "${second[@]}" "sp lin 2 1 10"

The failing circuits use a B source that pulls 10 mA when V(a) > 1 V, so there is no operating point once the input goes over 1 mA. In hertz*.cir, another B source drives hertz * 1u, so the operating point fails only at higher frequencies.

I added the previous diff into this attached patch as well.