Re: Bug in Java parser

[Joel E. Denny]

Hi Angelo,

On Sun, 20 Feb 2011, Angelo Borsotti wrote:

> I have found a bug in the generation of java parsers, in file lalr.java,
> line 264:
> 
>     java.util.Arrays.fill (valueStack, height - num + 1, height, null);
> 
> that should be replaced by:
> 
>     java.util.Arrays.fill (valueStack, height - num + 1, height + 1, null);
> 
> The original one does not clear the (old) top element contents. Actually, a
> call
> to pop(1) would decrement height, but clear nothing.

Thanks for the patch.  I've pushed it in your name to branch-2.5 and 
master.  The "tiny change" annotation in the ChangeLog just indicates that 
we don't have to ask you to sign copyright papers.  It does not reflect 
the significance of your contribution.

I also pushed a couple more patches in the same vein.  Unfortunately, I 
didn't think of a good way to add test cases for the stack clearing fixes.  
I might try to add a test case for the tracing fix later.

All patches follow.

>From 6f75992be50b83a084f955f63e5c35ccc8705f08 Mon Sep 17 00:00:00 2001
From: Angelo Borsotti <address@hidden>
Date: Sun, 6 Mar 2011 22:19:18 -0500
Subject: [PATCH 1/3] java: fix parser stack popping bug.

Reported at
<http://lists.gnu.org/archive/html/bug-bison/2011-02/msg00005.html>.
* THANKS (Angelo Borsotti): Add.
* data/lalr1.java (YYParser::YYStack::pop): Fix off-by-one error
in clearing the value stack.  Previously, the top element of the
stack wasn't cleared and so the value was not garbage collected.
---
 ChangeLog       |   10 ++++++++++
 THANKS          |    1 +
 data/lalr1.java |    2 +-
 3 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 69efad5..6331311 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2011-03-06  Angelo Borsotti  <address@hidden>  (tiny change)
+
+       java: fix parser stack popping bug.
+       Reported at
+       <http://lists.gnu.org/archive/html/bug-bison/2011-02/msg00005.html>.
+       * THANKS (Angelo Borsotti): Add.
+       * data/lalr1.java (YYParser::YYStack::pop): Fix off-by-one error
+       in clearing the value stack.  Previously, the top element of the
+       stack wasn't cleared and so the value was not garbage collected.
+
 2011-03-06  Joel E. Denny  <address@hidden>
 
        doc: cite publication for LAC.
diff --git a/THANKS b/THANKS
index 43acbed..ac6047a 100644
--- a/THANKS
+++ b/THANKS
@@ -8,6 +8,7 @@ Alexander Belopolsky      address@hidden
 Alexandre Duret-Lutz      address@hidden
 Andreas Schwab            address@hidden
 Andrew Suffield           address@hidden
+Angelo Borsotti           address@hidden
 Anthony Heading           address@hidden
 Arnold Robbins            address@hidden
 Art Haas                  address@hidden
diff --git a/data/lalr1.java b/data/lalr1.java
index aad54a3..29005c2 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -261,7 +261,7 @@ b4_lexer_if([[
     public final void pop (int num) {
       // Avoid memory leaks... garbage collection is a white lie!
       if (num > 0) {
-       java.util.Arrays.fill (valueStack, height - num + 1, height, null);
+       java.util.Arrays.fill (valueStack, height - num + 1, height + 1, null);
         ]b4_locations_if([[java.util.Arrays.fill (locStack, height - num + 1, 
height, null);]])[
       }
       height -= num;
-- 
1.7.0.4


>From 4c2a6e42ba8b6bc4e04985f5ef3ec8926048d4b1 Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sun, 6 Mar 2011 22:48:46 -0500
Subject: [PATCH 2/3] java: finish fixing parser stack popping bug.

* NEWS (2.5): Document.
* data/lalr1.java (YYParser::YYStack::pop): Fix off-by-one error
in clearing the location stack.  Also fix pop function that
accepts no arguments.
---
 ChangeLog       |    8 ++++++++
 NEWS            |    7 ++++++-
 data/lalr1.java |    4 ++--
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6331311..8444a58 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2011-03-06  Joel E. Denny  <address@hidden>
+
+       java: finish fixing parser stack popping bug.
+       * NEWS (2.5): Document.
+       * data/lalr1.java (YYParser::YYStack::pop): Fix off-by-one error
+       in clearing the location stack.  Also fix pop function that
+       accepts no arguments.
+
 2011-03-06  Angelo Borsotti  <address@hidden>  (tiny change)
 
        java: fix parser stack popping bug.
diff --git a/NEWS b/NEWS
index a657bcc..3f10257 100644
--- a/NEWS
+++ b/NEWS
@@ -291,7 +291,12 @@ Bison News
     canonical LR.  However, LAC is still experimental and is disabled
     by default.
 
-** A location handling bug in the Java skeleton has been fixed.
+** Java skeleton fixes:
+
+*** A location handling bug has been fixed.
+
+*** The top element of each of the value stack and location stack is now
+    cleared when popped so that it can be garbage collected.
 
 * Changes in version 2.4.3 (2010-08-05):
 
diff --git a/data/lalr1.java b/data/lalr1.java
index 29005c2..2e6cc8a 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -255,14 +255,14 @@ b4_lexer_if([[
     }
 
     public final void pop () {
-      height--;
+      pop (1);
     }
 
     public final void pop (int num) {
       // Avoid memory leaks... garbage collection is a white lie!
       if (num > 0) {
        java.util.Arrays.fill (valueStack, height - num + 1, height + 1, null);
-        ]b4_locations_if([[java.util.Arrays.fill (locStack, height - num + 1, 
height, null);]])[
+        ]b4_locations_if([[java.util.Arrays.fill (locStack, height - num + 1, 
height + 1, null);]])[
       }
       height -= num;
     }
-- 
1.7.0.4


>From f0e2c228a085196742250b3f394bb5ed55ac4d7c Mon Sep 17 00:00:00 2001
From: Joel E. Denny <address@hidden>
Date: Sun, 6 Mar 2011 22:27:28 -0500
Subject: [PATCH 3/3] java: fix parser tracing bug.

* NEWS (2.5): Document.
* data/lalr1.java (YYParser::YYStack::print): Don't skip top
element.
---
 ChangeLog       |    7 +++++++
 NEWS            |    2 ++
 data/lalr1.java |    2 +-
 3 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8444a58..f5eee1e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2011-03-06  Joel E. Denny  <address@hidden>
 
+       java: fix parser tracing bug.
+       * NEWS (2.5): Document.
+       * data/lalr1.java (YYParser::YYStack::print): Don't skip top
+       element.
+
+2011-03-06  Joel E. Denny  <address@hidden>
+
        java: finish fixing parser stack popping bug.
        * NEWS (2.5): Document.
        * data/lalr1.java (YYParser::YYStack::pop): Fix off-by-one error
diff --git a/NEWS b/NEWS
index 3f10257..423cd7e 100644
--- a/NEWS
+++ b/NEWS
@@ -298,6 +298,8 @@ Bison News
 *** The top element of each of the value stack and location stack is now
     cleared when popped so that it can be garbage collected.
 
+*** Parser traces now print the top element of the stack.
+
 * Changes in version 2.4.3 (2010-08-05):
 
 ** Bison now obeys -Werror and --warnings=error for warnings about
diff --git a/data/lalr1.java b/data/lalr1.java
index 2e6cc8a..eb69238 100644
--- a/data/lalr1.java
+++ b/data/lalr1.java
@@ -284,7 +284,7 @@ b4_lexer_if([[
     {
       out.print ("Stack now");
 
-      for (int i = 0; i < height; i++)
+      for (int i = 0; i <= height; i++)
         {
          out.print (' ');
          out.print (stateStack[i]);
-- 
1.7.0.4