bug#24576: 25.1; desktop.el does not fully preserve registers with macros

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Dmitri Paduchikh]

Hello,

Register ?a contains keyboard macro. After loading desktop file:

(registerv-insert-func (get-register ?a))
=> "Unprintable entity"

(registerv-print-func (get-register ?a))
=> "Unprintable entity"

This makes M-x list-registers and register insertion to fail. Using
interpreted version of kmacro-to-register fixes this problem.

(registerv-insert-func (get-register ?b))
=> (lambda (k) (insert (format-kbd-macro k)))

(registerv-print-func (get-register ?b))
=> (lambda (k) (princ (format "a keyboard macro:
   %s" (format-kbd-macro k))))

In GNU Emacs 25.1.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.20.9)
 of 2016-09-18 built on juergen

Regards,
Dmitri Paduchikh

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Matthew Newton]

Apologies for reviving a stale bug but it appears to not be resolved yet.

Here is my experience with this, running my own build from a recent master:
GNU Emacs 27.0.50 (build 1, x86_64-apple-darwin18.5.0, NS appkit-1671.40 Version 10.14.4 (Build 18E226)) of 2019-04-22

;; .emacs
(require ‘desktop)
(desktop-save-mode 1)


;; before killing Emacs
(pp register-alist)
((107 . #s(registerv "\x05"
                     #[257 "\300\301\302\303\x04!\"!\207"
                           [princ format "a keyboard macro:\n   %s" format-kbd-macro]
                           6 "\n\n(fn K)"]
                     kmacro-execute-from-register
                     #[257 "\300\x01!c\207"
                           [format-kbd-macro]
                           3 "\n\n(fn K)"])))

`M-x list-registers`
Register k contains a keyboard macro:
   C-e
Register p contains a buffer position:
    buffer t.el, position 21


;; after killing Emacs and running it again
(pp register-alist)
((107 . #s(registerv "\x05" "Unprintable entity" kmacro-execute-from-register "Unprintable entity")))

`M-x list-registers`
Invalid function: "Unprintable entity”

;; Abbreviated stack trace
Debugger entered--Lisp error: (invalid-function "Unprintable entity")
  "Unprintable entity"("\5")
  #f(compiled-function (val verbose) #<bytecode 0x44180705>)(#s(registerv :data "\5" :print-func "Unprintable entity" :jump-func kmacro-execute-from-register :insert-func "Unprintable entity") nil)
  apply(#f(compiled-function (val verbose) #<bytecode 0x44180705>) #s(registerv :data "\5" :print-func "Unprintable entity" :jump-func kmacro-execute-from-register :insert-func "Unprintable entity") nil)
  register-val-describe(#s(registerv :data "\5" :print-func "Unprintable entity" :jump-func kmacro-execute-from-register :insert-func "Unprintable entity") nil)
  describe-register-1(107)
  list-registers()
  funcall-interactively(list-registers)
  call-interactively(list-registers record nil)
  command-execute(list-registers record)
  (closure ((externs) (initial-input) smex-ido-cache smex-initialized-p amx-cache amx-initialized info-lookup-mode t) (cmd) (setq cmd (intern cmd)) (cond ((and (boundp 'amx-initialized) amx-initialized) (amx-rank cmd)) ((and (boundp 'smex-initialized-p) smex-initialized-p) (smex-rank cmd))) (setq prefix-arg current-prefix-arg) (setq this-command cmd) (setq real-this-command cmd) (command-execute cmd 'record))("list-registers")
  #f(compiled-function (x) #<bytecode 0x1ff2b34c45a5>)("list-registers")



So there seem to be two bugs:

1. `desktop-save-mode` doesn’t serialize/deserialize keyboard macros properly (is it difficult to serialize a function object?)
2. Either :print-func and :insert-func should never be set to “Unprintable entity” or `register-val-describe` should handle the case where they are set to that value instead of a function. I’ve also seen “Unprintable entity” show up when a buffer position register points to a nonexistent buffer. Not sure how to reproduce that one.

I could probably dive deeper and see if I can fix this. But I would appreciate some thoughts from experienced Emacs devs about whether I understand the problem correctly. Any hints as to the best solution would also be great.

Cheers,
Matt

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Noam Postavsky]

Matthew Newton <matt@knosis.org> writes:

> Apologies for reviving a stale bug but it appears to not be resolved yet.

No apologies needed, on the contrary, thank you for looking at it.

> So there seem to be two bugs:
>
> 1. `desktop-save-mode` doesn’t serialize/deserialize keyboard macros
> properly (is it difficult to serialize a function object?)

> 2. Either :print-func and :insert-func should never be set to
> “Unprintable entity” or `register-val-describe` should handle the case
> where they are set to that value instead of a function.

The "unprintable entity" comes from desktop--v2s, looks like it doesn't
handle compiled function values, so that's why :print-func and
:insert-func get messed up like that.

> I’ve also seen “Unprintable entity” show up when a buffer position
> register points to a nonexistent buffer. Not sure how to reproduce
> that one.

I guess if you save a position in a buffer, then kill the buffer.

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Matthew Newton]

It seems that the more pressing issue is desktop handling byte code.

> On May 11, 2019, at 5:15 AM, Noam Postavsky <npostavs@gmail.com> wrote:
> 
> Matthew Newton <matt@knosis.org> writes:
> 
>> Apologies for reviving a stale bug but it appears to not be resolved yet.
> 
> No apologies needed, on the contrary, thank you for looking at it.
> 
>> So there seem to be two bugs:
>> 
>> 1. `desktop-save-mode` doesn’t serialize/deserialize keyboard macros
>> properly (is it difficult to serialize a function object?)
Would it make sense to simply do something like this?

(defun desktop--v2s (value)
  ...
  (cond
   ((byte-code-function-p value)
    (let* ((pass1 (mapcar #'desktop--v2s value))
           (special (assq nil pass1)))
      (if special
          (cons nil `(make-byte-code
                      ,@(mapcar (lambda (el)
                                  (if (eq (car el) 'must)
                                      `',(cdr el) (cdr el)))
                                pass1)))
        (cons 'may `[,@(mapcar #'cdr pass1)]))))
   …))

I copied that from the `cond` clause for vector. It works in my limited testing.

Are there security concerns or other considerations?
> 
>> 2. Either :print-func and :insert-func should never be set to
>> “Unprintable entity” or `register-val-describe` should handle the case
>> where they are set to that value instead of a function.
> 
> The "unprintable entity" comes from desktop--v2s, looks like it doesn't
> handle compiled function values, so that's why :print-func and
> :insert-func get messed up like that.
> 
>> I’ve also seen “Unprintable entity” show up when a buffer position
>> register points to a nonexistent buffer. Not sure how to reproduce
>> that one.
> 
> I guess if you save a position in a buffer, then kill the buffer.
> 
Here is what I found: if the killed buffer visits a file, it gets converted into a file-query. If there is no `buffer-file-name` then it stays a file marker pointing to nowhere. Desktop handles both of those cases correctly. So while I have seen it happen I’m not sure of the case where a buffer position register becomes an “Unprintable entity”. Did you find reproduction steps?

Cheers,
Matt

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Noam Postavsky]

Matthew Newton <matt@knosis.org> writes:

> (defun desktop--v2s (value)
>   ...
>   (cond
>    ((byte-code-function-p value)
>     (let* ((pass1 (mapcar #'desktop--v2s value))
>            (special (assq nil pass1)))
>       (if special
>           (cons nil `(make-byte-code
>                       ,@(mapcar (lambda (el)
>                                   (if (eq (car el) 'must)
>                                       `',(cdr el) (cdr el)))
>                                 pass1)))
>         (cons 'may `[,@(mapcar #'cdr pass1)]))))

I don't think `[...] will ever return a byte code function, so the
non-special case doesn't seem quite right.

> Are there security concerns or other considerations?

Well, as (info "(elisp) Byte-Code Objects") mentions, passing the wrong
args to make-byte-code can produce a function which will crash Emacs
when called, so it's somewhat high risk.

>> The "unprintable entity" comes from desktop--v2s, looks like it doesn't
>> handle compiled function values, so that's why :print-func and
>> :insert-func get messed up like that.

I note that the functions in question come from this code in kmacro.el:

(defun kmacro-to-register (r)
  ... (registerv-make
		   last-kbd-macro
		   :jump-func 'kmacro-execute-from-register
		   :print-func (lambda (k)
				 (princ (format "a keyboard macro:\n   %s"
						(format-kbd-macro k))))
		   :insert-func (lambda (k)
				  (insert (format-kbd-macro k))))

While in register.el we have:

(cl-defun registerv-make (data &key print-func jump-func insert-func)
  ...
  (declare (obsolete "Use your own type with methods on register-val-(insert|describe|jump-to)" "27.1"))

So perhaps this is a good opportunity to change the kmacro register
stuff to use the non-obsolete way.
 
>>> I’ve also seen “Unprintable entity” show up when a buffer position
>>> register points to a nonexistent buffer. Not sure how to reproduce
>>> that one.
>> 
>> I guess if you save a position in a buffer, then kill the buffer.
>> 
> Here is what I found: if the killed buffer visits a file, it gets
> converted into a file-query. If there is no `buffer-file-name` then it
> stays a file marker pointing to nowhere. Desktop handles both of those
> cases correctly. So while I have seen it happen I’m not sure of the
> case where a buffer position register becomes an “Unprintable
> entity”. Did you find reproduction steps?

Ah, no, I was just guessing based on your initial description, no idea
how to reproduce it if desktop.el is already handling these cases.

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[Dmitri Paduchikh]

Noam Postavsky <npostavs@gmail.com> wrote:

>>> The "unprintable entity" comes from desktop--v2s, looks like it doesn't
>>> handle compiled function values, so that's why :print-func and
>>> :insert-func get messed up like that.

NP> I note that the functions in question come from this code in kmacro.el:

NP> (defun kmacro-to-register (r)
NP>   ... (registerv-make
NP>                    last-kbd-macro
NP>                    :jump-func 'kmacro-execute-from-register
NP>                    :print-func (lambda (k)
NP>                                  (princ (format "a keyboard macro:\n   %s"
NP>                                                 (format-kbd-macro k))))
NP>                    :insert-func (lambda (k)
NP>                                   (insert (format-kbd-macro k))))

This must be easy to fix. Just defun these anonymous functions and use their
names in place of lambdas. Or any newer approach, of course.

All the best.
Dmitri Paduchikh

bug#24576: 25.1; desktop.el does not fully preserve registers with macros

[npostavs]

tags 24576 fixed
close 24576 27.1
quit

> So perhaps this is a good opportunity to change the kmacro register
> stuff to use the non-obsolete way.

Lars did this, so I think the bug should be fixed now.

[1: 26f2b1f]: 2019-06-12 18:21:35 +0200
  Rewrite the kmacro register function to avoid using obsolete functions
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=26f2b1faaa1dc8847f2013268c20f51c144ae711