bugfix: strutils.find was broken for strings with uneven number of chars

For some reason, the problem was manifesting only inside the VM, it was
detecting an attempt to read past the string end (i.e. the formerly
accessible null byte).

To catch such errors, strutils now performs static tests too.

I've solved the problem by re-implementing the Boyer-Moore algotihm
in a cleaner way and I took the opportunity to make some other
optimisations to strutils.
This commit is contained in:
Zahary Karadjov 2018-05-04 19:56:03 +03:00 • committed by Andreas Rumpf
commit 70ec344bbf

View file

@ -1262,21 +1262,20 @@ proc initSkipTable*(a: var SkipTable, sub: string)
{.noSideEffect, rtl, extern: "nsuInitSkipTable".} = {.noSideEffect, rtl, extern: "nsuInitSkipTable".} =
## Preprocess table `a` for `sub`. ## Preprocess table `a` for `sub`.
let m = len(sub) let m = len(sub)
let m1 = m + 1
var i = 0 var i = 0
while i <= 0xff-7: while i <= 0xff-7:
a[chr(i + 0)] = m1 a[chr(i + 0)] = m
a[chr(i + 1)] = m1 a[chr(i + 1)] = m
a[chr(i + 2)] = m1 a[chr(i + 2)] = m
a[chr(i + 3)] = m1 a[chr(i + 3)] = m
a[chr(i + 4)] = m1 a[chr(i + 4)] = m
a[chr(i + 5)] = m1 a[chr(i + 5)] = m
a[chr(i + 6)] = m1 a[chr(i + 6)] = m
a[chr(i + 7)] = m1 a[chr(i + 7)] = m
i += 8 i += 8
for i in 0..m-1: for i in 0 ..< m - 1:
a[sub[i]] = m-i a[sub[i]] = m - 1 - i
proc find*(a: SkipTable, s, sub: string, start: Natural = 0, last: Natural = 0): int proc find*(a: SkipTable, s, sub: string, start: Natural = 0, last: Natural = 0): int
{.noSideEffect, rtl, extern: "nsuFindStrA".} = {.noSideEffect, rtl, extern: "nsuFindStrA".} =
@ -1284,18 +1283,29 @@ proc find*(a: SkipTable, s, sub: string, start: Natural = 0, last: Natural = 0):
## If `last` is unspecified, it defaults to `s.high`. ## If `last` is unspecified, it defaults to `s.high`.
## ##
## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned. ## Searching is case-sensitive. If `sub` is not in `s`, -1 is returned.
let let
last = if last==0: s.high else: last last = if last==0: s.high else: last
m = len(sub) sLen = last - start + 1
n = last + 1 subLast = sub.len - 1
# search:
var j = start if subLast == -1:
while j <= n - m: # this was an empty needle string,
block match: # we count this as match in the first possible position:
for k in 0..m-1: return start
if sub[k] != s[k+j]: break match
return j # This is an implementation of the Boyer-Moore Horspool algorithms
inc(j, a[s[j+m]]) # https://en.wikipedia.org/wiki/Boyer%E2%80%93Moore%E2%80%93Horspool_algorithm
var skip = start
while last - skip >= subLast:
var i = subLast
while s[skip + i] == sub[i]:
if i == 0:
return skip
dec i
inc skip, a[s[skip + subLast]]
return -1 return -1
when not (defined(js) or defined(nimdoc) or defined(nimscript)): when not (defined(js) or defined(nimdoc) or defined(nimscript)):
@ -1455,8 +1465,30 @@ proc contains*(s: string, chars: set[char]): bool {.noSideEffect.} =
proc replace*(s, sub: string, by = ""): string {.noSideEffect, proc replace*(s, sub: string, by = ""): string {.noSideEffect,
rtl, extern: "nsuReplaceStr".} = rtl, extern: "nsuReplaceStr".} =
## Replaces `sub` in `s` by the string `by`. ## Replaces `sub` in `s` by the string `by`.
var a {.noinit.}: SkipTable
result = "" result = ""
let subLen = sub.len
if subLen == 0:
for c in s:
add result, by
add result, c
add result, by
return
elif subLen == 1:
# when the pattern is a single char, we use a faster
# char-based search that doesn't need a skip table:
var c = sub[0]
let last = s.high
var i = 0
while true:
let j = find(s, c, i, last)
if j < 0: break
add result, substr(s, i, j - 1)
add result, by
i = j + subLen
# copy the rest:
add result, substr(s, i)
else:
var a {.noinit.}: SkipTable
initSkipTable(a, sub) initSkipTable(a, sub)
let last = s.high let last = s.high
var i = 0 var i = 0
@ -1465,11 +1497,7 @@ proc replace*(s, sub: string, by = ""): string {.noSideEffect,
if j < 0: break if j < 0: break
add result, substr(s, i, j - 1) add result, substr(s, i, j - 1)
add result, by add result, by
if sub.len == 0: i = j + subLen
if i < s.len: add result, s[i]
i = j + 1
else:
i = j + sub.len
# copy the rest: # copy the rest:
add result, substr(s, i) add result, substr(s, i)
@ -1492,6 +1520,7 @@ proc replaceWord*(s, sub: string, by = ""): string {.noSideEffect,
## Each occurrence of `sub` has to be surrounded by word boundaries ## Each occurrence of `sub` has to be surrounded by word boundaries
## (comparable to ``\\w`` in regular expressions), otherwise it is not ## (comparable to ``\\w`` in regular expressions), otherwise it is not
## replaced. ## replaced.
if sub.len == 0: return s
const wordChars = {'a'..'z', 'A'..'Z', '0'..'9', '_', '\128'..'\255'} const wordChars = {'a'..'z', 'A'..'Z', '0'..'9', '_', '\128'..'\255'}
var a {.noinit.}: SkipTable var a {.noinit.}: SkipTable
result = "" result = ""
@ -2326,6 +2355,57 @@ proc removePrefix*(s: var string, prefix: string) {.
s.delete(0, prefix.len - 1) s.delete(0, prefix.len - 1)
when isMainModule: when isMainModule:
proc nonStaticTests =
doAssert formatBiggestFloat(1234.567, ffDecimal, -1) == "1234.567000"
doAssert formatBiggestFloat(1234.567, ffDecimal, 0) == "1235."
doAssert formatBiggestFloat(1234.567, ffDecimal, 1) == "1234.6"
doAssert formatBiggestFloat(0.00000000001, ffDecimal, 11) == "0.00000000001"
doAssert formatBiggestFloat(0.00000000001, ffScientific, 1, ',') in
["1,0e-11", "1,0e-011"]
# bug #6589
doAssert formatFloat(123.456, ffScientific, precision = -1) == "1.234560e+02"
doAssert "$# $3 $# $#" % ["a", "b", "c"] == "a c b c"
doAssert "${1}12 ${-1}$2" % ["a", "b"] == "a12 bb"
block: # formatSize tests
doAssert formatSize((1'i64 shl 31) + (300'i64 shl 20)) == "2.293GiB"
doAssert formatSize((2.234*1024*1024).int) == "2.234MiB"
doAssert formatSize(4096) == "4KiB"
doAssert formatSize(4096, prefix=bpColloquial, includeSpace=true) == "4 kB"
doAssert formatSize(4096, includeSpace=true) == "4 KiB"
doAssert formatSize(5_378_934, prefix=bpColloquial, decimalSep=',') == "5,13MB"
block: # formatEng tests
doAssert formatEng(0, 2, trim=false) == "0.00"
doAssert formatEng(0, 2) == "0"
doAssert formatEng(53, 2, trim=false) == "53.00"
doAssert formatEng(0.053, 2, trim=false) == "53.00e-3"
doAssert formatEng(0.053, 4, trim=false) == "53.0000e-3"
doAssert formatEng(0.053, 4, trim=true) == "53e-3"
doAssert formatEng(0.053, 0) == "53e-3"
doAssert formatEng(52731234) == "52.731234e6"
doAssert formatEng(-52731234) == "-52.731234e6"
doAssert formatEng(52731234, 1) == "52.7e6"
doAssert formatEng(-52731234, 1) == "-52.7e6"
doAssert formatEng(52731234, 1, decimalSep=',') == "52,7e6"
doAssert formatEng(-52731234, 1, decimalSep=',') == "-52,7e6"
doAssert formatEng(4100, siPrefix=true, unit="V") == "4.1 kV"
doAssert formatEng(4.1, siPrefix=true, unit="V", useUnitSpace=true) == "4.1 V"
doAssert formatEng(4.1, siPrefix=true) == "4.1" # Note lack of space
doAssert formatEng(4100, siPrefix=true) == "4.1 k"
doAssert formatEng(4.1, siPrefix=true, unit="", useUnitSpace=true) == "4.1 " # Includes space
doAssert formatEng(4100, siPrefix=true, unit="") == "4.1 k"
doAssert formatEng(4100) == "4.1e3"
doAssert formatEng(4100, unit="V", useUnitSpace=true) == "4.1e3 V"
doAssert formatEng(4100, unit="", useUnitSpace=true) == "4.1e3 "
# Don't use SI prefix as number is too big
doAssert formatEng(3.1e22, siPrefix=true, unit="a", useUnitSpace=true) == "31e21 a"
# Don't use SI prefix as number is too small
doAssert formatEng(3.1e-25, siPrefix=true, unit="A", useUnitSpace=true) == "310e-27 A"
proc staticTests =
doAssert align("abc", 4) == " abc" doAssert align("abc", 4) == " abc"
doAssert align("a", 0) == "a" doAssert align("a", 0) == "a"
doAssert align("1232", 6) == " 1232" doAssert align("1232", 6) == " 1232"
@ -2347,33 +2427,13 @@ when isMainModule:
longOutp = "ThisIsOn\neVeryLon\ngStringW\nhichWeWi\nllSplitI\nntoEight\nSeparate\nPartsNow" longOutp = "ThisIsOn\neVeryLon\ngStringW\nhichWeWi\nllSplitI\nntoEight\nSeparate\nPartsNow"
doAssert wordWrap(longInp, 8, true) == longOutp doAssert wordWrap(longInp, 8, true) == longOutp
doAssert formatBiggestFloat(1234.567, ffDecimal, -1) == "1234.567000"
doAssert formatBiggestFloat(1234.567, ffDecimal, 0) == "1235."
doAssert formatBiggestFloat(1234.567, ffDecimal, 1) == "1234.6"
doAssert formatBiggestFloat(0.00000000001, ffDecimal, 11) == "0.00000000001"
doAssert formatBiggestFloat(0.00000000001, ffScientific, 1, ',') in
["1,0e-11", "1,0e-011"]
# bug #6589
doAssert formatFloat(123.456, ffScientific, precision = -1) == "1.234560e+02"
doAssert "$# $3 $# $#" % ["a", "b", "c"] == "a c b c"
doAssert "${1}12 ${-1}$2" % ["a", "b"] == "a12 bb"
block: # formatSize tests
doAssert formatSize((1'i64 shl 31) + (300'i64 shl 20)) == "2.293GiB"
doAssert formatSize((2.234*1024*1024).int) == "2.234MiB"
doAssert formatSize(4096) == "4KiB"
doAssert formatSize(4096, prefix=bpColloquial, includeSpace=true) == "4 kB"
doAssert formatSize(4096, includeSpace=true) == "4 KiB"
doAssert formatSize(5_378_934, prefix=bpColloquial, decimalSep=',') == "5,13MB"
doAssert "$animal eats $food." % ["animal", "The cat", "food", "fish"] == doAssert "$animal eats $food." % ["animal", "The cat", "food", "fish"] ==
"The cat eats fish." "The cat eats fish."
doAssert "-ld a-ldz -ld".replaceWord("-ld") == " a-ldz " doAssert "-ld a-ldz -ld".replaceWord("-ld") == " a-ldz "
doAssert "-lda-ldz -ld abc".replaceWord("-ld") == "-lda-ldz abc" doAssert "-lda-ldz -ld abc".replaceWord("-ld") == "-lda-ldz abc"
doAssert "-lda-ldz -ld abc".replaceWord("") == "lda-ldz ld abc" doAssert "-lda-ldz -ld abc".replaceWord("") == "-lda-ldz -ld abc"
doAssert "oo".replace("", "abc") == "abcoabcoabc" doAssert "oo".replace("", "abc") == "abcoabcoabc"
type MyEnum = enum enA, enB, enC, enuD, enE type MyEnum = enum enA, enB, enC, enuD, enE
@ -2521,35 +2581,6 @@ bar
doAssert s.splitWhitespace(maxsplit=3) == @["this", "is", "an", "example "] doAssert s.splitWhitespace(maxsplit=3) == @["this", "is", "an", "example "]
doAssert s.splitWhitespace(maxsplit=4) == @["this", "is", "an", "example"] doAssert s.splitWhitespace(maxsplit=4) == @["this", "is", "an", "example"]
block: # formatEng tests
doAssert formatEng(0, 2, trim=false) == "0.00"
doAssert formatEng(0, 2) == "0"
doAssert formatEng(53, 2, trim=false) == "53.00"
doAssert formatEng(0.053, 2, trim=false) == "53.00e-3"
doAssert formatEng(0.053, 4, trim=false) == "53.0000e-3"
doAssert formatEng(0.053, 4, trim=true) == "53e-3"
doAssert formatEng(0.053, 0) == "53e-3"
doAssert formatEng(52731234) == "52.731234e6"
doAssert formatEng(-52731234) == "-52.731234e6"
doAssert formatEng(52731234, 1) == "52.7e6"
doAssert formatEng(-52731234, 1) == "-52.7e6"
doAssert formatEng(52731234, 1, decimalSep=',') == "52,7e6"
doAssert formatEng(-52731234, 1, decimalSep=',') == "-52,7e6"
doAssert formatEng(4100, siPrefix=true, unit="V") == "4.1 kV"
doAssert formatEng(4.1, siPrefix=true, unit="V", useUnitSpace=true) == "4.1 V"
doAssert formatEng(4.1, siPrefix=true) == "4.1" # Note lack of space
doAssert formatEng(4100, siPrefix=true) == "4.1 k"
doAssert formatEng(4.1, siPrefix=true, unit="", useUnitSpace=true) == "4.1 " # Includes space
doAssert formatEng(4100, siPrefix=true, unit="") == "4.1 k"
doAssert formatEng(4100) == "4.1e3"
doAssert formatEng(4100, unit="V", useUnitSpace=true) == "4.1e3 V"
doAssert formatEng(4100, unit="", useUnitSpace=true) == "4.1e3 "
# Don't use SI prefix as number is too big
doAssert formatEng(3.1e22, siPrefix=true, unit="a", useUnitSpace=true) == "31e21 a"
# Don't use SI prefix as number is too small
doAssert formatEng(3.1e-25, siPrefix=true, unit="A", useUnitSpace=true) == "310e-27 A"
block: # startsWith / endsWith char tests block: # startsWith / endsWith char tests
var s = "abcdef" var s = "abcdef"
doAssert s.startsWith('a') doAssert s.startsWith('a')
@ -2559,3 +2590,8 @@ bar
doAssert s.endsWith('\0') == false doAssert s.endsWith('\0') == false
#echo("strutils tests passed") #echo("strutils tests passed")
nonStaticTests()
staticTests()
static: staticTests()