GNU bug report logs - #9983
valgrind warning in draw_glyphs

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9983 in the body.
You can then email your comments to 9983 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dan Nicolaescu <dann <at> gnu.org>
To: bug-gnu-emacs <at> gnu.org
Subject: valgrind warning in draw_glyphs
Date: Sun, 06 Nov 2011 23:36:42 -0500

trunk with lucid toolkit
valgrind ./temacs -Q
gives this warning (not sure exactly when, just playing with the menus
and tooltips, the only "editing" action was to run a grep)





==2341== Conditional jump or move depends on uninitialised value(s)
==2341==    at 0x44CDDA: draw_glyphs (xdisp.c:22981)
==2341==    by 0x44D9FC: expose_area (xdisp.c:27401)
==2341==    by 0x44DDA8: expose_line (xdisp.c:27426)
==2341==    by 0x45B6AA: expose_window (xdisp.c:27653)
==2341==    by 0x45C0DA: expose_window_tree (xdisp.c:27725)
==2341==    by 0x45C0B0: expose_window_tree (xdisp.c:27723)
==2341==    by 0x45C16E: expose_frame (xdisp.c:27780)
==2341==    by 0x4C188D: handle_one_xevent (xterm.c:6219)
==2341==    by 0x4C2F56: XTread_socket (xterm.c:7148)
==2341==    by 0x4F2F26: read_avail_input (keyboard.c:6821)
==2341==    by 0x4F3049: handle_async_input (keyboard.c:7149)
==2341==    by 0x4F26A4: process_pending_signals (keyboard.c:7165)
==2341==  Uninitialised value was created by a stack allocation
==2341==    at 0x44AAD3: draw_glyphs (xdisp.c:22835)
==2341== 


The warning is for this:
        if (check_mouse_face
              && mouse_beg_col < start && mouse_end_col > i)

it looks like mouse_beg_col and mouse_end_col could be left uninitialized a few lines above.

Message #8 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dan Nicolaescu <dann <at> gnu.org>
Cc: 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 00:49:07 -0500

> From: Dan Nicolaescu <dann <at> gnu.org>
> Date: Sun, 06 Nov 2011 23:36:42 -0500
> 
> The warning is for this:
>         if (check_mouse_face
>               && mouse_beg_col < start && mouse_end_col > i)
> 
> it looks like mouse_beg_col and mouse_end_col could be left uninitialized a few lines above.

I don't see how.  These variables are initialized in this block:

	  if (row >= mouse_beg_row && row <= mouse_end_row)
	    {
	      check_mouse_face = 1;
	      mouse_beg_col = (row == mouse_beg_row)
		? hlinfo->mouse_face_beg_col : 0;
	      mouse_end_col = (row == mouse_end_row)
		? hlinfo->mouse_face_end_col
		: row->used[TEXT_AREA];
	    }

check_mouse_face starts as zero, and is only set to 1 in this block.
So any test that is conditioned on check_mouse_face being non-zero is
okay with looking at mouse_beg_col and mouse_end_col.

The other variables in the line being flagged, `start' and `i', are
also okay: `start' is one of the call arguments and `i' is computed
right before the line being flagged.

Did I miss something?

Message #11 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Dan Nicolaescu <dann <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 06:52:03 -0500

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Dan Nicolaescu <dann <at> gnu.org>
>> Date: Sun, 06 Nov 2011 23:36:42 -0500
>> 
>> The warning is for this:
>>         if (check_mouse_face
>>               && mouse_beg_col < start && mouse_end_col > i)
>> 
>> it looks like mouse_beg_col and mouse_end_col could be left uninitialized a few lines above.
>
> I don't see how.  These variables are initialized in this block:
>
> 	  if (row >= mouse_beg_row && row <= mouse_end_row)
> 	    {
> 	      check_mouse_face = 1;
> 	      mouse_beg_col = (row == mouse_beg_row)
> 		? hlinfo->mouse_face_beg_col : 0;
> 	      mouse_end_col = (row == mouse_end_row)
> 		? hlinfo->mouse_face_end_col
> 		: row->used[TEXT_AREA];
> 	    }
>
> check_mouse_face starts as zero, and is only set to 1 in this block.
> So any test that is conditioned on check_mouse_face being non-zero is
> okay with looking at mouse_beg_col and mouse_end_col.
>
> The other variables in the line being flagged, `start' and `i', are
> also okay: `start' is one of the call arguments and `i' is computed
> right before the line being flagged.
>
> Did I miss something?

Hmm, you might be right.  Telling valgrind to attach gdb at that point:

(gdb) info local
overlap_hl = <optimized out>
hlinfo = <optimized out>
mouse_beg_col = 0x0
check_mouse_face = 0x0
dummy_x = 0x0
h = <optimized out>
t = <optimized out>
mouse_end_col = 0x50b3a22
head = 0x7feffe3a0
tail = 0x7feffe3a0
s = <optimized out>
clip_head = 0x0
clip_tail = 0x0
i = <optimized out>
j = <optimized out>
x_reached = 0x184
last_x = 0x2ec
area_left = 0x1c
f = 0x5157bf0

Maybe a compiler problem, evaluating the && in the wrong order?

Message #14 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Dan Nicolaescu <dann <at> gnu.org>, 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 13:21:38 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

> The other variables in the line being flagged, `start' and `i', are
> also okay: `start' is one of the call arguments

Just being an argument does not necessarily make it ok.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #17 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Dan Nicolaescu <dann <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 07:44:49 -0500

Dan Nicolaescu <dann <at> gnu.org> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Dan Nicolaescu <dann <at> gnu.org>
>>> Date: Sun, 06 Nov 2011 23:36:42 -0500
>>> 
>>> The warning is for this:
>>>         if (check_mouse_face
>>>               && mouse_beg_col < start && mouse_end_col > i)
>>> 
>>> it looks like mouse_beg_col and mouse_end_col could be left uninitialized a few lines above.
>>
>> I don't see how.  These variables are initialized in this block:
>>
>> 	  if (row >= mouse_beg_row && row <= mouse_end_row)
>> 	    {
>> 	      check_mouse_face = 1;
>> 	      mouse_beg_col = (row == mouse_beg_row)
>> 		? hlinfo->mouse_face_beg_col : 0;
>> 	      mouse_end_col = (row == mouse_end_row)
>> 		? hlinfo->mouse_face_end_col
>> 		: row->used[TEXT_AREA];
>> 	    }
>>
>> check_mouse_face starts as zero, and is only set to 1 in this block.
>> So any test that is conditioned on check_mouse_face being non-zero is
>> okay with looking at mouse_beg_col and mouse_end_col.
>>
>> The other variables in the line being flagged, `start' and `i', are
>> also okay: `start' is one of the call arguments and `i' is computed
>> right before the line being flagged.
>>
>> Did I miss something?
>
> Hmm, you might be right.  Telling valgrind to attach gdb at that point:
>
> (gdb) info local
> overlap_hl = <optimized out>
> hlinfo = <optimized out>
> mouse_beg_col = 0x0
> check_mouse_face = 0x0
> dummy_x = 0x0
> h = <optimized out>
> t = <optimized out>
> mouse_end_col = 0x50b3a22
> head = 0x7feffe3a0
> tail = 0x7feffe3a0
> s = <optimized out>
> clip_head = 0x0
> clip_tail = 0x0
> i = <optimized out>
> j = <optimized out>
> x_reached = 0x184
> last_x = 0x2ec
> area_left = 0x1c
> f = 0x5157bf0
>
> Maybe a compiler problem, evaluating the && in the wrong order?

The assembly in question looks like this:

.LBB3664:
        .loc 1 22981 0
        movl -152(%rbp), %ebx
        cmpl %ebx, -260(%rbp)
        jge  .L7265
        movl -192(%rbp), %eax
        testl            %eax, %eax
        je               .L7265
        .loc 1 22982 0
        movslq -232(%rbp), %rax
        .loc 1 22985 0
        xorl %r14d, %r14d
        cmpq %rax, -120(%rbp)
        setl %r14b
        leal (%r14,%r14,2), %ebx
        movl %ebx, -144(%rbp)

the debugger is stopped on the "jge" line, that means that the "cmpl"
lined did the access.

(gdb) p/d (long long)&mouse_beg_col - (long long)$rbp
$18 = -260
(gdb) p/d (long long)&check_mouse_face - (long long)$rbp
$19 = -192

so it looks like check_mouse_face is tested after mouse_beg_col.
(unless I'm missing something...)

The compiler here is the system compiler on an up to date Fedora15 system.
 gcc --version
gcc (GCC) 4.6.1 20110908 (Red Hat 4.6.1-9)
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Message #20 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Dan Nicolaescu <dann <at> gnu.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 13:59:37 +0100

Dan Nicolaescu <dann <at> gnu.org> writes:

> so it looks like check_mouse_face is tested after mouse_beg_col.

That is perfectly ok under the as-if rule.  See also
<http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50975>.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #25 received at 9983-close <at> debbugs.gnu.org (full text, mbox):

From: Dan Nicolaescu <dann <at> gnu.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 9983-close <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 10:21:18 -0500

Andreas Schwab <schwab <at> linux-m68k.org> writes:

> Dan Nicolaescu <dann <at> gnu.org> writes:
>
>> so it looks like check_mouse_face is tested after mouse_beg_col.
>
> That is perfectly ok under the as-if rule.  See also

Right.  Closing this bug then.

Message #28 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: dann <at> gnu.org, 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 17:54:56 +0200

> From: Andreas Schwab <schwab <at> linux-m68k.org>
> Cc: Eli Zaretskii <eliz <at> gnu.org>,  9983 <at> debbugs.gnu.org
> Date: Mon, 07 Nov 2011 13:59:37 +0100
> 
> Dan Nicolaescu <dann <at> gnu.org> writes:
> 
> > so it looks like check_mouse_face is tested after mouse_beg_col.
> 
> That is perfectly ok under the as-if rule.  See also
> <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50975>.

Where can I find the definition of the "side effects" that would
disallow such optimizations?

Message #31 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: dann <at> gnu.org, 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Mon, 07 Nov 2011 17:57:04 +0200

> From: Andreas Schwab <schwab <at> linux-m68k.org>
> Cc: Dan Nicolaescu <dann <at> gnu.org>,  9983 <at> debbugs.gnu.org
> Date: Mon, 07 Nov 2011 13:21:38 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > The other variables in the line being flagged, `start' and `i', are
> > also okay: `start' is one of the call arguments
> 
> Just being an argument does not necessarily make it ok.

Indeed.  I also thought about that, so I checked the callers two
levels up, and came up empty-handed.

I guess the optimization done by GCC 4.6 fooled valgrind as well.

Message #34 received at 9983 <at> debbugs.gnu.org (full text, mbox):

From: Dan Nicolaescu <dann <at> gnu.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Andreas Schwab <schwab <at> linux-m68k.org>, 9983 <at> debbugs.gnu.org
Subject: Re: bug#9983: valgrind warning in draw_glyphs
Date: Tue, 08 Nov 2011 09:17:08 -0500

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Andreas Schwab <schwab <at> linux-m68k.org>
>> Cc: Dan Nicolaescu <dann <at> gnu.org>,  9983 <at> debbugs.gnu.org
>> Date: Mon, 07 Nov 2011 13:21:38 +0100
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > The other variables in the line being flagged, `start' and `i', are
>> > also okay: `start' is one of the call arguments
>> 
>> Just being an argument does not necessarily make it ok.
>
> Indeed.  I also thought about that, so I checked the callers two
> levels up, and came up empty-handed.
>
> I guess the optimization done by GCC 4.6 fooled valgrind as well.

Not trying to split hairs, but valgrind wasn't fooled, it did the right
thing at the level that valgrind is working at...

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.