From 1d63995896c3261b4ce68ca6f9a77bb2c8982d49 Mon Sep 17 00:00:00 2001 From: n5m <72841454+n5m@users.noreply.github.com> Date: Mon, 19 Oct 2020 20:25:43 +0000 Subject: [PATCH] fix #15631 (#15632) * trigger valgrind failure on memory leak * remove non-malloc tests * remove ORC test is redundant because we already have an ARC test * only run valgrind tests on 64-bit Linux * disable freebsd and openbsd * Remove tleak_refc As to not test implementation details (or bug) * Fix test failures by removing redundant test Since this tests/shoulfail/tvalgrind.nim was specified here to fail this test itself fails since it will be skipped on non-linux CI * Remove test, reason detailed in the previous commit * Remove redundant disables * Revert removing disables * Add and use valgrind: leaks * Fix Co-authored-by: Clyybber Co-authored-by: n5m (cherry picked from commit 436e1fa5b13d0a5b667a800b1716e5139f1c20fe) --- doc/testament.rst | 5 +++++ testament/specs.nim | 14 +++++++++----- testament/testament.nim | 7 +++++-- tests/arc/tasyncorc.nim | 2 +- tests/arc/thavlak_orc_stress.nim | 2 +- tests/valgrind/tleak_arc.nim | 14 ++++++++++++++ 6 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 tests/valgrind/tleak_arc.nim diff --git a/doc/testament.rst b/doc/testament.rst index d515ac2cb5..1b2b71c18b 100644 --- a/doc/testament.rst +++ b/doc/testament.rst @@ -103,6 +103,11 @@ Example "template" **to edit** and write a Testament unittest: target: "c js" # Targets to run the test into (C, C++, JavaScript, etc). disabled: "bsd" # Disable the test by condition, here BSD is disabled just as an example. + disabled: "win" # Can disable multiple OSes at once + disabled: "32bit" # ...or architectures + disabled: "i386" + disabled: "azure" # ...or pipeline runners + disabled: true # ...or can disable the test entirely """ assert true diff --git a/testament/specs.nim b/testament/specs.nim index e0b9cbeec7..eaae598bbc 100644 --- a/testament/specs.nim +++ b/testament/specs.nim @@ -43,7 +43,7 @@ type reFilesDiffer, # expected and given filenames differ reLinesDiffer, # expected and given line numbers differ reOutputsDiffer, - reExitcodesDiffer, + reExitcodesDiffer, # exit codes of program or of valgrind differ reTimeout, reInvalidPeg, reCodegenFailure, @@ -67,6 +67,9 @@ type msg*: string line*, col*: int + ValgrindSpec* = enum + disabled, enabled, leaking + TSpec* = object action*: TTestAction file*, cmd*: string @@ -92,7 +95,7 @@ type # whether this test can be batchable via `NIM_TESTAMENT_BATCH`; only very # few tests are not batchable; the ones that are not could be turned batchable # by making the dependencies explicit - useValgrind*: bool + useValgrind*: ValgrindSpec timeout*: float # in seconds, fractions possible, # but don't rely on much precision inlineErrors*: seq[InlineError] # line information to error message @@ -306,14 +309,15 @@ proc parseSpec*(filename: string): TSpec = result.unjoinable = not parseCfgBool(e.value) of "valgrind": when defined(linux) and sizeof(int) == 8: - result.useValgrind = parseCfgBool(e.value) + result.useValgrind = if e.value.normalize == "leaks": leaking + else: ValgrindSpec(parseCfgBool(e.value)) result.unjoinable = true - if result.useValgrind: + if result.useValgrind != disabled: result.outputCheck = ocSubstr else: # Windows lacks valgrind. Silly OS. # Valgrind only supports OSX <= 17.x - result.useValgrind = false + result.useValgrind = disabled of "disabled": case e.value.normalize of "y", "yes", "true", "1", "on": result.err = reDisabled diff --git a/testament/testament.nim b/testament/testament.nim index 4afafb645e..8fead76eff 100644 --- a/testament/testament.nim +++ b/testament/testament.nim @@ -501,8 +501,11 @@ proc testSpecHelper(r: var TResults, test: var TTest, expected: TSpec, args = concat(@[exeFile], args) else: exeCmd = exeFile.dup(normalizeExe) - if expected.useValgrind: - args = @["--error-exitcode=1"] & exeCmd & args + if expected.useValgrind != disabled: + var valgrindOptions = @["--error-exitcode=1"] + if expected.useValgrind != leaking: + valgrindOptions.add "--leak-check=yes" + args = valgrindOptions & exeCmd & args exeCmd = "valgrind" var (_, buf, exitCode) = execCmdEx2(exeCmd, args, input = expected.input) # Treat all failure codes from nodejs as 1. Older versions of nodejs used diff --git a/tests/arc/tasyncorc.nim b/tests/arc/tasyncorc.nim index 56b8909b1f..63703b559b 100644 --- a/tests/arc/tasyncorc.nim +++ b/tests/arc/tasyncorc.nim @@ -1,7 +1,7 @@ discard """ output: '''230000''' cmd: '''nim c --gc:orc -d:useMalloc $file''' - valgrind: "true" + valgrind: "leaks" """ # bug #14402 diff --git a/tests/arc/thavlak_orc_stress.nim b/tests/arc/thavlak_orc_stress.nim index 3c61f10efb..862e1a0978 100644 --- a/tests/arc/thavlak_orc_stress.nim +++ b/tests/arc/thavlak_orc_stress.nim @@ -1,6 +1,6 @@ discard """ cmd: "nim c --gc:orc -d:useMalloc -d:nimStressOrc $file" - valgrind: "true" + valgrind: "leaks" output: "done" """ diff --git a/tests/valgrind/tleak_arc.nim b/tests/valgrind/tleak_arc.nim new file mode 100644 index 0000000000..8dea7c62aa --- /dev/null +++ b/tests/valgrind/tleak_arc.nim @@ -0,0 +1,14 @@ +discard """ +valgrind: true +cmd: "nim $target --gc:arc -d:useMalloc $options $file" +exitcode: 1 +outputsub: " definitely lost: 7 bytes in 2 blocks" +disabled: "freebsd" +disabled: "macosx" +disabled: "openbsd" +disabled: "windows" +disabled: "32bit" +""" + +discard alloc(3) +discard alloc(4)