Tk Source Code

f$ valgrind --tool=memcheck --leak-check=full /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6
==21950== Memcheck, a memory error detector
==21950== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==21950== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==21950== Command: /home/francois/Documents/tcltk/fossil/tcltk/bin/wish8.6
==21950== 
==21950== Invalid read of size 1
==21950==    at 0x4C2D1D3: strcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21950==    by 0x5CEEC23: _XimUnRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x5CD5652: XUnregisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x4F77B13: InstantiateIMCallback (tkUnixEvent.c:681)
==21950==    by 0x5CEEB04: _XimRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x5CD55EB: XRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x4F77241: TkpOpenDisplay (tkUnixEvent.c:184)
==21950==    by 0x4EABC89: GetScreen (tkWindow.c:465)
==21950==    by 0x4EABA4A: CreateTopLevelWindow (tkWindow.c:348)
==21950==    by 0x4EAC6F1: TkCreateMainWindow (tkWindow.c:854)
==21950==    by 0x4EBB357: CreateFrame (tkFrame.c:582)
==21950==    by 0x4EBAE8F: TkListCreateFrame (tkFrame.c:468)
==21950==  Address 0x81b4e40 is 0 bytes inside a block of size 1 free'd
==21950==    at 0x4C29E90: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21950==    by 0x5CE4B5F: XSetLocaleModifiers (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x4F77BFD: OpenIM (tkUnixEvent.c:731)
==21950==    by 0x4F77AE7: InstantiateIMCallback (tkUnixEvent.c:680)
==21950==    by 0x5CEEB04: _XimRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x5CD55EB: XRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.3.0)
==21950==    by 0x4F77241: TkpOpenDisplay (tkUnixEvent.c:184)
==21950==    by 0x4EABC89: GetScreen (tkWindow.c:465)
==21950==    by 0x4EABA4A: CreateTopLevelWindow (tkWindow.c:348)
==21950==    by 0x4EAC6F1: TkCreateMainWindow (tkWindow.c:854)
==21950==    by 0x4EBB357: CreateFrame (tkFrame.c:582)
==21950==    by 0x4EBAE8F: TkListCreateFrame (tkFrame.c:468)

I more than strongly suspect the fix for [7d967c68a0], i.e. [d4e818fadc], that was intended to fix Tk applications segmentation fault when ibus-daemon IME is restarted.

After a quick look at this patch I find a bit strange that InstantiateIMCallback()calls XUnregisterIMInstantiateCallback() while DestroyIMCallback()calls XRegisterIMInstantiateCallback(), but that is probably my misunderstanding.

In any case, something seems wrong in that patch.

Thank you both, John and Christopher! I have merged the bugfix branch into core-8-6-branch and trunk. Closing now.

Thank you both for your comments and feedback.

@John, I have now followed your recommendation regarding ignoring the return value of XSetLocaleModifiers(""). This is [f2c30a23].

@Christopher, thanks for the explanation about what versions of the libX11 contain a given commit.

I'm currently using libX11 1.7.2. Also, since you didn't encounter the leak on exit with libX11 1.8.6, it is likely that 1d11822601, which is included in 1.8.2 through the current 1.8.6 indeed fixed it. I think we can therefore close the present ticket.

The next issue triggers upon entering exit in wish:

==165932== 408 bytes in 1 blocks are definitely lost in loss record 101 of 226
…

I did not encounter this leak with libx11 1.8.6.

Now what is not clear to me is in which libX11 version this fix got distributed first? Did this fix made its way into the libX11 used?

The page for a commit on GitHub or GitLab lists the tags which descend from it (and so will exclude e.g. tags which the commit was only cherry-picked to). gitlab.freedesktop.org is temporarily down for maintenance, so I will refer to an unofficial GitHub mirror. a3c0b5d was part of libx11 1.7.0. 1d11822 was part of libx11 1.8.2.

When typing exit in wish, TkpCloseDisplay() gets correctly called, which calls XCloseIM(). Isn't XCloseIM() supposed to take care of cleaning the possibly instantiated callback as part of the input method closure?

In any case, adding a call to XUnregisterIMInstantiateCallback() just before XCloseIM() does not help in removing this second memory leak.

Doesn't this look like a leak in libX11?

There is a similar bug, see https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/36 and the fix https://gitlab.freedesktop.org/xorg/lib/libx11/-/commit/a3c0b5dbd6b12ae64bc78b11795647a7f6df0c7a consisting in adding calls to XFree() in _XimRegisterIMInstantiateCallback() and _XimUnRegisterIMInstantiateCallback().

Now what is not clear to me is in which libX11 version this fix got distributed first? Did this fix made its way into the libX11 used?

Startup of wish is now clean from any valgrind output.

The next issue triggers upon entering exit in wish:

==165932== 408 bytes in 1 blocks are definitely lost in loss record 101 of 226
==165932==    at 0x483AB65: calloc (vg_replace_malloc.c:760)
==165932==    by 0x510ECDB: _XimOpenIM (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0)
==165932==    by 0x510E914: _XimRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0)
==165932==    by 0x50F5D38: XRegisterIMInstantiateCallback (in /usr/lib/x86_64-linux-gnu/libX11.so.6.4.0)
==165932==    by 0x499C234: TkpOpenDisplay (tkUnixEvent.c:189)
==165932==    by 0x48C7EDB: GetScreen (tkWindow.c:467)
==165932==    by 0x48C7C6C: CreateTopLevelWindow (tkWindow.c:350)
==165932==    by 0x48C8B0F: TkCreateMainWindow (tkWindow.c:863)
==165932==    by 0x48D88A4: CreateFrame (tkFrame.c:582)
==165932==    by 0x48D83DC: TkListCreateFrame (tkFrame.c:468)
==165932==    by 0x48CC86B: Initialize (tkWindow.c:3359)
==165932==    by 0x48CBB0F: Tk_Init (tkWindow.c:3016)

(Sorry, my message below should read: What should we do if XSetLocaleModifiers() returns NULL?)

From the manual page for XSetLocaleModifiers(""), a NULL return happens when "invalid values are given for one or more modifier categories supported by the locale".

The actual call is XSetLocaleModifiers(""), which I don't see why it could return NULL so we could probably just remove the if statement surrounding this call entirely.

However, in the unlikely case NULL would be returned I believe this case should not be ignored and TkpOpenDisplay() should return NULL (as when XkbOpenDisplay() returns NULL, Tk would not be operative).

So my proposed fix is [cff2f2aa]. The invalid read does no longer trigger from valgrind.

Hmmm... What should we do if XSetLocaleModifiers() returns FALSE...? Will investigate a bit.

The moved code is no longer in a function returning void:

warning: ‘return’ with no value, in function returning non-void [-Wreturn-type]

I am not sure whether returning early is still correct. If it is, then return NULL should be used instead.

Proposed patch committed in [bf200aba3e] for testing.

Many thanks for your input! With this information we can certainly take care of this (within the present ticket).

Reported once more here in the Tcl tracker: https://core.tcl-lang.org/tcl/tktview/e649d9f567d765ecb0afe2446f350479d9360b1d

Since the fix is in libX11 I'm closing the ticket.

Fixed in libX11 on June 3 2021.

It seems that this was reported again, see [30672bcb71].

The bug report at freedesktop just moved here.

Bug report at freedesktop received an interesting comment today. I don't think there is any hope to see this fixed.

Thanks for the explanation, this sounds correct indeed.

And thanks for the link as well. So this is a libX11 bug. I have added a comment in the freedesktop bugzilla ticket for cross reference.

Setting status of the present ticket to 'Pending' until this gets resolved in libX11.

There's nothing unusual about the code.

I followed the logic of another implementation.  The logic you quote had me confused at first, but I worked through it and it is correct also.

- Once a XIM instance is instantiated, you don't need to watch for one to be registered.
- If you destroy a XIM instance, you need to register a callback in case a new one shows up again.

https://bugs.freedesktop.org/show_bug.cgi?id=81338
( https://github.com/mirror/libX11/blob/master/modules/im/ximcp/imInsClbk.c )

Looks like some people have run into this before, but the suggested patch has not been applied.

I think that these complaints are not a problem, valgrind cannot resolve special memory hacks, and some libraries, not only X11, are using special memory hacks. In a big application you will see some more complaints of valgrind related to external libraries.