Path: utzoo!utgpu!jarvis.csri.toronto.edu!mailrus!csd4.milw.wisc.edu!dogie.macc.wisc.edu!uwvax!rennet.cs.wisc.edu!stuart
From: stuart@rennet.cs.wisc.edu (Stuart Friedberg)
Newsgroups: comp.windows.x
Subject: Xaw Box infinite recursion bug
Message-ID: <7498@spool.cs.wisc.edu>
Date: 9 May 89 22:38:08 GMT
Sender: news@spool.cs.wisc.edu
Reply-To: stuart@rennet.cs.wisc.edu (Stuart Friedberg)
Organization: U of Wisconsin CS Dept
Lines: 88


			  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.