[comp.windows.x] Xaw Box infinite recursion bug

stuart@rennet.cs.wisc.edu (Stuart Friedberg) (05/10/89)

			  X Window System Bug Report
			    xbugs@expo.lcs.mit.edu




VERSION:
    R3

CLIENT MACHINE and OPERATING SYSTEM:
    Sun 4/110 running SunOS 4.0.1
      probably irrelevant except for severity of resulting system lockup

DISPLAY:
    Sun BW2, but irrelevant

WINDOW MANAGER:
    uwm, but irrelevant

AREA:
    Xaw Box widget

SYNOPSIS:
    DoLayout (Box.c) can go into infinite recursion

DESCRIPTION:
    Numeric overflow in the layout algorithm can cause DoLayout to call
    itself indefinitely.  On a Sun 4/110 this amounts to crashing the
    whole machine, rather than just the X client.  A simple user error
    can trigger this behavior: trying to use "negative" vSpace and
    hSpace to overlap the borders of adjacent child widgets.

    Initialize() usually sets core.width to box.h_space, via preferred_width,
    similarly .height .  This is the source of the initial 65535 values passed
    to DoLayout.

    ChangeManaged(w)
    TryNewLayout(bbw)
    DoLayout(bbw, 65535, 65535, ..., FALSE)
      lw = 65535;
      bw = 30 * 2*1 + 65535;	/* truncated to 31 on assignment */
      if (lw + bw > 65535)	/* true, promotion to int! */
        if (lw > 65535)		/* no, they're == */
        else if (!FALSE)
          DoLayout(bbw, 65535 + 31 /* truncated to 30 on passing */, ...)
	    if (lw + bw > 30)
	      if (lw > 65535)
	      else if (!FALSE)
	        DoLayout(bbw, 30, ...)
		  DoLayout(bbw, 30, ...)
                    ...

    I think you get the picture.

REPEAT BY:
    Set vSpace and hSpace resources to (Dimension)65535 (aka -1).
    *** Be prepared to reboot to recover control on a Sun 4/110. ***
    On machines with larger shorts, use a larger value.

SAMPLE FIX:
    There are actually three problems here, and I'll make modest
    proposals about each one.  None of the "real" fixes are trivial,
    but I'd like to STRESS that a trivial user error can cause a
    catastrophic crash, and at least a patch should be offered.  I
    recommend fix (1) as the most straightforward.

    (1) Box (and other widgets!) should check its arguments for
    consistency and plausibility.  It seems reasonable to insist that
    all Dimensions handled by Boxes must be in the range [0 .. 2^15),
    rather than [0 .. 2^16).  Ideally this constraint would be enforced
    whenever new or initial resource values are provided.  But even
    that won't protect against this particular problem (as the offending
    value could be a resource of a child widget), so DoLayout should check
    for implausible bw and bbw->box.v_space values.

    (2) It would occasionally be convenient to treat hSpace and vSpace as
    Offset (should that exist) rather than Dimension.  For example, you
    can *easily* get nice looking menus if you can overlap the non-zero
    borders of adjacent items, using an box spacing of minus the border width.
    If you can't do that, you have to draw all the lines between items yourself.
    So, it would be nice if Box interpreted its resources differently.

    (3) The fundamental problem here is overflow of 16 bit integers.  If
    proposals (1) and (2) are rejected, there is no alternative but to
    test every expression that is assigned, or passed, to a Dimension
    to ensure that it is not truncated, either by design, or by run-time
    testing.

dheller@cory.Berkeley.EDU (Dan Heller) (05/10/89)

In article <7498@spool.cs.wisc.edu> stuart@rennet.cs.wisc.edu (Stuart Friedberg) writes:
>     DoLayout (Box.c) can go into infinite recursion

> REPEAT BY:
>     Set vSpace and hSpace resources to (Dimension)65535 (aka -1).
>     *** Be prepared to reboot to recover control on a Sun 4/110. ***
>     On machines with larger shorts, use a larger value.

I get the same problem all the time, but not because of negative, or
even "unreasonable" values, but because h-space and v-space is set
as well as width and height being set to incompatible values with
the width and height of  widgets within the box.

for example, the "box" likes to layout items in the box vertically
by default.  I've found that you can force horizontal layout if you
specify a width to the box when it is created.  this width should be
width enough to handle (be greater than or equal to) the sum of the
total widths of the widgets within + h-space of the box.  the height
should be great-than or equal-to the max height of a widget in the box
but not bigger than the height of the sum of any two widgets therein.

still with me?  the point is, I want to specify some width and height
of the box such that when the items are laid out within, the box
(DoLayout) doesn't try to stack the widgets any differently than left-right.

If you make a subtle error here than you will get either an incorrect
(unintended layout pattern) or the infinite recursion bug mentioned.
It depends on the mood of the system. :-)  (I haven't tested it close
enough to see where the problem really lies.)

You can see this problem if you twiddle the h-space value a little
in the boxWidget provided in the same program "prompt.c" in the WidgetWrap
code on expo.
Dan Heller	<island!argv@sun.com>