From bf79131f40c77ae771490ae2d0fed6a442273889 Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Wed, 8 Jul 2020 23:23:19 +0100
Subject: [PATCH] Use vmstate for memory leak regress tests (re: ad9a9219)

'ps' does not always give reliable results; on macOS, 'ps' appears
to produce nondeterministic (i.e. randomly varying) results for
'vsz' and 'rss', making it unusable for memory leak tests. See:
https://github.com/ksh93/ksh/pull/64#issuecomment-655094931
and further comments.

So let's compile in the vmstate builtin so that we can make sure to
measure things properly. It also reports bytes instead of 1024-byte
blocks, so smaller leaks can be detected.

To be decided: whether or not to disable the vmstate builtin for
release builds in order to save about 12K in the ksh binary.

src/cmd/ksh93/data/builtins.c:
- Add vmstate to the list of builtins that are compiled in.

src/cmd/ksh93/tests/leaks.sh:
- getmem(): get size using: vmstate --format='%(busy_size)u'
  (Using busy_size instead of size seems to make more sense as it
  excludes freed blocks. See vmstate --man)
- Introduce a $unit variable for reporting leaks and set it to
  'bytes'; this makes it easier to change the unit in future.
- Since the tests are now more sensitive, initialise all variables
  before use to avoid false leak detections.
- The last test seemed to need a few more 'read' invocations in
  order to get memory usage to a steady state before the test.
---
 src/cmd/ksh93/data/builtins.c |  1 +
 src/cmd/ksh93/tests/leaks.sh  | 22 +++++++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/src/cmd/ksh93/data/builtins.c b/src/cmd/ksh93/data/builtins.c
index 4c289f0da..b06054e53 100644
--- a/src/cmd/ksh93/data/builtins.c
+++ b/src/cmd/ksh93/data/builtins.c
@@ -148,6 +148,7 @@ const struct shtable3 shtab_builtins[] =
 	CMDLIST(uname)
 	CMDLIST(wc)
 	CMDLIST(sync)
+	CMDLIST(vmstate)
 #endif
 #if SHOPT_REGRESS
 	"__regress__",		NV_BLTIN|BLT_ENV,	bltin(__regress__),
diff --git a/src/cmd/ksh93/tests/leaks.sh b/src/cmd/ksh93/tests/leaks.sh
index 54b0f711e..f4b06f4f3 100755
--- a/src/cmd/ksh93/tests/leaks.sh
+++ b/src/cmd/ksh93/tests/leaks.sh
@@ -30,16 +30,18 @@ Command=${0##*/}
 integer Errors=0
 
 [[ -d $tmp && -w $tmp ]] || { err\_exit "$LINENO" '$tmp not set; run this from shtests. Aborting.'; exit 1; }
+builtin vmstate 2>/dev/null || { err\_exit "$LINENO" 'vmstate built-in command not compiled in; skipping tests'; exit 0; }
 
 # Get the current amount of memory usage
 function getmem
 {
-	UNIX95=1 ps -p "$$" -o vsz=
+	vmstate --format='%(busy_size)u'
 }
+unit=bytes
 n=$(getmem)
 if	! let "$n == $n" 2>/dev/null	# not a number?
-then	err\_exit "$LINENO" "'ps' not POSIX-compliant; skipping tests"
-	exit 0
+then	err\_exit "$LINENO" "'vmstate' output unexpected; tests cannot be run. (expected a number; got $(printf %q "$n"))"
+	exit 1
 fi
 
 # test for variable reset leak #
@@ -53,6 +55,9 @@ function test_reset
 	done
 }
 
+# Initialise variables used below to avoid false leaks
+before=0 after=0 i=0 u=0
+
 N=1000
 
 # one round to get to steady state -- sensitive to -x
@@ -64,7 +69,7 @@ test_reset $N
 after=$(getmem)
 
 if	(( after > before ))
-then	err_exit "variable value reset memory leak -- $((after - before)) KiB after $N iterations"
+then	err_exit "variable value reset memory leak -- $((after - before)) $unit after $N iterations"
 fi
 
 # buffer boundary tests
@@ -83,14 +88,17 @@ done |	while read -u$n -C stat
 	do	:
 	done	{n}<&0-
 after=$(getmem)
-(( after > before )) && err_exit "memory leak with read -C when deleting compound variable (leaked $((after - before)) KiB)"
+(( after > before )) && err_exit "memory leak with read -C when deleting compound variable (leaked $((after - before)) $unit)"
 
-read -C stat <<< "$data"
+# extra 'read's to get to steady state
+for ((i=0; i < 10; i++))
+do	read -C stat <<< "$data"
+done
 before=$(getmem)
 for ((i=0; i < 500; i++))
 do      read -C stat <<< "$data"
 done
 after=$(getmem)
-(( after > before )) && err_exit "memory leak with read -C when using <<< (leaked $((after - before)) KiB)"
+(( after > before )) && err_exit "memory leak with read -C when using <<< (leaked $((after - before)) $unit)"
 
 exit $((Errors<125?Errors:125))