Bug 253456

This bug is actually in lesstif somewhere. I can confirm it by replacing the
rawhide lesstif package with the .f7 package (lesstif-0.95.0-15.fc7.x86_64.rpm)
and relaunching any lesstif app (ddd and xpdf show this failure).

Does an xpdf rebuild against the latest lesstif fix the
issue?

No, but I've narrowed it down to: lesstif-64.patch

If that patch is applied to lesstif, then apps(xpdf, ddd) don't get the correct
WM hinting. If that patch is removed, they work properly.

Rebuilding xpdf doesn't make a difference.

lesstif-64.patch is just wrong:

CARD32 and INT32 are defined in /usr/include/X11/Xmd.h as:

#ifdef LONG64
typedef int INT32;
#else
typedef long INT32;
#endif

#ifdef LONG64
typedef unsigned int CARD32;
#else
typedef unsigned long CARD32;
#endif

This patch redefines them only to their 32bit values, breaking all LONG64 arches
(alpha, ia64, sparc64, s390x, x86_64, ppc64).

I just hit this too (x86_64), nasty bug. I've added the author of the patch to 
the CC. kgallowa, can you explain the how and why of this patch, AFAIK it was
written to fix some issues with icedtea on 64 bit, but as Spot explains very
good the comment above this comment, the patch seems completely wrong (and
breaks all existing lesstif apps). So can you explain what was going wrong with
icedtea and lesstif on 64 bit and how this patch fixes it?

I can give you what Kyle explained me:

I am writing this about an issue we (being the team working on
OpenJDK/IcedTea) have been having with lesstif.  Lesstif, unlike OpenMotif,
uses a type it calls CARD32 and an INT32 to represent the members of the
PropMwmHints struct.  OpenMotif uses unsigned longs and longs repectively.
Because of this, the sizes of the values are different on x86 and x86_64
systems, which causes window decorations not to appear when building the
OpenJDK against lesstif.  Since we cannot ship openmotif, we need to have
consistency.

lesstif is not API/ABI compatible with OpenMotif on x86_64 systems.  On
32-bit systems, a long is a 32bit value, but on a 64bit system a long is
64bits long because the type is defined as the size of a memory address.
In openmotif, they use longs in these structures meaning that the size of
their structures is architecture dependent.  In lesstif, since the types
are always 32bits, even on 64bit systems, it is not, in fact, ABI
compatible with OpenMotif.  I'm not asking for lesstif to change their ABI,
simply to become compliant with OpenMotif.


That seemed to make sense to me. Maybe what is needed is more patching 
to the code in the lib that uses the structures?

I asked advice on the lesstif list but sadly nobody responded. lesstif
seems to be completely dead.

Okay,

I've taken a quick peek at how openmotif defines the 2 involved structs in its
headers. I do not believe that this constitutes copyright infringement because
only a small sniplet is involved and what is expressed there cannot be expressed
in many other ways, but to be sure lets do this the cleanroom way, so no code
from me, only a description.

The problematic structure is the Motif window manager hints structure. This
structure has 2 representations, the real one as seen by the X-server and window
manager and an internal one called properties representation in motif speak.

Funny / strange enough the real struct has 32 bits integers in it, and the
properties version of the struct has 64 bits integers.

So neither the old nor the new code is correct. For reasons given above I'll
leave writing a patch for this to someone else, and I will not be giving any
feedback on this patch.

Kyle, can you please check if icedtea still works once this patch is written?
Thanks!

I am not a lawyer, but I recall having read that an API cannot
be copyrighted.

This isn't related to that.

Hans (or anyone) can't look at the openmotif code, then write a patch which is
essentially copying that code into lesstif. What he can do is a cleanroom
approach, where he, without any code, describes the issue and tells someone else
how to write the code for lesstif.

And while neither the old nor the patched code may be correct, the code with
Kyle's patch in it definitely breaks all existing lesstif based apps in Fedora.

I'd suggest that Kyle should look at how existing lesstif apps (e.g. xpdf and
ddd) access the PropMwmHints struct.

I just realized I made a mistake in the description of how openmotif declares
the struct. Here is a fixed description:

The problematic structure is the Motif window manager hints structure. This
structure has 2 representations, the real one as seen by the X-server and window
manager and an internal one called properties representation in motif speak.

Funny / strange enough the real struct has 32 bits integers in it on all
platforms, and the properties version of the struct has 64 bits integers on 64
bit platforms and 32 bits integers on 32 bit platforms.

---

p.s. For the lawyers under us I only looked at openmotif's MwmUtil.h, and only
looked up the 2 struct definitions in that file. I haven't looked at any other
openmotif code and have removed openmotif immediately after peeking at the 2
structs.

Patrice has told me in another lesstif bug, to go ahead and fix that bug. While
I was at it I also disabled the patch causing this problem, thus I'm closing
this bug.

To the icedtea team: I'm sorry about this but you're patch really is breaking
more then it fixes, please do as Spot suggested and try to rewrite the involved
code so that it doesn't care about the size of the integers in the PropMwmHints
struct (it should gets that size from the system MwmUtil.h header, so that it
matches what ever the system motif uses), or write a lesstif patch which doesn't
cause regressions elsewhere.

I'm more then willing to apply such a patch if one is written (in case Patrice
is short on time like he was with the other bug).

I just had an eureka moment while bicycling from home to work. Yesterday I fixed
a 64 bit lesstif cut and paste issue, see bug 243508, and today I realized this
cut and paste bug has the same root cause as the icedtea 64 bit problem.

The problem is some braindeadness in XGetWindowProperty and XChangeProperty,
when setting 32 bits (format == 32) properties, the data pointer which should be
passed / is returned, should be / is a pointer to an array of longs. So when
Changing 32 bits properties on x86_64 you cannot directly pass / memcpy the data
to store in the property, instead it first must be copied a 32 bit int at a time
to an array of longs. The reverse happens when retrieving a property.

Ao maybe IcedTea is calling XGetWindowProperty or XChangeProperty directly
itself, passing one of the MwmHints structures.

If it is not, then IcedTea is calling some lesstif function which does this
wrong, seeing how lesstif did this wrong in the CutPaste.c code, I guess it does
it wrong in other places too. I'll audit lesstif for this and write a patch for
any other wrong uses of XGetWindowProperty / XChangeProperty.

I've just added a very similar comment to:
http://iced-tea.org/bugzilla/show_bug.cgi?id=37

Please do any further discussion of this there, I just added this comment here
for those interest reading this bug. If you want to follow this further please
add yourself to the CC of the icedtea bug.

---

Shortly after the eureka moment, I also had a @$#%^#@&&!! moment, by temporarily
 accepting the icedtea patch we have changed the ABI of lesstif, and now we have
changed it back again, and this in the midst of a mass rebuild period, OOPS!

We might be lucky though, as most motif programs will not manipulate the mwm
hint structs themselves, but we do need to check them all, and where necessary
rebuild.

So does anyone have any good script-fu to get a list of all lesstif using packages?

---

This also brings us to another question, currently lesstif is not ABI compatible
with motif, only API, do we want to accept patches to lesstif to get closer to
ABI compatibility and if we do how do we handle this? Spare them all up till
right after a test1 release and bump the soname? I'm tending towards maintaining
the stance that lesstif is not ABI compatible with motif, and thus keeping
lesstif's ABI and soname stable, this is also important in case people try to
run binaries from other dists on rawhide. We really want our libs to be ABI
compatible with other dists ((assuming they are the same version).