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

3564–3566

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

3564–3566

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)
     {
         _ForwardIterator __temp = _VSTD::next(__first);
         if (__temp != __last)
         {
             if (*__first == '\\')
             { 
-                int __val = __traits_.value(*__temp, 10);
+                unsigned __val = __traits_.value(*__temp, 10);
                 if (__val >= 1 && __val <= 9)
                 {
+                    if (__val > mark_count())
+                        __throw_regex_error<regex_constants::error_backref>();
                     __push_back_ref(__val);
                     __first = ++__temp;
                 }
             }
         }
     }
     return __first;
 }
if (__first != __last)
                     __push_char(*__temp);
                     __first = ++__temp;
                     break;
                 default:
                     if (__get_grammar(__flags_) == awk)
                         __first = __parse_awk_escape(++__first, __last);
                     else {
                         unsigned __val = __traits_.value(*__temp, 10);
                         if (__val >= 1 && __val <= 9)
                             if (__val > mark_count())
                                 __throw_regex_error<regex_constants::error_backref>();
                             __push_back_ref(__val);
                             __first = ++__temp;
                     }
                     break;
                 }
             }
         }
     }

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 = std::regex_constants::ECMAScript)
 {
     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
 
 //  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.
     }
+//  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::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;
 }