Valgrind reports bogus memory leak

[Bram Moolenaar]

Valgrind has been reporting a leak when running test_popupwin.
I narrowed it down to Test_simple_popup(), somewhere halfway.

The valgrind report is:

==22433== 8 bytes in 1 blocks are definitely lost in loss record 17 of 371
==22433== at 0x483874F: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==22433== by 0x280E74: lalloc (misc2.c:924)
==22433== by 0x280D1F: alloc (misc2.c:827)
==22433== by 0x281395: vim_strsave (misc2.c:1276)
==22433== by 0x2BAA4A: buf_copy_options (option.c:5848)
==22433== by 0x1872E4: buflist_new (buffer.c:2116)
==22433== by 0x2D3AA9: popup_create (popupwin.c:1706)
==22433== by 0x2D430A: f_popup_create (popupwin.c:1923)

The buf_copy_options() line is:

buf->b_p_nf = vim_strsave(p_nf);

This is very strange, since several other buffer-local options are
allocated, and they are not reported.

I did quite a bit of digging and finally found that this line in
defaults.vim triggers it:

set nrformats-=octal

I tried setting other options there, and got confusing results, until I
figured the leak is only reported if the string is exactly 8 bytes! That
is 7 text characters plus NUL. If I change nrformats to only "hex" or
make it "hex,hex,hex" then the leak is not reported.

I assume this must be a bug in valgrind. Can't find anything about it
though.

--
WOMAN: Dennis, there's some lovely filth down here. Oh -- how d'you do?
ARTHUR: How do you do, good lady. I am Arthur, King of the Britons.
Who's castle is that?
WOMAN: King of the who?
The Quest for the Holy Grail (Monty Python)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Christian Brabandt]

What version of valgrind are you running? I wonder if we should report
this at bugs.kde.org. Is there an easy way to reproduce this?

Best,
Christian
--
Sorgen sind wie Babies: Je mehr man sie hätschelt, desto besser
gedeihen sie.
-- Helmut Qualtinger

[Dominique Pellé]

Does it mean that you can reproduce the leak with something like:
$ valgrind --leak-check=full vim --clean -c 'set nrformats-=octal' -c q

If not, what is the minimalistic way of reproducing the leak?

The above command does not leak for me.

It's not impossible to find a bug with valgrind, but I think it's more likely
to be a real bug with vim.

Regards
Dominique

[Bram Moolenaar]

valgrind-3.14.0

"make test_popupwin" with valgrind enabled in src/testdir/Makefile.
One would have to try out what can be left out without making the
problem disappear.

Dominique wrote:

> It's not impossible to find a bug with valgrind, but I think it's more likely
> to be a real bug with vim.

When I change "set nrformats-=octal" in defaults.vim to something that
is not 8 bytes, then the report goes away.
If I set another buffer-local variable to a value with 8 bytes that
one is also reported. I don't think Vim itself does something special
for an 8 byte string.


--
ARTHUR: I am your king!
WOMAN: Well, I didn't vote for you.
ARTHUR: You don't vote for kings.
WOMAN: Well, 'ow did you become king then?

[Chr. von Stuckrad]

On Sun, 29 Sep 2019, Bram Moolenaar wrote:

> When I change "set nrformats-=octal" in defaults.vim to something that
> is not 8 bytes, then the report goes away.
> If I set another buffer-local variable to a value with 8 bytes that
> one is also reported. I don't think Vim itself does something special
> for an 8 byte string.

Strange ... BUT, could you try to run the test with binaries
compiled with different 'compiler optimisation levels'. We once
had an error, which went away, if we lowered/raised the level
form our default=2 only on 2 it happened, and then we noticed
it happened only on a specific kind of CPU. Something in the
combination of possible registers on the CPU and the way the
parameters of routines were handled by optimizing, lost one
word of memory and potentially crashed the program. Crashes
happened on a small subset of our desktop PCs.

Stucki

--
Christoph von Stuckrad * * | also XMPP = |Mail <stu...@mi.fu-berlin.de> \
Freie Universitaet Berlin |/_*| 'jabber' via|Tel(Di.,Fr.):+49 30 77 39 6600|
IT Mathematik & Informatik|\ *|stucki@jabber| (Mo,Mi,Do):+49 30 838-75 459|
Takustr. 9 / 14195 Berlin * * |.fu-berlin.de|Fax(@home): +49 30 77 39 6601/

[Bram Moolenaar]

Stucki wrote:

> > When I change "set nrformats-=octal" in defaults.vim to something that
> > is not 8 bytes, then the report goes away.
> > If I set another buffer-local variable to a value with 8 bytes that
> > one is also reported. I don't think Vim itself does something special
> > for an 8 byte string.
>
> Strange ... BUT, could you try to run the test with binaries
> compiled with different 'compiler optimisation levels'. We once
> had an error, which went away, if we lowered/raised the level
> form our default=2 only on 2 it happened, and then we noticed
> it happened only on a specific kind of CPU. Something in the
> combination of possible registers on the CPU and the way the
> parameters of routines were handled by optimizing, lost one
> word of memory and potentially crashed the program. Crashes
> happened on a small subset of our desktop PCs.

I tried with -g (no optimization) -O1, -O2 and got the same results.

My CPU is a Ryzen 2700X

--
"Hegel was right when he said that we learn from history that man can
never learn anything from history." (George Bernard Shaw)

[Christian Brabandt]

On Sa, 28 Sep 2019, Bram Moolenaar wrote:

> Valgrind has been reporting a leak when running test_popupwin.
> I narrowed it down to Test_simple_popup(), somewhere halfway.

Hm, #5003 sounds like it could fix this?

Best,
Christian
--
Wer Erfolg haben will, muß die zwischenmenschlichen Beziehungen
verbessern.
-- Böckmann, Walter, Dr. Böckmann, Walter, Dr.

[Bram Moolenaar]

Christian wrote:

> On Sa, 28 Sep 2019, Bram Moolenaar wrote:
>
> > Valgrind has been reporting a leak when running test_popupwin.
> > I narrowed it down to Test_simple_popup(), somewhere halfway.
>
> Hm, #5003 sounds like it could fix this?

Strange, I didn't get an email for that pull request.

It does appear to fix the problem. Thus instead of valgrind giving a
false alarm for the 8-byte string, it's the opposite: It only warns for
an 8-byte string and not for others. Weird.

--
Advice to worms: Sleep late.