On Sat, 29 Sep 2012, Jon Trulson wrote: > On Sun, 30 Sep 2012, Marcin Cieslak wrote:
> Looks like menory corruption bug somewhere - somethingis scribbling > over this memory... No, va_args promotes chars and shorts to ints, but it does not automatically promote ints to longs. > > Providing a long value on None (0L) as in this change fixes the > > problem. > > > > I understand is that it possible to use such an "empty" widget > > is to create additional space at the bottom of the newly created > > "icon_scrolled_win". > > > > What needs to be clarified - shouldn't be such an (int) value be > > automatically promoted to (long) (or XtArgVal, XtPointer, ...) > > Yes, there is not problem I am aware of with int 0 being promoted to > long 0. I've never heard of a problem where ints are erroneously > promoted to longs. The other way around of course is problematic, > but... there is something else wrong here. The problem is, they are *not* promoted (fine according to C, a bit troublesome whem coming to the X Toolkit). TL;DR: I have reasons to believe that my fix is correct :) Here's what happens: XtVaCreateManagedWidget invocation looks like this before patching: Let's create the assembler file in cde/programs/dtcreate with the command: gcc -c -S -g -pipe -I. -I../.. -I../../exports/include -I../../imports/motif/include -I../../imports/x11/include -I/usr/local/include -DCSRG_BASED -DANSICPP -DMULTIBYTE -DNLS16 -DOSMAJORVERSION=10 -DOSMINORVERSION=0 icon_selection_dialog.c icon_selection_dialog.s: 3983 .loc 1 1768 0 /* ... */ 4004 movq UxIcon_selection_dialogContext(%rip), %rax 4005 movq (%rax), %rax 4006 movq %rax, -1128(%rbp) 4007 movq xmScrolledWindowWidgetClass(%rip), %rax 4008 movq %rax, -424(%rbp) 4009 movq $0, 216(%rsp) <-- final NULL, 64-bit 0 4010 movl $3, 208(%rsp) <-- XmATTACH_WIDGET, 32-bit 3 4011 movq %rdx, 200(%rsp) 4012 movl $0, 192(%rsp) <-- 32-bit 0 for XmNbottomWidget 4013 movq %rcx, 184(%rsp) 4014 movl $10, 176(%rsp) <-- 32-bit 10 for XmNbottomOffset 4015 movq %rsi, 168(%rsp) 4016 movl $4, 160(%rsp) <-- XmATTACH_OPPOSITE_WIDGET, 32-bit 4 4017 movq %rdi, 152(%rsp) 4018 movq %r8, 144(%rsp) 4019 movq %r9, 136(%rsp) 4020 movl $0, 128(%rsp) 4021 movq %r10, 120(%rsp) 4022 movl $3, 112(%rsp) 4023 movq %r11, 104(%rsp) 4024 movq %r12, 96(%rsp) 4025 movq %r13, 88(%rsp) 4026 movl $0, 80(%rsp) 4027 movq %r14, 72(%rsp) 4028 movl $1, 64(%rsp) 4029 movq %r15, 56(%rsp) 4030 movl $10, 48(%rsp) 4031 movq -464(%rbp), %rdx 4032 movq %rdx, 40(%rsp) 4033 movl $1, 32(%rsp) 4034 movq -456(%rbp), %rcx 4035 movq %rcx, 24(%rsp) 4036 movl $84, 16(%rsp) <-- 84 32-bit for XmNy 4037 movq -448(%rbp), %rax 4038 movq %rax, 8(%rsp) 4039 movl $282, (%rsp) <-- 282 32-bit for XmNx 4040 movq -440(%rbp), %r9 4041 movl $0, %r8d 4042 movq -432(%rbp), %rcx 4043 movq -1128(%rbp), %rdx 4044 movq -424(%rbp), %rsi 4045 movl $.LC49, %edi 4046 movl $0, %eax 4047 call XtVaCreateManagedWidget In the above listing we can see that plain integer values (int, enum, etc.) are stored on stack using 32-bit movl. Long, pointer values are stored on stack using 64-bit movq. As stack values are aligned on 8 bytes anyway, this means that upper 32 bits are left as they were - that means they are undefined. My patch changes only this: 4012 movl $0, 192(%rsp) <-- 32-bit 0 for XmNbottomWidget to this: 4012 movq $0, 192(%rsp) Because XmATTACH_NONE - 32 bit integer zero becomes long None - 64 bit integer zero and a whole 64 bit word is cleared. Further we need to examine _XtVaToTypedArgList from X Toolkit that fetches variadic arguments from stack. This is the fragment that deals with normal (name, value) pairs: Varargs.c of libXt-1.1.1: 505 } else { 506 args[count].name = attr; 507 args[count].type = NULL; 508 args[count].value = va_arg(var, XtArgVal); 509 ++count; 510 } This means that every value is taken from the stack and coverted to XtArgVal value, which means a whole 64 bit value is taken in our case. We can reproduce this if we have libXt sources and debug symbols somewhere around. For this we can use a small gdb script: ---8<--- .gdbrc begin --- break icon_selection_dialog.c:1768 run break Varargs.c:507 cont display attr display args[count].value ---8<--- .gdbrc end --- then we start dtcreate: gdb -x .gdbrc ./dtcreate and then we just click "Find Set..." it should stop: Breakpoint 1 at 0x42112b: file icon_selection_dialog.c, line 1768. Breakpoint 1, _Uxbuild_icon_selection_dialog () at icon_selection_dialog.c:1768 warning: Source file is more recent than executable. 1768 icon_scrolled_win = XtVaCreateManagedWidget( "icon_scrolled_win", Breakpoint 2 at 0x801910814: file Varargs.c, line 507. Breakpoint 2, _XtVaToTypedArgList (var=0x7fffffffbb40, max_count=15, args_return=0x7fffffffbae8, num_args_return=0x7fffffffbae4) at Varargs.c:507 507 args[count].type = NULL; (gdb) n 508 args[count].value = va_arg(var, XtArgVal); 2: args[count].value = -6510615555426900571 1: attr = 0x62c327 "scrollingPolicy" (gdb) n 509 ++count; 2: args[count].value = 0 1: attr = 0x62c327 "scrollingPolicy" so there the value 0 is read for the scrollingPolicy parameter. Let's contine and single step for the next parameter till we reach line 509 again: 508 args[count].value = va_arg(var, XtArgVal); 2: args[count].value = -6510615555426900571 1: attr = 0x630115 "x" (gdb) n 509 ++count; 2: args[count].value = 34359738650 1: attr = 0x630115 "x" What is 34359738650? (gdb) print args[count].value & 0xffffffff $2 = 282 So, we have fetched a full 64-bit value from the stack and only lower 32-bit were set by the "movl" instruction by the caller. This is *not* a problem because later this will be converted to Dimension or some other 32-bit type and upper bits will be truncated and the value "282" will be used. Let's type few times "cont" until we get to the "bottomOffset": 509 ++count; 2: args[count].value = 10 1: attr = 0x62b20c "bottomOffset" (gdb) cont Continuing. Breakpoint 2, _XtVaToTypedArgList (var=0x7fffffffbb40, max_count=15, args_return=0x7fffffffbae8, num_args_return=0x7fffffffbae4) at Varargs.c:507 507 args[count].type = NULL; 2: args[count].value = -6510615555426900571 1: attr = 0x62b24d "bottomWidget" gdb) n 508 args[count].value = va_arg(var, XtArgVal); 2: args[count].value = -6510615555426900571 1: attr = 0x62b24d "bottomWidget" (gdb) n 509 ++count; 2: args[count].value = 34359738368 1: attr = 0x62b24d "bottomWidget" What is 34359738368? It's 0x800000000 - so the lower 32-bits are zero put there by the "movl" command - the same case as for the "x" parameter above. In this case it is a problem, because this value is supposed to be a pointer to the Widget, (NULL is allowed). So a full 64-bit value will be passed to the code. It crashes finally at Form.c:1955, in ConstraintInitialize() (this is part of libXm): Form.c: 87 #define SIBLINGS(w,s) (((w != NULL) && (s != NULL)) &&\ 88 (XtParent(w) == XtParent(s))) 1952 if ((nc->att[i].type == XmATTACH_WIDGET) || 1953 (nc->att[i].type == XmATTACH_OPPOSITE_WIDGET)) { 1954 1955 while ((nc->att[i].w) && !SIBLINGS(nc->att[i].w, new_w)) { 1956 nc->att[i].w = XtParent(nc->att[i].w); 1957 } 1958 } (gdb) print nc->att[3] $2 = {type = 3 '\003', w = 0x800000000, percent = 0, offset = 10, value = 0, tempValue = 0} Although this code and SIBLINGS macro are very careful about not trying to dereference NULL pointer, the address 0x800000000 cannot be recognized as invalid. So it tries to derefence this and we have segmentation fault. Since there is nothing in va_arg() or Xt specification that would tell us that "int value will be automically promoted to long", I believe that compiler, libXt and libXm are correct in this regard. It is caller responsibility to set the argument correctly as the null pointer to Widget (None from X.h can be used). This is similar to attempts to use 0 for NULL pointers as seen somewhere else in the code. And I must admint dtcreate is poorly written, mosly in the pre-ANSI C. This is bad news for us: apart from checking parameters of all invocations of XtGetValues() and XtVaGetValues() we need to check all invocations of other XtVaXXX functions. All variadic values where pointers, Widgets, etc. are expected *must* be called with real 64-bit values without any attempt to rely on non-existing conversion. Probably it would be good to have a quick reference of all Motif attibutes and their types to quickly check only those that can cause problems. //Marcin ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://ad.doubleclick.net/clk;258768047;13503038;j? http://info.appdynamics.com/FreeJavaPerformanceDownload.html _______________________________________________ cdesktopenv-devel mailing list cdesktopenv-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/cdesktopenv-devel