EditRes functionality is unreliable on 64 bit builds
So I wanted to poke at widget resources in a Motif application using editres, but found the following somewhat twisty set of issues. It starts in libXmu's EditResCom.c, appears to affect any 64 bit Xaw client, has cosmetic effects on 64 bit editres as an application, and affects 64 bit Motif in a silly way. The net effect is that editres is unlikely to be able to edit resources of widgets in a 64 bit program except by chance.
I'm attaching a working draft of what I think ought to be a general fix for this stuff.
Background
EditRes lets one program (e.g., editres) query and manipulate resources and other info associated with Widgets in another program. (Let's call the second program the /editEE/, and programs like editres the /editOR/.)
libXmu's src/EditResCom.c implements everything an editee needs to participate in EditRes, as well as some functionality an editor might want. In particular, this file contains an implementation of
- a set functions and types to make sending and receiving EditRes events resemble buffered stream I/O (e.g.,
ProtocolStream,_XEditResPut32/_XEditResGet32, et al.), - an EditRes event handler,
_XEditResCheckMessages, that uses those I/O operations, and - some static functions
_XEditResCheckMessagescalls, - and static globals that
_XEditResCheckMessagesand its callees assign and use.
Note that the I/O functionality and the event handler are public, but the handler encapsulates all the functionality logically "between" the event and the I/O functions. So there end up being 3 kinds of "user" of libXmu's src/EditResCom.c:
-
editresuses the I/O functionality to act as an editor, - programs that install
_XEditResCheckMessagesuse that handler to act as an editee, - programs that install Motif's EditRes event handler use libXmu's I/O functionality, but not its event handler.
Next, the EditRes protocol uses 32 bit integers, called IDs, to refer to Widgets in the editee. (I can't find the original paper on EditRes, but I believe that IDs have no semantics or constraints beyond that it's good if they're globally unique within an editee's report of its widget tree in reply to a SendWidgetTree request.)
When _XEditResCheckMessages has to transmit a Widget ID to an editor, it does so by extracting the low 4 bytes of the Widget, and passing those 32 bits to one of the public I/O operations, either _XEditResPut32 or _XEditResPutWidgetInfo. Obviously, in a 32 bit editee, that's the whole pointer; such an editee can receive that 32 bit integer back from the editor and use it as a pointer verbatim.
Problem
There's a simple bug, an API defect, and a general design flaw in the approach libXmu uses for turning 32 bit IDs "back" into 64 bit pointers. This affects both editres as an editor and any editee that uses libXmu.
The bug and the defect start in _XEditResGetWidgetInfo (linking to the latest commit as of the time of this bug filing):
The bug is at line 1952, which allocates but doesn't initialize the buffer it then writes 32-bit IDs into. So the high 4 bytes of each long might be nonzero junk. Changing that XtMalloc to XtCalloc solves that problem, and suffices to allow Xaw clients to have their resources peeked and poked at by editres, modulo the design flaw.
The API defect is that the IOR at line 1963 uses globals.base_address. globals is a static variable declared at line 182:
and the base_address member gets assigned at line 503, in the static function ExecuteCommand, which is called only within _XEditResCheckMessages:
That is, the set of public I/O operations depend on a static that only gets initialized in editees that use libXmu's _XEditResCheckMessages.
This has two observable consequences:
-
When
editresis acting as an editor, the high 4 bytes of its widget IDs will tend to be random garbage. (This is only an aesthetic issue when viewing Widget IDs, sinceeditresdoes not send those high 4 bytes to editees.) -
Motif's EditRes handler, which uses the I/O operations, can't initialize libXmu's
globals.base_address. (And so 64 bit Motif programs will get pointers with random contents in their high 4 bytes from the I/O operations. By inspection, editres will typically not be able to manipulate resources in 64 bit Motif programs at the moment.)
IOW, the API defect is that the I/O interface looks orthogonal to the handler, but actually depends on initialization that only happens in one kind of libXmu client, programs that call _XEditResCheckMessages and that have received EditRes events.
The general design flaw is that using the high 4 bytes of the last shell widget that handled a SendWidgetTree request seems like the sort of thing that can go wrong in a long-running program or get tripped up by funny malloc implementations.
Reproduction
- Start some 64-bit client that supports the EditRes protocol to serve as editee.
- Any Xaw or Motif application should do.
- Let's say it's "xmessage Hello"
- Start editres
- (Commands > Get Tree), click on the editee's window.
- In the tree view, select the label for some widget that's visible in the editee's window (e.g., for xmessage, click on the "okay" label)
- (Tree > Flash Active Widget)
- Probable outcome: editres will show "This widget no longer exists in the client." (whereas, in fact, the widget does exist).
- Also possible: a solid color window will appear and disappear over the selected widget in the editee's window. (This can happen "by accident" in an Xaw client if the
XtMalloccall in_XEditResGetWidgetInforeturns a zeroed buffer. It seems exceedingly unlikely to happen in a Motif application even then.)
- Note that (Tree > Show Widget IDs) should show some IDs that are 8-byte aligned.
Proposed solution
The change I have attached does a couple things:
- It adds 4 new "I/O" functions, resembling existing ones, that indirect through an internal lookup table between Widgets and Widget IDs
- These are called
_XEditResGetWidgetInfoResolve,_XEditResGetWidgetResolve_XEditResPutWidgetInfoUnresolve,_XEditResPutWidgetUnesolve. - The lookup table is entirely encapsulated within libXmu; in particular, it gets initialized implicitly by those I/O functions.
- These are called
- It modifies
_XEditResCheckMessageshandler and all its callees to use these new functions. - It makes these 4 "I/O" functions public, so that toolkits that define their own EditRes event handler can benefit from the lookup table functionality.
- For example, to get 64 bit Motif to handle EditRes events, it's sufficient simply to put these 4 new functions into Motif's fork of EditResCom.c in exactly the places where they're called in libXmu's.
I believe these changes are backward compatible in every sense: existing Xaw applications will get the new implementation of _XEditResCheckMessages the next time they run; applications that use existing Motif libraries will continue to not be manipulable by editres, but with a trivial set of changes to Motif, those applications will regain EditRes capabilities.
Commentary on the working draft
The working draft is presented for the purpose of gathering feedback. It's not suitable for inclusion in libXmu as-is. In particular, contains a lot of debugging code that is not intended to go into the canonical tree. It's also more verbosely commented than the rest of the file.
I decided to insinuate the idea of a lookup table at the I/O level because everything "higher" than that is currently private to libXmu's EditResCom.c (and effectively forked in Motif).
Because libXmu's existing public _XEditRes* functions get used in existing libXmu clients, it's necessary to keep their interfaces stable, i.e., none of the existing public functions use the lookup table implicitly. In particular, in order for an editres process to speak the EditRes protocol with itself, the /editOR/ portion of the editres process needs to Get and Put 32 bit IDs verbatim, while the /editEE/ portion of that process needs to perform the Widget<->WidgetID conversion. (Lightly tested, but this appears to work.)
There's nothing especially deep about the design of the lookup table. It's mostly just a pair of open addressing hash tables, along with some extra doodads for handling the (farfetched) possibility of running out of WidgetIDs. I've done no analysis of whether the hashing is any good, and in fact I'm not hashing pointers at all at the moment. Because the lookup table is totally encapsulated by the 4 new "I/O" routines, it could be replaceable by something else later.
I do not understand enough about concurrency within Xt to know whether the lookup table needs to be protected by locks, be reentrant, etc. I'd appreciate any advice on that.
Handling the possibility of exhausting of the 32 bit Widget ID space is almost certainly overengineered relative to the probability of exhaustion occurring, but I did it anyway, and it works in my testing. (Specifically, an editee would need to have issued roughly 2^32 WidgetIDs in response to SendWidgetTree requests over its lifetime in order to run out of IDs. I doubt that can happen in reality.)
Anyhow, my main goal is getting editres working again for Xaw and Motif applications; the working draft does so for me, but I'm open to other approaches. I'd appreciate any feedback/pointers/tips on what people might consider acceptable solutions here.
0001-Draft-Change-how-EditResCom.c-converts-between-Widge.patch