* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window @ 2011-11-03 21:55 Dan Nicolaescu 2011-11-03 22:17 ` Eli Zaretskii 2011-11-07 5:04 ` Paul Eggert 0 siblings, 2 replies; 10+ messages in thread From: Dan Nicolaescu @ 2011-11-03 21:55 UTC (permalink / raw) To: 9948 valgrind gives this warning on emacs -Q, lucid toolkit, compiled -g -O0 ==7776== Conditional jump or move depends on uninitialised value(s) ==7776== at 0x44F58B: redisplay_window (xdisp.c:15575) ==7776== by 0x4496BF: redisplay_window_0 (xdisp.c:13573) ==7776== by 0x5D3DBF: internal_condition_case_1 (eval.c:1537) ==7776== by 0x449632: redisplay_windows (xdisp.c:13553) ==7776== by 0x44864F: redisplay_internal (xdisp.c:13130) ==7776== by 0x4463AF: redisplay (xdisp.c:12353) ==7776== by 0x53A8C6: read_char (keyboard.c:2443) ==7776== by 0x54895A: read_key_sequence (keyboard.c:9290) ==7776== by 0x5387ED: command_loop_1 (keyboard.c:1447) ==7776== by 0x5D3C46: internal_condition_case (eval.c:1499) ==7776== by 0x5380F7: command_loop_2 (keyboard.c:1158) ==7776== by 0x5D35D0: internal_catch (eval.c:1256) ==7776== The line in question is aggressive = scrolling_up ? BVAR (current_buffer, scroll_up_aggressively) : BVAR (current_buffer, scroll_down_aggressively); ^^^^^^^^^^^^^^^^^^ This one ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-03 21:55 bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window Dan Nicolaescu @ 2011-11-03 22:17 ` Eli Zaretskii 2011-11-03 23:30 ` Dan Nicolaescu 2011-11-07 5:04 ` Paul Eggert 1 sibling, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2011-11-03 22:17 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 9948 > From: Dan Nicolaescu <dann@gnu.org> > Date: Thu, 03 Nov 2011 17:55:03 -0400 > > aggressive = > scrolling_up > ? BVAR (current_buffer, scroll_up_aggressively) > : BVAR (current_buffer, scroll_down_aggressively); > ^^^^^^^^^^^^^^^^^^ > This one How can this be uninitialized? This is a buffer-local value of a variable that is initialized to nil on buffer.c. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-03 22:17 ` Eli Zaretskii @ 2011-11-03 23:30 ` Dan Nicolaescu 2011-11-04 9:04 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Dan Nicolaescu @ 2011-11-03 23:30 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 9948 Eli Zaretskii <eliz@gnu.org> writes: >> From: Dan Nicolaescu <dann@gnu.org> >> Date: Thu, 03 Nov 2011 17:55:03 -0400 >> >> aggressive = >> scrolling_up >> ? BVAR (current_buffer, scroll_up_aggressively) >> : BVAR (current_buffer, scroll_down_aggressively); >> ^^^^^^^^^^^^^^^^^^ >> This one > > How can this be uninitialized? This is a buffer-local value of a > variable that is initialized to nil on buffer.c. The line might be incorrect, are all the variables in that expression initialized correctly? ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-03 23:30 ` Dan Nicolaescu @ 2011-11-04 9:04 ` Eli Zaretskii 2011-11-04 13:05 ` Dan Nicolaescu 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2011-11-04 9:04 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 9948 > From: Dan Nicolaescu <dann@gnu.org> > Cc: 9948@debbugs.gnu.org > Date: Thu, 03 Nov 2011 19:30:55 -0400 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> From: Dan Nicolaescu <dann@gnu.org> > >> Date: Thu, 03 Nov 2011 17:55:03 -0400 > >> > >> aggressive = > >> scrolling_up > >> ? BVAR (current_buffer, scroll_up_aggressively) > >> : BVAR (current_buffer, scroll_down_aggressively); > >> ^^^^^^^^^^^^^^^^^^ > >> This one > > > > How can this be uninitialized? This is a buffer-local value of a > > variable that is initialized to nil on buffer.c. > > The line might be incorrect, are all the variables in that expression > initialized correctly? I think so, yes. The only other variables are: . current_buffer -- a global variable that always has some value . scrolling_up -- computed in the line above this snippet. It depends on margin_pos, which is initialized at the beginning of the parent block. The BVAR macro doesn't reference any variables except its first argument. So I'm really puzzled about this one. Is there any way to ask valgrind for a more detailed report? ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-04 9:04 ` Eli Zaretskii @ 2011-11-04 13:05 ` Dan Nicolaescu 2016-07-11 3:49 ` npostavs 0 siblings, 1 reply; 10+ messages in thread From: Dan Nicolaescu @ 2011-11-04 13:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 9948 Eli Zaretskii <eliz@gnu.org> writes: >> From: Dan Nicolaescu <dann@gnu.org> >> Cc: 9948@debbugs.gnu.org >> Date: Thu, 03 Nov 2011 19:30:55 -0400 >> >> Eli Zaretskii <eliz@gnu.org> writes: >> >> >> From: Dan Nicolaescu <dann@gnu.org> >> >> Date: Thu, 03 Nov 2011 17:55:03 -0400 >> >> >> >> aggressive = >> >> scrolling_up >> >> ? BVAR (current_buffer, scroll_up_aggressively) >> >> : BVAR (current_buffer, scroll_down_aggressively); >> >> ^^^^^^^^^^^^^^^^^^ >> >> This one >> > >> > How can this be uninitialized? This is a buffer-local value of a >> > variable that is initialized to nil on buffer.c. >> >> The line might be incorrect, are all the variables in that expression >> initialized correctly? > > I think so, yes. The only other variables are: > > . current_buffer -- a global variable that always has some value > > . scrolling_up -- computed in the line above this snippet. It > depends on margin_pos, which is initialized at the beginning of > the parent block. > > The BVAR macro doesn't reference any variables except its first > argument. > > So I'm really puzzled about this one. Is there any way to ask > valgrind for a more detailed report? It looks like there's a --track-origins=yes that might help. Now just need to reproduce the same issue (which occurred after quite a bit of random editing/playing around, so it might not be easy...). ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-04 13:05 ` Dan Nicolaescu @ 2016-07-11 3:49 ` npostavs 0 siblings, 0 replies; 10+ messages in thread From: npostavs @ 2016-07-11 3:49 UTC (permalink / raw) To: Dan Nicolaescu; +Cc: 9948 tags 9948 unreproducible close 9948 quit Dan Nicolaescu <dann@gnu.org> writes: > It looks like there's a --track-origins=yes that might help. > Now just need to reproduce the same issue (which occurred after quite a > bit of random editing/playing around, so it might not be easy...). I guess it was never reproduced. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-03 21:55 bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window Dan Nicolaescu 2011-11-03 22:17 ` Eli Zaretskii @ 2011-11-07 5:04 ` Paul Eggert 2011-11-07 6:00 ` Eli Zaretskii 1 sibling, 1 reply; 10+ messages in thread From: Paul Eggert @ 2011-11-07 5:04 UTC (permalink / raw) To: 9948 --track-origins=yes should help, but in the meantime, valgrind's bug report doesn't necessarily mean that no code ever set scrolling_up. It could be that scrolling_up was set this way: scrolling_up = PT > margin_pos; but that margin_pos wasn't properly initialized. For example, suppose margin_pos was set this way: margin_pos = IT_CHARPOS (it1); This initialization would not be correct if IT_CHARPOS (it1) referenced an uninitialized variable. Unfortunately valgrind won't report any error in IT_CHARPOS (it1) until scrolling_up is used. That's because it does not report use of uninitialized storage: it reports only when conditional branches or syscalls or addresses depend on the contents of the uninitialized storage. It's too bad that valgrind works this way. It'd be more convenient if it reported use of uninitialized storage right away. But it'd be hard for valgrind to do better, without reporting lots of false positives for structures that contain holes, so it is what it is. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-07 5:04 ` Paul Eggert @ 2011-11-07 6:00 ` Eli Zaretskii 2011-11-07 6:08 ` Paul Eggert 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2011-11-07 6:00 UTC (permalink / raw) To: Paul Eggert; +Cc: 9948 > Date: Sun, 06 Nov 2011 21:04:40 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > > --track-origins=yes should help, but in the meantime, valgrind's > bug report doesn't necessarily mean that no code ever set scrolling_up. > > It could be that scrolling_up was set this way: > > scrolling_up = PT > margin_pos; > > but that margin_pos wasn't properly initialized. For example, suppose > margin_pos was set this way: > > margin_pos = IT_CHARPOS (it1); > > This initialization would not be correct if IT_CHARPOS (it1) referenced > an uninitialized variable. IT_CHARPOS is defined as follows: #define CHARPOS(POS) (POS).charpos #define IT_CHARPOS(IT) CHARPOS ((IT).current.pos) And margin_pos is computed as follows: EMACS_INT margin_pos = CHARPOS (startp); <<<<<<<<<<<<<< int scrolling_up; Lisp_Object aggressive; /* If there is a scroll margin at the top of the window, find its character position. */ if (margin /* Cannot call start_display if startp is not in the accessible region of the buffer. This can happen when we have just switched to a different buffer and/or changed its restriction. In that case, startp is initialized to the character position 1 (BEG) because we did not yet have chance to display the buffer even once. */ && BEGV <= CHARPOS (startp) && CHARPOS (startp) <= ZV) { struct it it1; void *it1data = NULL; SAVE_IT (it1, it, it1data); start_display (&it1, w, startp); move_it_vertically (&it1, margin); margin_pos = IT_CHARPOS (it1); <<<<<<<<<<<<<<<<<< RESTORE_IT (&it, &it, it1data); } scrolling_up = PT > margin_pos; aggressive = scrolling_up ? BVAR (current_buffer, scroll_up_aggressively) : BVAR (current_buffer, scroll_down_aggressively); Both `startp' and `it1' have a valid CHARPOS, the former by virtue of this (near the beginning of the function): SET_TEXT_POS_FROM_MARKER (startp, w->start); and the latter by virtue of the start_display call above, which initializes `it1's character position to `startp'. Again, I don't see how any of this could involve an uninitialized variable. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-07 6:00 ` Eli Zaretskii @ 2011-11-07 6:08 ` Paul Eggert 2011-11-07 6:30 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Paul Eggert @ 2011-11-07 6:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 9948 On 11/06/11 22:00, Eli Zaretskii wrote: > Again, I don't see how any of this could involve an uninitialized > variable. I don't either, but the analysis you gave is not complete. For example: SET_TEXT_POS_FROM_MARKER (startp, w->start); does not set startp to a valid charpos if w->start points to uninitialized data. A problem with valgrind is that this sort of analysis can be very painful to do, if one wants to do it completely. I found this out the hard way when trying to apply valgrind to Emacs's garbage collector.... ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window 2011-11-07 6:08 ` Paul Eggert @ 2011-11-07 6:30 ` Eli Zaretskii 0 siblings, 0 replies; 10+ messages in thread From: Eli Zaretskii @ 2011-11-07 6:30 UTC (permalink / raw) To: Paul Eggert; +Cc: 9948 > Date: Sun, 06 Nov 2011 22:08:54 -0800 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 9948@debbugs.gnu.org > > SET_TEXT_POS_FROM_MARKER (startp, w->start); > > does not set startp to a valid charpos if w->start > points to uninitialized data. w->startp is the window-start of the window represented by `w'. It is a very important player in displaying the window. If that is uninitialized, we have a much larger catastrophe on our hands than some obscure local variable that _could_ be uninitialized ;-) IOW, if w->startp does not have a valid value, Emacs will crash pronto. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-07-11 3:49 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-03 21:55 bug#9948: valgrind warning: Conditional jump or move depends on uninitialised value(s) in redisplay_window Dan Nicolaescu 2011-11-03 22:17 ` Eli Zaretskii 2011-11-03 23:30 ` Dan Nicolaescu 2011-11-04 9:04 ` Eli Zaretskii 2011-11-04 13:05 ` Dan Nicolaescu 2016-07-11 3:49 ` npostavs 2011-11-07 5:04 ` Paul Eggert 2011-11-07 6:00 ` Eli Zaretskii 2011-11-07 6:08 ` Paul Eggert 2011-11-07 6:30 ` Eli Zaretskii
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.