bytes, internal/bytealg: fix incorrect IndexString usage

The IndexString implementation in the bytealg package requires that
the string passed into it be in the range '2 <= len(s) <= MaxLen'
where MaxLen may be any value (including 0).

CL 156998 added calls to bytealg.IndexString where MaxLen was not
first checked. This led to an illegal instruction on s390x with
the vector facility disabled.

This CL guards the calls to bytealg.IndexString with a MaxLen check.
If the check fails then the code now falls back to the pre CL 156998
implementation (a loop over the runes in the string).

Since the MaxLen check is now in place the generic implementation is
no longer called so I have returned it to its original unimplemented
state.

In future we may want to drop MaxLen to prevent this kind of
confusion.

Fixes #41552.

Change-Id: Ibeb3f08720444a05c08d719ed97f6cef2423bbe9
Reviewed-on: https://go-review.googlesource.com/c/go/+/256717
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Keith Randall <khr@golang.org>

1 files changed, 32 insertions, 18 deletions

diff --git a/src/bytes/bytes.go b/src/bytes/bytes.go
index aa07b9fbc1..ce52649f13 100644
--- a/src/bytes/bytes.go
+++ b/src/bytes/bytes.go
@@ -227,19 +227,26 @@ func IndexAny(s []byte, chars string) int {
 			continue
 		}
 		r, width = utf8.DecodeRune(s[i:])
-		if r == utf8.RuneError {
-			for _, r = range chars {
-				if r == utf8.RuneError {
+		if r != utf8.RuneError {
+			// r is 2 to 4 bytes
+			if len(chars) == width {
+				if chars == string(r) {
 					return i
 				}
+				continue
+			}
+			// Use bytealg.IndexString for performance if available.
+			if bytealg.MaxLen >= width {
+				if bytealg.IndexString(chars, string(r)) >= 0 {
+					return i
+				}
+				continue
 			}
-			continue
 		}
-		// r is 2 to 4 bytes. Using strings.Index is more reasonable, but as the bytes
-		// package should not import the strings package, use bytealg.IndexString
-		// instead. And this does not seem to lose much performance.
-		if chars == string(r) || bytealg.IndexString(chars, string(r)) >= 0 {
-			return i
+		for _, ch := range chars {
+			if r == ch {
+				return i
+			}
 		}
 	}
 	return -1
@@ -304,19 +311,26 @@ func LastIndexAny(s []byte, chars string) int {
 		}
 		r, size := utf8.DecodeLastRune(s[:i])
 		i -= size
-		if r == utf8.RuneError {
-			for _, r = range chars {
-				if r == utf8.RuneError {
+		if r != utf8.RuneError {
+			// r is 2 to 4 bytes
+			if len(chars) == size {
+				if chars == string(r) {
 					return i
 				}
+				continue
+			}
+			// Use bytealg.IndexString for performance if available.
+			if bytealg.MaxLen >= size {
+				if bytealg.IndexString(chars, string(r)) >= 0 {
+					return i
+				}
+				continue
 			}
-			continue
 		}
-		// r is 2 to 4 bytes. Using strings.Index is more reasonable, but as the bytes
-		// package should not import the strings package, use bytealg.IndexString
-		// instead. And this does not seem to lose much performance.
-		if chars == string(r) || bytealg.IndexString(chars, string(r)) >= 0 {
-			return i
+		for _, ch := range chars {
+			if r == ch {
+				return i
+			}
 		}
 	}
 	return -1