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
[email protected]
https://lists.sourceforge.net/lists/listinfo/cdesktopenv-devel
