bugGNU Zile (Zile is Lossy Emacs) - Bugs: bug #33910, execute-kbd-macro crashes Lua Zile...

 
 

bug #33910: execute-kbd-macro crashes Lua Zile when it gets key codes > 255

Submitted by:  Gary V. Vaughan <gary>
Submitted on:  Sun 31 Jul 2011 01:33:08 PM UTC  
 
Category: LuaSeverity: 5 - Blocker
Status: FixedAssigned to: Gary V. Vaughan <gary>
Open/Closed: ClosedRelease: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Fri 03 Feb 2012 03:37:01 PM UTC, comment #18:

Fix is now ported to the lua branch, along with all the necessary supporting code. I'm not entirely happy with the ugliness of the code in the lua branch, particularly where we need to keep so many conversion maps:

1. read syntax -> keycode
2. keycode -> read syntax
3. keycode -> display syntax

And these are particularly messy because some keycode atoms can be reached from several valid read syntaxes: \t === \\t === \\TAB.

None the less, as of changeset 908135c5, this bug is now squashed :)

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Wed 18 Jan 2012 02:26:14 AM UTC, comment #17:

Fix is in the `zi' branch already, but it relies on the rewrite of keycode handling I did for that branch (not using curses keypad mode, and looking up and pushing back into the key buffer of terminal escape sequences is performed in lua with the help of tigetstr).

I've made a start in porting the subset of patches that also make sense on the lua branch (hence the currently failing #33910 regression test), with more to come soon... all of which will make for rebasing zi on the lua branch head when we're both done a much smaller selection of differences:

1. rip out the lisp engine
2. consolidate read and write syntaxes for keypresses
3. syntax parser and highlighter (much later)

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Tue 17 Jan 2012 06:29:45 PM UTC, comment #16:

You've got curses.tigetstr now; please can we have a fix? :)

Reuben Thomas <rrt>
Project Administrator
Thu 08 Sep 2011 05:05:52 AM UTC, comment #15:

I haven't yet committed a fix to prevent crashing Lua Zile with (execute-kbd-macro "\C-q\F1"), for which I first needed curses.tigetstr().

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Wed 07 Sep 2011 09:25:55 PM UTC, comment #14:

Am I missing something here? I have fixed this in C, and Gary, you seem to have fixed it in Lua, so why's the bug still open?

Reuben Thomas <rrt>
Project Administrator
Thu 18 Aug 2011 09:32:22 PM UTC, comment #13:

Agreed, and I like your solution because it enables my code to be simplified considerably as the unfiltered getkey routines now only need to get one character. Fixed.

Reuben Thomas <rrt>
Project Administrator
Thu 18 Aug 2011 08:04:26 PM UTC, comment #12:

+1 on "disallow key codes > 255 in string read-syntax"

Regarding the ideal way to to implement quoted-insert, it seems to me that emacs states that even if you are in interactive mode, when you press C-q, emacs only promises not to interpret the next character. And sure enough, if I reprogram my F1 key to emit to ^[ characters, and then in emacs I press `C-q F1'... the first ^[ is inserted into the buffer, at which point quoted insert is finished, and since the rest of the goop that comes from pressing my reprogrammed F1 matches the terminfo entry for an F1 key press event, emacs shows <f1> in the mini buffer and is awaiting my next key.

Now that I've moved the better quoted-insert stress test into master, your new term_getkey_unfiltered loop wraps up the whole of key_buf and returns it *codes. Now, for any long keyboard macro to that calls quoted_insert (remembering that execute-kbd-macro has used ungetkey to put the whole keyboard macro into key_buf), the term_getkey_unfiltered loop pulls the rest of keyboard macro out of key_buf and sends it back as if it had been generated by a single key press.

I guess the inserting-a-TAB-instead-of-escaped-keycode bug I've mentioned in the comments a couple of times is a possible symptom of the same bug?

I've fixed the whole-key_buf-return problem in Lua Zile, with 8babc3f.

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Thu 18 Aug 2011 04:08:42 PM UTC, comment #11:

The correct fix would be to have the key buffer work on unfiltered key codes. Then, instead of returning key codes directly from the buffer, they would be passed to ungetch. This would mean that key handling would be pretty ugly: as there is no curses call to turn curses keycodes back into bytes, this requires either reimplementing the conversion, or capturing all bytes before ungetch-ing them to get the code, and then having some way to forget the bytes once the keycode has been definitively consumed. Essentially a nasty programming problem.

I'm inclined to Emacs's solution: disallow key codes > 255 in the string read-syntax. So that would mean not allowing any fancy non-modifier keys, right? If we ever really want this, we can implement the vector read syntax.

Reuben Thomas <rrt>
Project Administrator
Thu 18 Aug 2011 12:34:11 PM UTC, comment #10:

I have reverted my patch for #33906 from the Lua branch and ported your better patch to lua, however I can still easily crash Lua Zile:

$ echo '(execute-kbd-macro "\C-x\C-b\RET\C-q\F1")' > die.el
$ ./src/zile -l die.el

Possibly I've introduced a similar but different bug while porting. I'll keep looking into this one.

While C Zile doesn't crash, it does accept and push into key_buf any key codes > 255, but then mangles them when quote-inserting from the key_buf:

$ echo '(execute-kbd-macro "\C-x\C-f,,t\RETb\C-q\SPCm\C-q\F1e\C-x\C-s\C-x\C-c")' > mangle.el
$ ./src/zile -l mangle.el

Now the file ,,t contains:

b<a literal space>m<a literal tab>e

C Zile mangled the KEY_F1 pushed into key_buf before quote inserting it into the buffer.

My question from comment #4 still stands, and the answer possibly holds the key to a solution...

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Thu 18 Aug 2011 12:14:43 PM UTC, comment #9:

Re: #33906

ah yes, thanks... I was wondering why I couldn't find anything relevant in my mail archive!

And, I do agree that #33096 is fixed.

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Wed 17 Aug 2011 02:16:40 PM UTC, comment #8:

I think this issue is not a problem on the C branch. Can you confirm/deny? Certainly, the structure of the code you discuss has changed on the C branch, and further I can't see any reason why codes bigger than 255 would be a problem there.

Reuben Thomas <rrt>
Project Administrator
Wed 17 Aug 2011 01:40:40 PM UTC, comment #7:

The "other issue" to which you refer was #33906, which I believe I fixed. Can you confirm or deny that before I look into this one in C Zile?

Reuben Thomas <rrt>
Project Administrator
Wed 17 Aug 2011 06:27:36 AM UTC, comment #6:

On reflection, the behaviors differ because you implemented a different fix to the bug on master than my v2.4.0alpha1-140-ge07a801 on the lua branch.

We didn't resolve that issue either, except that your last email said that "both our solutions only worked by accident".

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Wed 17 Aug 2011 06:24:58 AM UTC, comment #5:

Interestingly, C Zile doesn't crash if you feed key codes > 255 to execute-kbd-macro, but neither does it insert anything worthwhile (I keep seeing tab chars for some reason?!?)

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Wed 17 Aug 2011 06:24:11 AM UTC, comment #4:

Revised the summary to describe the more general problem that the original report was just one symptom of, and re-opened...

We need to determine the best way to prevent crashing whenever reading back a curses key code that was pushed into the ungetkey buffer; consider:

1. In interactive mode C-q <f1> inserts ^[OP (or similar) into the current buffer, as does Emacs.
2. Evaluating (using the new M-x eval-buffer!) '(execute-kbd-macro "\C-q\F1")' ends up pushing curses.KEY_F1 into the buffer, so that when term_xgetkey fetches it in UNFILTERED mode, it is out of range and crashes Zile :(

It sort of makes sense that the kbd-macro should do the same as interactive mode, only by the time curses.getchar has returned a code in keypad mode, we've already lost the key sequence details, and certainly don't want to have to dig around in terminfo to look it up!

Emacs simply disallows key codes > 255 in the string read-syntax, but Zile doesn't have a key vector read-syntax so don't necessarily want to just disallow it, unless it can be done only in keyboard macro strings?

I see there is code to convert back and forth between "\\F1" and KBD_F1 in keycodes.lua, but it's not clear to me where else this is used - presumably in more places than just execute-kbd-macro?

What to do?

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Mon 01 Aug 2011 01:53:53 AM UTC, comment #3:

I look forward to cross pollinating our respective fixes into the other branches then :)

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Sun 31 Jul 2011 10:03:14 PM UTC, comment #2:

That's certainly possible. Recent code changes to master make it less likely there by bringing more of the interactive code on to the non-interactive path.

Reuben Thomas <rrt>
Project Administrator
Sun 31 Jul 2011 09:57:20 PM UTC, comment #1:

Fixed the bug with commit v2.4.0alpha1-140-ge07a801, but the execute-kbd-macro test case failed to crash Lua Zile even though manually entering those same keys did!?!

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.
Sun 31 Jul 2011 01:33:08 PM UTC, original submission:

Possibly related to the "\C-q\C-h inserts ^G" bug in C Zile, but for Lua the whole application crashes with the following stack trace:

/usr/local/bin/lua: /Users/gary/Devo/zile--savannah--0/src/funcs.lua:368: bad argument #1 to 'char' (invalid value)
stack traceback:
[C]: in function 'char'
/Users/gary/Devo/zile--savannah--0/src/funcs.lua:368: in function 'func'
/Users/gary/Devo/zile--savannah--0/src/lisp.lua:56: in function </Users/gary/Devo/zile--savannah--0/src/lisp.lua:36>
(tail call): ?
/Users/gary/Devo/zile--savannah--0/src/bind.lua:69: in function 'process_command'
/Users/gary/Devo/zile--savannah--0/src/main.lua:354: in function 'main'
./src/zile:39: in main chunk
[C]: ?

Gary V. Vaughan <gary>
Project AdministratorIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rrt (Posted a comment)
  • -unavailable- added by gary (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 17 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 03 Feb 2012 03:37:33 PM UTCgaryOpen/ClosedOpen=>Closed
    Fri 03 Feb 2012 03:37:01 PM UTCgaryStatusIn Progress=>Fixed
    Fri 19 Aug 2011 04:23:45 AM UTCgaryStatusNeed Info=>In Progress
    Fri 19 Aug 2011 04:18:02 AM UTCgaryStatusFixed=>Need Info
      Open/ClosedClosed=>Open
    Thu 18 Aug 2011 09:32:22 PM UTCrrtStatusNeed Info=>Fixed
      Open/ClosedOpen=>Closed
    Wed 17 Aug 2011 06:24:58 AM UTCgarySummary\\C-q\\C-h crashes Lua Zile=>execute-kbd-macro crashes Lua Zile when it gets key codes > 255
    Wed 17 Aug 2011 06:24:11 AM UTCgaryStatusFixed=>Need Info
      Open/ClosedClosed=>Open
      Summary\\C-q\\C-h crashes Lua Zile=>\C-q\C-h crashes Lua Zile
    Mon 01 Aug 2011 01:53:53 AM UTCgarySummary\\C-q\\C-h crashes Lua Zile=>\C-q\C-h crashes Lua Zile
    Sun 31 Jul 2011 10:03:14 PM UTCrrtSummary\\C-q\\C-h crashes Lua Zile=>\C-q\C-h crashes Lua Zile
    Sun 31 Jul 2011 09:57:20 PM UTCgaryStatusNone=>Fixed
      Assigned toNone=>gary
      Open/ClosedOpen=>Closed
      Summary\\C-q\\C-h crashes Lua Zile=>\C-q\C-h crashes Lua Zile

    Back to the top


    Powered by Savane 3.1-cleanup1