At least two problems with CLM's string interning
## Describe the bug
In CLM, when strings are communicated between Lisp and `motifd`, any string that's `STRING=` to an element in a precomputed global string table is communicated by the index into the table, rather than by transmitting the characters. Lisp and `motifd` have corresponding string tables. This "string interning" is implicit, and used for all strings communicated in either direction.
This approach creates at least the following problems:
1. On the Lisp side, any CLM API call that returns a string might return an element of the string table, but the API client won't know that in general. If the client program goes on to destructively modifies the string, Lisp and `motifd` will get out of sync. So, in effect, CLM interfaces that return strings have an undocumented constraint that strings returned from CLM interfaces should not be modified. I doubt this was intended.
2. On the `motifd` side, one of the code paths that's supposed to construct and return an `XmString` doesn't handle the CLM wire encoding for interned strings. CLM programs that trigger that code path can either evince unintended behavior or crash `motifd`.
## To Reproduce
To demonstrate the first problem, we first obtain an element of `XTI::*TOOLKIT-STRING-TABLE*` via some innocuous-looking CLM interface, and then we destructively modify the string. Because `motifd` "tokenizes" all strings it sends to Lisp, an input field's input is one way to get `motifd` to transmit a `string_token` to Lisp. To trigger the unwanted behavior:
1. Evaluate the following `DEFUN`.
2. Call the program, e.g., `(input)`.
3. Type the string that's an element of the string table, e.g., `"foreground"`, into the input field, without the quotation marks, and press enter.
4. Note that Lisp output indicates that TEXT-GET-STRING returned the string `"foreground"`, and that that string is a member of Lisp's string table.
5. Clear out the input field (e.g., with backspace).
6. Type in the string `"foreground"` again, without quotation marks, and press enter.
7. Note that Lisp output indicates that TEXT-GET-STRING now returns the string `"%oreground"`, and that that string is present in Lisp's string table.
I believe the analogous consequences can occur using other CLM interfaces that return strings originating from `motifd`. In short, modifying strings that CLM interfaces return can put Lisp into a state where transmissions from `motifd` don't get represented correctly in Lisp.
```
(defun input ()
(labels ((create-input-field (parent)
(let ((text (create-text-field parent "input")))
(manage-child text)
text))
(enter-callback (widget call-data &rest client-data)
(declare (ignore call-data client-data))
(let* ((string (text-get-string widget))
(initial (copy-seq string))
(position (position string xti::*toolkit-string-table*)))
(setf (char string 0) #\%)
(format t "~&initial: ~S, position: ~A; final: ~S"
initial position string)
(finish-output)))
(main ()
(let* ((shell (create-application-shell))
(text (create-input-field shell)))
(add-callback text :activate-callback #'enter-callback)
(realize-widget shell))))
(run-motif-application
#'main :application-name "input" :application-class "Input")))
```
For the second problem, we want Lisp to communicate a string that it will tokenize in a context where `motifd` will build an `XmString` using inputs from the wire. An `XmLabel`'s `labelString` resource is required to be an `XmString`, so supplying a string that Lisp will tokenize for the `:LABEL-STRING` argument will trigger this. To reproduce unwanted behavior:
1. Evaluate the following `DEFUN`.
2. Call the program with a string that's `STRING=` to the first element in the string table, i.e., `(memo "accelerator")`.
3. When I call the program this way, I get a window containing the string `"message"`. (I believe it would be possible for the invocation to crash `motifd` instead of showing the wrong string, as well.)
Next,
4. Call the program with some extra widget initargs after the label string `(memo "accelerator" :foreground #xff0000)`.
5. When I call the program this way, `motifd` always segfaults. (In principle I think it'd be possible for `motifd` not to segfault, though ISTM unlikely.)
Finally,
6. If desired, run the program with any string that's not in the string table to verify that the program isn't itself at fault, e.g., `(memo "foo" :foreground #xff0000)`.
What's going on: the string, `"accelerator"` gets transmitted as a tag plus a `string_token`. `toolkit_read_value` decomposes the tag and `string_token`, then calls `message_read_xm_string`, with the tag and the `string_token`. `message_read_xm_string` doesn't have a case for `string_token_tag`, so it tries to read an `OID` off the wire using `message_read_oid`.
In the first case (no arguments after the `"labelString"`, `message_read_oid` will read four (possibly uninitialized) bytes from a buffer, and will likely return `NULL`. `NULL` isn't a valid `XmString`, and in this case the widget behaves as if no `"labelString"` was supplied. (I don't know whether Xt or Motif guarantees that behavior, though.)
In the second case, after the tagged `string_token` representing `"accelerator"`, `message_read_xm_string` reads the tagged `string_token` representing `"foreground"`; this will return `NULL`. Evidently, `NULL` is an invalid resource name for `XtCreateManagedWidget`.
```
(defun memo (&optional (message (error "Need a message to display.")) &rest label-initargs)
(labels ((main (message)
(let* ((shell (create-application-shell))
(msg (apply
#'create-managed-widget
"message" :label shell :label-string message
label-initargs)))
(realize-widget shell))))
(run-motif-application
#'main :init-args (list message)
:application-name "memo" :application-class "Memo")))
```
## Expected behavior
For the first problem, general I'd expect Lisp interfaces that return strings either to allow clients to destructively modify those strings freely, or else to document any restrictions on destructive modification. Further, IMO, there must be good justifications for any such restrictions; and I don't think there are in this case.
The second problem is just a common-or-garden bug, missing coverage for a wire protocol state. However, the way I propose addressing the first problem means that the offending protocol state can't happen.
## Fix for this defect
Working on it. (I'd actually already done some of the work, as I'd noticed a peculiar inefficiency in this aspect of CLM some time ago, though I hadn't noticed these problems.) ETA later today or tomorrow.
## Desktop (please complete the following information):
A Debian 10 virtual machine.
## Additional context
I observe that all the strings in the string table are resource names (analogous to slot names & initargs in CLOS). If that's all that the string table is for, then I don't believe the interning needs to be intertwined with string I/O. More results shortly, I hope.
issue