[PATCH] D37956: [libc++] Check back reference subscript overflow in a single place.

[Tim Shen via Phabricator via cfe-commits]

timshen created this revision.
Herald added a subscriber: sanjoy.
Herald added a reviewer: EricWF.

Currently it's checked in three places, including one place that's at
regex runtime.

There is no need to do runtime check, as all uses and definitions of
subexpress is known at regex compile-time.

This fixes PR34297.


https://reviews.llvm.org/D37956

Files:
  libcxx/include/regex
  libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp


Index: libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
===================================================================
--- libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
+++ libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
@@ -19,11 +19,14 @@
 #include <cassert>
 #include "test_macros.h"
 
-static bool error_badbackref_thrown(const char *pat)
+static bool error_badbackref_thrown(
+    const char *pat,
+    std::regex_constants::syntax_option_type flags =
+        std::regex_constants::ECMAScript)
 {
     bool result = false;
     try {
-        std::regex re(pat);
+        std::regex re(pat, flags);
     } catch (const std::regex_error &ex) {
         result = (ex.code() == std::regex_constants::error_backref);
     }
@@ -34,6 +37,7 @@
 {
     assert(error_badbackref_thrown("\\1abc"));      // no references
     assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
+    assert(error_badbackref_thrown("(cat)\\1", std::regex_constants::basic));
 
 //  this should NOT throw, because we only should look at the '1'
 //  See https://bugs.llvm.org/show_bug.cgi?id=31387
Index: libcxx/include/regex
===================================================================
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -1745,8 +1745,6 @@
 void
 __back_ref<_CharT>::__exec(__state& __s) const
 {
-    if (__mexp_ > __s.__sub_matches_.size())
-        __throw_regex_error<regex_constants::error_backref>();
     sub_match<const _CharT*>& __sm = __s.__sub_matches_[__mexp_-1];
     if (__sm.matched)
     {
@@ -4320,8 +4318,6 @@
             for (++__first;
                     __first != __last && '0' <= *__first && *__first <= '9'; 
++__first)
                 __v = 10 * __v + *__first - '0';
-            if (__v > mark_count())
-                __throw_regex_error<regex_constants::error_backref>();
             __push_back_ref(__v);
         }
     }
@@ -4712,6 +4708,9 @@
 void
 basic_regex<_CharT, _Traits>::__push_back_ref(int __i)
 {
+    if (__i > static_cast<int>(mark_count()))
+        __throw_regex_error<regex_constants::error_backref>();
+
     if (flags() & icase)
         __end_->first() = new __back_ref_icase<_CharT, _Traits>
                                               (__traits_, __i, 
__end_->first());



Index: libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
===================================================================
--- libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
+++ libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
@@ -19,11 +19,14 @@
 #include <cassert>
 #include "test_macros.h"
 
-static bool error_badbackref_thrown(const char *pat)
+static bool error_badbackref_thrown(
+    const char *pat,
+    std::regex_constants::syntax_option_type flags =
+        std::regex_constants::ECMAScript)
 {
     bool result = false;
     try {
-        std::regex re(pat);
+        std::regex re(pat, flags);
     } catch (const std::regex_error &ex) {
         result = (ex.code() == std::regex_constants::error_backref);
     }
@@ -34,6 +37,7 @@
 {
     assert(error_badbackref_thrown("\\1abc"));      // no references
     assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
+    assert(error_badbackref_thrown("(cat)\\1", std::regex_constants::basic));
 
 //  this should NOT throw, because we only should look at the '1'
 //  See https://bugs.llvm.org/show_bug.cgi?id=31387
Index: libcxx/include/regex
===================================================================
--- libcxx/include/regex
+++ libcxx/include/regex
@@ -1745,8 +1745,6 @@
 void
 __back_ref<_CharT>::__exec(__state& __s) const
 {
-    if (__mexp_ > __s.__sub_matches_.size())
-        __throw_regex_error<regex_constants::error_backref>();
     sub_match<const _CharT*>& __sm = __s.__sub_matches_[__mexp_-1];
     if (__sm.matched)
     {
@@ -4320,8 +4318,6 @@
             for (++__first;
                     __first != __last && '0' <= *__first && *__first <= '9'; ++__first)
                 __v = 10 * __v + *__first - '0';
-            if (__v > mark_count())
-                __throw_regex_error<regex_constants::error_backref>();
             __push_back_ref(__v);
         }
     }
@@ -4712,6 +4708,9 @@
 void
 basic_regex<_CharT, _Traits>::__push_back_ref(int __i)
 {
+    if (__i > static_cast<int>(mark_count()))
+        __throw_regex_error<regex_constants::error_backref>();
+
     if (flags() & icase)
         __end_->first() = new __back_ref_icase<_CharT, _Traits>
                                               (__traits_, __i, __end_->first());

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits