bug#8751: 24.0.50; CCL is broken

bug#8751: 24.0.50; CCL is broken

[Kazuhiro Ito]

Below code raises error "Error in CCL program at 4th code" on trunk.

(progn
  (define-ccl-program ccl-test
    '(1 (loop
	 (read r0)
	 (write-repeat r0))))
  (ccl-execute-on-string 'ccl-test (make-vector 9 0) "_"))

-- 
Kazuhiro Ito

bug#8751: 24.0.50; CCL is broken

[Kazuhiro Ito]

The problem may be that ccl-compiler produce negative integer as CCL
code (at latest, since Emacs21).

(ccl-compile
 '(1 (loop
      (read r0)
      (write-repeat r0))))

-> [1 4 14 -506 22]
           ~~~~

But the document of ccl.c says "CCL code is a sequence of 28-bit
non-negative integers", and current ccl.c does not accept such
negative value.  I don't know which whould be changed, ccl-compiler or
document and current ccl.c.

Kazuhiro Ito wrote:
>
> Below code raises error "Error in CCL program at 4th code" on trunk.
> 
> (progn
>   (define-ccl-program ccl-test
>     '(1 (loop
> 	 (read r0)
> 	 (write-repeat r0))))
>   (ccl-execute-on-string 'ccl-test (make-vector 9 0) "_"))

-- 
Kazuhiro Ito

bug#8751: 24.0.50; CCL is broken

[Stefan Monnier]

> The problem may be that ccl-compiler produce negative integer as CCL
> code (at latest, since Emacs21).

The negative number is normal (it's due to the relative instruction
address for the backward jump at the end of the loop).  The problem must
be in the ccl.c code.


        Stefan

bug#8751: 24.0.50; CCL is broken

[Kazuhiro Ito]

> > The problem may be that ccl-compiler produce negative integer as CCL
> > code (at latest, since Emacs21).
> 
> The negative number is normal (it's due to the relative instruction
> address for the backward jump at the end of the loop).  The problem must
> be in the ccl.c code.

If that is right (and I think so, too), the comment of ccl.c should be
fixed.

> /* CCL code is a sequence of 28-bit non-negative integers (i.e. the
                                      ~~~~~~~~~~~~
>    MSB is always 0), each contains CCL command and/or arguments in the
>    following format:
>
> 	|----------------- integer (28-bit) ------------------|
> 	|------- 17-bit ------|- 3-bit --|- 3-bit --|- 5-bit -|
> 	|--constant argument--|-register-|-register-|-command-|
> 	   ccccccccccccccccc      RRR        rrr       XXXXX
>   or
> 	|------- relative address -------|-register-|-command-|
> 	       cccccccccccccccccccc          rrr       XXXXX
>   or
> 	|------------- constant or other args ----------------|
>                      cccccccccccccccccccccccccccc
>
>    where, `cc...c' is a non-negative integer indicating constant value
                          ~~~~~~~~~~~~
>    (the left most `c' is always 0) or an absolute jump address, `RRR'
>    and `rrr' are CCL register number, `XXXXX' is one of the following
>    CCL commands.  */


Here is a quick fix for actual code, but I don't check whether there
is other problems.

=== modified file 'src/ccl.c'
--- src/ccl.c	2011-05-27 21:24:11 +0000
+++ src/ccl.c	2011-05-30 22:19:52 +0000
@@ -100,6 +100,7 @@
    CCL commands.  */
 
 #define CCL_CODE_MAX ((1 << (28 - 1)) - 1)
+#define CCL_CODE_MIN (- CCL_CODE_MAX - 1)
 
 /* CCL commands
 
@@ -756,7 +757,7 @@
   while (0)
 
 #define GET_CCL_CODE(code, ccl_prog, ic)			\
-  GET_CCL_RANGE (code, ccl_prog, ic, 0, CCL_CODE_MAX)
+  GET_CCL_RANGE (code, ccl_prog, ic, CCL_CODE_MIN, CCL_CODE_MAX)
 
 #define GET_CCL_INT(var, ccl_prog, ic)				\
   GET_CCL_RANGE (var, ccl_prog, ic, INT_MIN, INT_MAX)


-- 
Kazuhiro Ito

bug#8719: bug#8751: 24.0.50; CCL is broken

[Paul Eggert]

> Here is a quick fix for actual code, but I don't check whether there
> is other problems.

Thanks, I came up with something similar independently, and installed
a fix into the trunk (bzr 104441).  This fix adjusted the comment as
best I could.

Bug#8719 and Bug#8751 are related so I'll CC: this to bug 8719 to give
Kenichi Handa a heads-up.

bug#8751: 24.0.50; CCL is broken

[Kenichi Handa]

Sorry for the late response on this matter.

In article <20110530232630.273942C803A@msa105.auone-net.jp>, Kazuhiro Ito <kzhr@d1.dion.ne.jp> writes:

> > > The problem may be that ccl-compiler produce negative integer as CCL
> > > code (at latest, since Emacs21).
> > 
> > The negative number is normal (it's due to the relative instruction
> > address for the backward jump at the end of the loop).  The problem must
> > be in the ccl.c code.

> If that is right (and I think so, too), the comment of ccl.c should be
> fixed.

> > /* CCL code is a sequence of 28-bit non-negative integers (i.e. the

Yes, this comment is wrong.  It seems that I have forgotten
to update it when I changed CCL to accept negative integer.

> Here is a quick fix for actual code, but I don't check whether there
> is other problems.

> === modified file 'src/ccl.c'
> --- src/ccl.c	2011-05-27 21:24:11 +0000
> +++ src/ccl.c	2011-05-30 22:19:52 +0000
> @@ -100,6 +100,7 @@
>     CCL commands.  */
 
>  #define CCL_CODE_MAX ((1 << (28 - 1)) - 1)
> +#define CCL_CODE_MIN (- CCL_CODE_MAX - 1)
 
>  /* CCL commands
 
> @@ -756,7 +757,7 @@
>    while (0)
 
>  #define GET_CCL_CODE(code, ccl_prog, ic)			\
> -  GET_CCL_RANGE (code, ccl_prog, ic, 0, CCL_CODE_MAX)
> +  GET_CCL_RANGE (code, ccl_prog, ic, CCL_CODE_MIN, CCL_CODE_MAX)
 
>  #define GET_CCL_INT(var, ccl_prog, ic)				\
>    GET_CCL_RANGE (var, ccl_prog, ic, INT_MIN, INT_MAX)

Sorry but I still don't have a time to check the recent
change.  If the above patch fixes the code to match with the
following revised comment, it is the right patch.

/* CCL code is a sequence of 28-bit integers, each contains CCL
   command and/or arguments in the following format:

	|----------------- integer (28-bit) ------------------|
	|------- 17-bit ------|- 3-bit --|- 3-bit --|- 5-bit -|
	|--constant argument--|-register-|-register-|-command-|
	   ccccccccccccccccc      RRR        rrr       XXXXX
  or
	|------- relative address -------|-register-|-command-|
	       cccccccccccccccccccc          rrr       XXXXX
  or
	|------------- constant or other args ----------------|
                     cccccccccccccccccccccccccccc

   where, `cc...c' is a 17-bit, 20-bit, or 28-bit integer indicating a
   constant value or a relative/absolute jump address, `RRR' and `rrr'
   are CCL register number, `XXXXX' is one of the following CCL
   command codes.  */

---
Kenichi Handa
handa@m17n.org

bug#8751: 24.0.50; CCL is broken

[Paul Eggert]

On 05/30/11 22:12, Kenichi Handa wrote:
> If the above patch fixes the code to match with the
> following revised comment, it is the right patch.

Thanks, I put that comment in, as bzr 104445.

bug#8751: 24.0.50; CCL is broken

[Stefan Monnier]

Paul, can you take a look at this?  It seems this was introduced very
recently, probably by

	* ccl.c (ccl_driver): Redo slightly to avoid the need for 'unsigned'.

	ccl: add integer overflow checks
	* ccl.c (CCL_CODE_MAX, GET_CCL_RANGE, GET_CCL_CODE, GET_CCL_INT):
	(IN_INT_RANGE): New macros.
	(ccl_driver): Use them to check for integer overflow when
	decoding a CCL program.  Many of the new checks are whether XINT (x)
	fits in int; it doesn't always, on 64-bit hosts.  The new version
	doesn't catch all possible integer overflows, but it's an
	improvement.  (Bug#8719)


-- Stefan


>>>>> "Kazuhiro" == Kazuhiro Ito <kzhr@d1.dion.ne.jp> writes:

> Below code raises error "Error in CCL program at 4th code" on trunk.
> (progn
>   (define-ccl-program ccl-test
>     '(1 (loop
> 	 (read r0)
> 	 (write-repeat r0))))
>   (ccl-execute-on-string 'ccl-test (make-vector 9 0) "_"))

> -- 
> Kazuhiro Ito