Regex backreference [3/3] Validate backreferences in the constructor.

[Mordante]

Summary

This patch enables throwing exceptions for invalid backreferences
in the constructor when using the basic, extended, grep, or egrep grammar.

This fixes bug 34297.

[mclow.lists]

I think you need more diverse tests here.
What about a reference to a pattern that hasn't been defined yet? Is that valid? Something like "\\1(cat)"

[Mordante]

Added more unit tests.
Moved the range validation from D62451 in this patch.

[Mordante]

Ping

[zoecarver]

libcxx/include/regex

3541–3542

Doesn't this if statement need braces around it?

libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp

21

Not an issue at all, just a small nit: the default value of f is never used.

[zoecarver]

libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp

32

All the std::regex::extended and std::regex::egrep tests fail for me.

[Mordante]

libcxx/include/regex

3541–3542

Yes. This is the same location as you spotted in D62451.

libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp

21

Actually the default argument is used in one call. I'll adjust that call and remove the default.

[Mordante]

Fixes the issues found by @zoecarver.
Only the unit tests don't fail for me. Will try to catch @zoecarver on IRC.

[zoecarver]

Thanks for fixing those issues :) Looking at it more, it looks like the whole patch wasn't applied. Lines 3555 and 3556 were not update based on the diff I downloaded. When was the last time you rebased off master? I will be on IRC for the next few minutes. Otherwise, my IRC bouncer will catch any messages you send me.

[zoecarver]

After applying both this and D62451, the tests all passed. Does D62452 rely on / is relied on by any other patches?

[Mordante]

Friendly ping

[ldionne]

Could you please rebase that on top of the latest master?

[ldionne]

(requesting changes so it shows up in my review queue)

[Mordante]

Rebased as requested.

[ldionne]

Thanks!

libcxx/include/regex

if (__first != __last)
                 case '?':
                 case '{':
                 case '}':
                     __push_char(*__temp);
                     __first = ++__temp;
                     break;
                 default:
                     if (__get_grammar(__flags_) == awk)
                         __first = __parse_awk_escape(++__first, __last);
                     else if(__test_back_ref(*__temp))
                         __first = ++__temp;
                     break;
                 }
             }
         }
     }
     return __first;
 }

 
 template <class _CharT, class _Traits>
 bool
 basic_regex<_CharT, _Traits>::__test_back_ref(_CharT c)
 {
     unsigned __val = __traits_.value(c, 10);
     if (__val >= 1 && __val <= 9)
     {
+        if (__val > mark_count())
+            __throw_regex_error<regex_constants::error_backref>();
         __push_back_ref(__val);
         return true;
     }
 
     return false;
 }
 
 template <class _CharT, class _Traits>

libcxx/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp

 
 // template <class ST, class SA>
 //    basic_regex(const basic_string<charT, ST, SA>& s);
 
 #include <regex>
 #include <cassert>
 #include "test_macros.h"
 
-static bool error_badbackref_thrown(const char *pat)
+static bool error_badbackref_thrown(const char *pat, std::regex::flag_type f)
 {
     bool result = false;
     try {
-        std::regex re(pat);
+        std::regex re(pat, f);
     } catch (const std::regex_error &ex) {
         result = (ex.code() == std::regex_constants::error_backref);
     }
     return result;
 }
 
 int main(int, char**)
 {
-    assert(error_badbackref_thrown("\\1abc"));      // no references
-    assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
-    assert(error_badbackref_thrown("\\800000000000000000000000000000")); // overflows
+//  no references
+    assert(error_badbackref_thrown("\\1abc", std::regex_constants::ECMAScript));
+    assert(error_badbackref_thrown("\\1abd", std::regex::basic));
+    assert(error_badbackref_thrown("\\1abd", std::regex::extended));
+    assert(error_badbackref_thrown("\\1abd", std::regex::awk) == false);
+    assert(error_badbackref_thrown("\\1abd", std::regex::grep));
+    assert(error_badbackref_thrown("\\1abd", std::regex::egrep));
+
+//  only one reference
+    assert(error_badbackref_thrown("ab(c)\\2def", std::regex_constants::ECMAScript));
+    assert(error_badbackref_thrown("ab\\(c\\)\\2def", std::regex_constants::basic));
+    assert(error_badbackref_thrown("ab(c)\\2def", std::regex_constants::extended));
+    assert(error_badbackref_thrown("ab\\(c\\)\\2def", std::regex_constants::awk) == false);
+    assert(error_badbackref_thrown("ab(c)\\2def", std::regex_constants::awk) == false);
+    assert(error_badbackref_thrown("ab\\(c\\)\\2def", std::regex_constants::grep));
+    assert(error_badbackref_thrown("ab(c)\\2def", std::regex_constants::egrep));
+
+
+    assert(error_badbackref_thrown("\\800000000000000000000000000000", std::regex_constants::ECMAScript)); // overflows
 
 //  this should NOT throw, because we only should look at the '1'
 //  See https://bugs.llvm.org/show_bug.cgi?id=31387
     {
     const char *pat1 = "a(b)c\\1234";
     std::regex re(pat1, pat1 + 7); // extra chars after the end.
     }
 
+//  reference before group
+    assert(error_badbackref_thrown("\\1(abc)", std::regex_constants::ECMAScript));
+    assert(error_badbackref_thrown("\\1\\(abd\\)", std::regex::basic));
+    assert(error_badbackref_thrown("\\1(abd)", std::regex::extended));
+    assert(error_badbackref_thrown("\\1(abd)", std::regex::awk) == false);
+    assert(error_badbackref_thrown("\\1\\(abd\\)", std::regex::awk) == false);
+    assert(error_badbackref_thrown("\\1\\(abd\\)", std::regex::grep));
+    assert(error_badbackref_thrown("\\1(abd)", std::regex::egrep));
+
+//  reference limit
+    assert(error_badbackref_thrown("(cat)\\10", std::regex::ECMAScript));
+    assert(error_badbackref_thrown("\\(cat\\)\\10", std::regex::basic) == false);
+    assert(error_badbackref_thrown("(cat)\\10", std::regex::extended) == false);
+    assert(error_badbackref_thrown("\\(cat\\)\\10", std::regex::awk) == false);
+    assert(error_badbackref_thrown("(cat)\\10", std::regex::awk) == false);
+    assert(error_badbackref_thrown("\\(cat\\)\\10", std::regex::grep) == false);
+    assert(error_badbackref_thrown("(cat)\\10", std::regex::egrep) == false);
+
+//  https://bugs.llvm.org/show_bug.cgi?id=34297
+    assert(error_badbackref_thrown("(cat)\\1", std::regex::basic));
+    assert(error_badbackref_thrown("\\(cat\\)\\1", std::regex::basic) == false);
+    assert(error_badbackref_thrown("(cat)\\1", std::regex::extended) == false);
+    assert(error_badbackref_thrown("\\(cat\\)\\1", std::regex::extended));
+    assert(error_badbackref_thrown("(cat)\\1", std::regex::awk) == false);
+    assert(error_badbackref_thrown("\\(cat\\)\\1", std::regex::awk) == false);
+    assert(error_badbackref_thrown("(cat)\\1", std::regex::grep));
+    assert(error_badbackref_thrown("\\(cat\\)\\1", std::regex::grep) == false);
+    assert(error_badbackref_thrown("(cat)\\1", std::regex::egrep) == false);
+    assert(error_badbackref_thrown("\\(cat\\)\\1", std::regex::egrep));
+
   return 0;
 }