From bcedd5454eafd46f9db6bac7442cc57973a69ec1 Mon Sep 17 00:00:00 2001
From: zerbina <100542850+zerbina@users.noreply.github.com>
Date: Tue, 9 Jan 2024 23:25:41 +0100
Subject: [PATCH] testament: support `\0` byte in output comparison (#1083)

## Summary

Neither the program output nor the comparison string are cut off at the
first null byte anymore.

## Details

Output comparisons in testament used `strutils.contains`, which uses
`strutils.find`, which, by default, uses `c_strstr`.

`c_strstr` operates on null-terminated strings, and thus stopped at the
first occurrence of one, even if the end of the NimSkull string hasn't
been reached yet.

Testament now uses a custom `contains` implementation based on C's
`memcmp`, so that the `\0` byte doesn't terminate the search.
---
 testament/testament.nim                       | 25 ++++++++++++++++++-
 .../tests/shouldfail/toutput_null_byte.nim    |  6 +++++
 tests/testament/tshould_not_work.nim          |  2 ++
 3 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 testament/tests/shouldfail/toutput_null_byte.nim

diff --git a/testament/testament.nim b/testament/testament.nim
index 95e28539065..2a3b4c05368 100644
--- a/testament/testament.nim
+++ b/testament/testament.nim
@@ -10,12 +10,15 @@
 ## Testament runs tests for the compiler.
 
 import std/[
-  strutils, pegs, os, osproc, streams, json, parseopt, browsers,
+  pegs, os, osproc, streams, json, parseopt, browsers,
   terminal, algorithm, times, md5, intsets, macros, tables,
   options, sequtils, hashes
 ]
 import system/platforms
 import backend, htmlgen, specs
+# the ``contains`` overload for strings from strutils doesn't support NULL
+# chars...
+import std/strutils except contains
 from std/sugar import dup
 import compiler/utils/nodejs
 import lib/stdtest/testutils
@@ -190,6 +193,26 @@ let
 
 # ----------------------------------------------------------------------------
 
+proc memcmp(a, b: pointer, len: csize_t): cint {.importc, header: "<string.h>".}
+
+{.push checks: off.} # for efficiency, omit all run-time checks
+
+proc contains(a, b: string): bool =
+  ## Returns whether `b` is part of `a`, but doesn't cut off strings at the first
+  ## NULL byte. If `b` is empty, 'true' is returned.
+  if b.len == 0:
+    return true
+
+  let c = b[0]
+  # search for `b`'s first character in `a`, then do a memcmp
+  for i in 0 .. (a.len - b.len):
+    if a[i] == c and memcmp(addr a[i], addr b[0], csize_t(b.len)) == 0:
+      return true
+
+  result = false
+
+{.pop.}
+
 proc trimUnitSep(x: var string) =
   let L = x.len
   if L > 0 and x[^1] == '\31':
diff --git a/testament/tests/shouldfail/toutput_null_byte.nim b/testament/tests/shouldfail/toutput_null_byte.nim
new file mode 100644
index 00000000000..17d2998a010
--- /dev/null
+++ b/testament/tests/shouldfail/toutput_null_byte.nim
@@ -0,0 +1,6 @@
+discard """
+  output: "a\0b"
+"""
+
+# the string differ after the NULL byte
+echo "a\0c"
\ No newline at end of file
diff --git a/tests/testament/tshould_not_work.nim b/tests/testament/tshould_not_work.nim
index 57aef7cc355..794accab290 100644
--- a/tests/testament/tshould_not_work.nim
+++ b/tests/testament/tshould_not_work.nim
@@ -26,6 +26,8 @@ FAIL: tests/shouldfail/tnimoutfull.nim
 Failure: reMsgsDiffer
 FAIL: tests/shouldfail/toutput.nim
 Failure: reOutputsDiffer
+FAIL: tests/shouldfail/toutput_null_byte.nim
+Failure: reOutputsDiffer
 FAIL: tests/shouldfail/toutputsub.nim
 Failure: reOutputsDiffer
 FAIL: tests/shouldfail/treject.nim