[kudu] branch master updated: gutil/strings: fix UBSAN error in FindNth

[ad...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 26dcf68  gutil/strings: fix UBSAN error in FindNth
26dcf68 is described below

commit 26dcf68c7080933d48fb38b564aac41f4bcd2eeb
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Tue Oct 29 20:49:20 2019 -0700

    gutil/strings: fix UBSAN error in FindNth
    
    UBSAN complains when we overflow an unsigned type (size_t). It's not
    undefined behavior, but UBSAN's position is that these kinds of overflows
    are often unintentional and signal a bug.
    
    Newer versions of the code (in other Google codebases) address the problem
    by using an int instead of size_t, so we'll do the same thing too.
    
      src/kudu/gutil/strings/util.cc:1030:34: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'unsigned long'
        #0 0x7fcee8a0798e in FindNth(StringPiece, char, int) /src/kudu/gutil/strings/util.cc:1030:34
    
    Change-Id: I00a36f999a8c3b13469600b785da95c487643d4a
    Reviewed-on: http://gerrit.cloudera.org:8080/14585
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
    Tested-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/gutil/strings/string_util-test.cc | 10 ++++++++++
 src/kudu/gutil/strings/util.cc             |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/kudu/gutil/strings/string_util-test.cc b/src/kudu/gutil/strings/string_util-test.cc
index 6f9f6bd..e761f16 100644
--- a/src/kudu/gutil/strings/string_util-test.cc
+++ b/src/kudu/gutil/strings/string_util-test.cc
@@ -19,6 +19,8 @@
 
 #include "kudu/gutil/strings/util.h"
 
+#include <limits>
+
 #include <gtest/gtest.h>
 
 namespace kudu {
@@ -56,4 +58,12 @@ TEST(StringUtilTest, MatchPatternTest) {
                            "He********************************o")) ;
 }
 
+TEST(StringUtilTest, FindNthTest) {
+  EXPECT_EQ(-1, FindNth("aabbcc", 'b', 0));
+  EXPECT_EQ(2, FindNth("aabbcc", 'b', 1));
+  EXPECT_EQ(3, FindNth("aabbcc", 'b', 2));
+  EXPECT_EQ(-1, FindNth("aabbcc", 'b', 3));
+  EXPECT_EQ(-1, FindNth("aabbcc", 'b', std::numeric_limits<int>::max()));
+}
+
 } // namespace kudu
diff --git a/src/kudu/gutil/strings/util.cc b/src/kudu/gutil/strings/util.cc
index f5b033b..6fc6b10 100644
--- a/src/kudu/gutil/strings/util.cc
+++ b/src/kudu/gutil/strings/util.cc
@@ -1024,7 +1024,7 @@ void InsertString(string *const s,
 //  (returns string::npos = -1 if n <= 0)
 //------------------------------------------------------------------------
 int FindNth(StringPiece s, char c, int n) {
-  size_t pos = string::npos;
+  int pos = -1;
 
   for ( int i = 0; i < n; ++i ) {
     pos = s.find_first_of(c, pos + 1);