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>