On Thu, 17 Jul 2014 at 10:38:24 +0100, Simon McVittie wrote:
> If min gets larger than 214748365 (slightly more than 2^31 / 10) in any
> given iteration, then the next multiplication by 10 is a signed integer
> overflow, which is formally undefined behaviour in C.
This was already reported as http://bugs.exim.org/show_bug.cgi?id=1463
and fixed upstream as part of r1472. However, the upstream fix did not
update the expected output, so the tests still fail.
I attach updated patches.
S
>From d0a1e185e351cde74b7fc0c072087d8d234e85b5 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Thu, 17 Jul 2014 10:21:06 +0100
Subject: [PATCH 1/3] Run tests with VERBOSE=1 so we can see the logs for
failing tests
---
debian/changelog | 8 ++++++++
debian/rules | 2 +-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/debian/changelog b/debian/changelog
index 0467f9c..5e10cfc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+pcre3 (1:8.35-2.1) UNRELEASED; urgency=medium
+
+ * Non-maintainer upload.
+ * Run tests with VERBOSE=1 so we can see the logs for failing tests
+ (Closes: #755052)
+
+ -- Simon McVittie <s...@debian.org> Thu, 17 Jul 2014 09:27:30 +0100
+
pcre3 (1:8.35-2) unstable; urgency=medium
* Build-depends on auto-reconf (Closes: 754540)
diff --git a/debian/rules b/debian/rules
index 108a738..41ef1a3 100755
--- a/debian/rules
+++ b/debian/rules
@@ -50,7 +50,7 @@ build-stamp: configure-stamp
# Add here commands to compile the package.
$(MAKE)
ifeq ($(DEB_BUILD_GNU_TYPE),$(DEB_HOST_GNU_TYPE))
- $(MAKE) check
+ $(MAKE) check VERBOSE=1
endif
touch build-stamp
--
2.0.1
>From fb90c3add46ec5e8b55a5bf1284f0f7bc3b674f4 Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Sun, 20 Jul 2014 15:33:54 +0100
Subject: [PATCH 2/3] Apply part of upstream r1472 to fix undefined behaviour
when parsing {n} or {m,n} quantifiers, which causes mis-parsing and test
failures under gcc 4.9 (Closes: #751828)
---
debian/changelog | 3 +
.../patches/Fix-silly-quantifier-size-check.patch | 82 ++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 86 insertions(+)
create mode 100644 debian/patches/Fix-silly-quantifier-size-check.patch
diff --git a/debian/changelog b/debian/changelog
index 5e10cfc..775bdb6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -3,6 +3,9 @@ pcre3 (1:8.35-2.1) UNRELEASED; urgency=medium
* Non-maintainer upload.
* Run tests with VERBOSE=1 so we can see the logs for failing tests
(Closes: #755052)
+ * Apply part of upstream r1472 to fix undefined behaviour when parsing
+ {n} or {m,n} quantifiers, which causes mis-parsing and test failures
+ under gcc 4.9 (Closes: #751828)
-- Simon McVittie <s...@debian.org> Thu, 17 Jul 2014 09:27:30 +0100
diff --git a/debian/patches/Fix-silly-quantifier-size-check.patch b/debian/patches/Fix-silly-quantifier-size-check.patch
new file mode 100644
index 0000000..375bcc8
--- /dev/null
+++ b/debian/patches/Fix-silly-quantifier-size-check.patch
@@ -0,0 +1,82 @@
+From: Philip Hazel <ph10>
+Date: Mon, 21 Apr 2014 16:11:50 +0000
+Subject: Fix silly quantifier size check
+
+The tests for quantifiers being too big (greater than 65535) were being
+applied after reading the number, and stupidly assuming that integer
+overflow would give a negative number. The tests are now applied as the
+numbers are read.
+
+[This is part of upstream r1472, which also contains unrelated changes.
+It avoids signed overflow, which is undefined behaviour in ISO C, and
+causes gcc 4.9 to produce undesired results here. -smcv]
+
+Bug: http://bugs.exim.org/show_bug.cgi?id=1463
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=751828
+Origin: upstream, http://vcs.pcre.org/viewvc?view=revision&sortby=date&revision=1472
+Applied-upstream: 8.36
+---
+ pcre_compile.c | 35 ++++++++++++++++-------------------
+ 1 file changed, 16 insertions(+), 19 deletions(-)
+
+diff --git a/pcre_compile.c b/pcre_compile.c
+index 8a5b723..ae0027b 100644
+--- a/pcre_compile.c
++++ b/pcre_compile.c
+@@ -1583,30 +1583,30 @@ read_repeat_counts(const pcre_uchar *p, int *minp, int *maxp, int *errorcodeptr)
+ int min = 0;
+ int max = -1;
+
+-/* Read the minimum value and do a paranoid check: a negative value indicates
+-an integer overflow. */
+-
+-while (IS_DIGIT(*p)) min = min * 10 + (int)(*p++ - CHAR_0);
+-if (min < 0 || min > 65535)
++while (IS_DIGIT(*p))
+ {
+- *errorcodeptr = ERR5;
+- return p;
+- }
+-
+-/* Read the maximum value if there is one, and again do a paranoid on its size.
+-Also, max must not be less than min. */
++ min = min * 10 + (int)(*p++ - CHAR_0);
++ if (min > 65535)
++ {
++ *errorcodeptr = ERR5;
++ return p;
++ }
++ }
+
+ if (*p == CHAR_RIGHT_CURLY_BRACKET) max = min; else
+ {
+ if (*(++p) != CHAR_RIGHT_CURLY_BRACKET)
+ {
+ max = 0;
+- while(IS_DIGIT(*p)) max = max * 10 + (int)(*p++ - CHAR_0);
+- if (max < 0 || max > 65535)
++ while(IS_DIGIT(*p))
+ {
+- *errorcodeptr = ERR5;
+- return p;
+- }
++ max = max * 10 + (int)(*p++ - CHAR_0);
++ if (max > 65535)
++ {
++ *errorcodeptr = ERR5;
++ return p;
++ }
++ }
+ if (max < min)
+ {
+ *errorcodeptr = ERR4;
+@@ -1615,9 +1615,6 @@ if (*p == CHAR_RIGHT_CURLY_BRACKET) max = min; else
+ }
+ }
+
+-/* Fill in the required variables, and pass back the pointer to the terminating
+-'}'. */
+-
+ *minp = min;
+ *maxp = max;
+ return p;
diff --git a/debian/patches/series b/debian/patches/series
index aef6044..d573218 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,3 +4,4 @@ pcre_info.patch
pcregrep.1-patch
soname.patch
no_jit_ppc64el.patch
+Fix-silly-quantifier-size-check.patch
--
2.0.1
>From 1c64efa28aaa66ea7dda4d6fc7d9f261df37b6ca Mon Sep 17 00:00:00 2001
From: Simon McVittie <s...@debian.org>
Date: Sun, 20 Jul 2014 15:34:28 +0100
Subject: [PATCH 3/3] Add an additional patch to update the expected test
output for the fix for #751828, without which the tests would still fail
---
debian/changelog | 2 ++
...cted-test-output-to-account-for-avoiding-.patch | 34 ++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 37 insertions(+)
create mode 100644 debian/patches/Update-expected-test-output-to-account-for-avoiding-.patch
diff --git a/debian/changelog b/debian/changelog
index 775bdb6..96e1619 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -6,6 +6,8 @@ pcre3 (1:8.35-2.1) UNRELEASED; urgency=medium
* Apply part of upstream r1472 to fix undefined behaviour when parsing
{n} or {m,n} quantifiers, which causes mis-parsing and test failures
under gcc 4.9 (Closes: #751828)
+ * Add an additional patch to update the expected test output for the
+ fix for #751828, without which the tests would still fail
-- Simon McVittie <s...@debian.org> Thu, 17 Jul 2014 09:27:30 +0100
diff --git a/debian/patches/Update-expected-test-output-to-account-for-avoiding-.patch b/debian/patches/Update-expected-test-output-to-account-for-avoiding-.patch
new file mode 100644
index 0000000..0e35ac9
--- /dev/null
+++ b/debian/patches/Update-expected-test-output-to-account-for-avoiding-.patch
@@ -0,0 +1,34 @@
+From: Simon McVittie <s...@debian.org>
+Date: Sun, 20 Jul 2014 15:29:29 +0100
+Subject: Update expected test output to account for avoiding signed overflow
+
+The new implementation for detecting excessively large quantifiers
+reports the error sooner than the old one.
+
+Bug: http://bugs.exim.org/show_bug.cgi?id=1463
+Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=751828
+---
+ testdata/testoutput2 | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/testdata/testoutput2 b/testdata/testoutput2
+index b6da7df..cfb446e 100644
+--- a/testdata/testoutput2
++++ b/testdata/testoutput2
+@@ -5821,13 +5821,13 @@ No match
+ No match
+
+ /a{11111111111111111111}/I
+-Failed: number too big in {} quantifier at offset 22
++Failed: number too big in {} quantifier at offset 8
+
+ /(){64294967295}/I
+-Failed: number too big in {} quantifier at offset 14
++Failed: number too big in {} quantifier at offset 9
+
+ /(){2,4294967295}/I
+-Failed: number too big in {} quantifier at offset 15
++Failed: number too big in {} quantifier at offset 11
+
+ "(?i:a)(?i:b)(?i:c)(?i:d)(?i:e)(?i:f)(?i:g)(?i:h)(?i:i)(?i:j)(k)(?i:l)A\1B"I
+ Capturing subpattern count = 1
diff --git a/debian/patches/series b/debian/patches/series
index d573218..376b8e9 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,4 @@ pcregrep.1-patch
soname.patch
no_jit_ppc64el.patch
Fix-silly-quantifier-size-check.patch
+Update-expected-test-output-to-account-for-avoiding-.patch
--
2.0.1
