* C warning in MSYS2 build @ 2017-10-05 17:44 Richard Copley 2017-10-05 18:54 ` Eli Zaretskii 0 siblings, 1 reply; 7+ messages in thread From: Richard Copley @ 2017-10-05 17:44 UTC (permalink / raw) To: Emacs Development; +Cc: Eli Zaretskii Hi, One more warning crept into the MSYS2 build after the recent update to the system headers/libraries: make[1]: Entering directory '/c/projects/emacs/src' CC indent.o indent.c: In function 'scan_for_column': indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] return 0; ^ indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] make[1]: Leaving directory '/c/projects/emacs/src' ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-05 17:44 C warning in MSYS2 build Richard Copley @ 2017-10-05 18:54 ` Eli Zaretskii 2017-10-05 19:32 ` Paul Eggert 0 siblings, 1 reply; 7+ messages in thread From: Eli Zaretskii @ 2017-10-05 18:54 UTC (permalink / raw) To: Richard Copley; +Cc: emacs-devel > From: Richard Copley <rcopley@gmail.com> > Date: Thu, 5 Oct 2017 18:44:43 +0100 > Cc: Eli Zaretskii <eliz@gnu.org> > > One more warning crept into the MSYS2 build after the recent update to > the system headers/libraries: > > make[1]: Entering directory '/c/projects/emacs/src' > CC indent.o > indent.c: In function 'scan_for_column': > indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] > return 0; > ^ > indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] > indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] > indent.c:69:10: warning: potential null pointer dereference [-Wnull-dereference] > make[1]: Leaving directory '/c/projects/emacs/src' Thanks, but it isn't new. You already reported this very warning in http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00478.html and I already said here: http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00427.html that I thought it was a GCC bug. Paul seemed to confirm that in http://lists.gnu.org/archive/html/emacs-devel/2017-09/msg00493.html I have no idea how to fix this, except by removing -Wnull-dereference from the warning flags. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-05 18:54 ` Eli Zaretskii @ 2017-10-05 19:32 ` Paul Eggert 2017-10-05 20:08 ` Richard Copley 2017-10-06 15:16 ` Richard Stallman 0 siblings, 2 replies; 7+ messages in thread From: Paul Eggert @ 2017-10-05 19:32 UTC (permalink / raw) To: emacs-devel; +Cc: Richard Copley, Eli Zaretskii [-- Attachment #1: Type: text/plain, Size: 1243 bytes --] On 10/05/2017 11:54 AM, Eli Zaretskii wrote: > Thanks, but it isn't new. In my experience this kind of warning can appear and vanish almost at whim: compiling at a different optimization levels, or changing compilation in some other way, can cause GCC to omit or include the warning. So possibly Richard wasn't getting the warning a day ago, but got the warning after changing from -O2 to -O0 (or whatever), or by upgrading GCC. If it's a major annoyance we could disable --Wnull-dereference for indent.c. I hope that's not needed, though. Come to think of it, last year I filed a GCC bug report about a similar problem with lib-src/etags.c and -Wnull-dereference, here: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71157 The GCC bug is not fixed yet. etags.c confused GCC by using an assignment inside an 'if' expression, which is contrary to the usual GNU style, and last year I worked around the GCC bug by changing etags.c to use a cleaner style. I notice that indent.c also has an assignment inside an 'if' expression that is relevant to these warnings. Richard, does it help to recode indent.c to use the usual GNU style, as in the attached patch? If so, let's do that instead. (Perhaps we should do that anyway....) [-- Attachment #2: indent.diff --] [-- Type: text/x-patch, Size: 761 bytes --] diff --git a/src/indent.c b/src/indent.c index 26507b5eb5..79841a3d2c 100644 --- a/src/indent.c +++ b/src/indent.c @@ -76,15 +76,17 @@ buffer_display_table (void) static int character_width (int c, struct Lisp_Char_Table *dp) { - Lisp_Object elt; - /* These width computations were determined by examining the cases in display_text_line. */ /* Everything can be handled by the display table, if it's present and the element is right. */ - if (dp && (elt = DISP_CHAR_VECTOR (dp, c), VECTORP (elt))) - return ASIZE (elt); + if (dp) + { + Lisp_Object elt = DISP_CHAR_VECTOR (dp, c); + if (VECTORP (elt)) + return ASIZE (elt); + } /* Some characters are special. */ if (c == '\n' || c == '\t' || c == '\015') ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-05 19:32 ` Paul Eggert @ 2017-10-05 20:08 ` Richard Copley 2017-10-05 22:44 ` Paul Eggert 2017-10-06 15:16 ` Richard Stallman 1 sibling, 1 reply; 7+ messages in thread From: Richard Copley @ 2017-10-05 20:08 UTC (permalink / raw) To: Paul Eggert; +Cc: Eli Zaretskii, Emacs Development On 5 October 2017 at 20:32, Paul Eggert <eggert@cs.ucla.edu> wrote: > On 10/05/2017 11:54 AM, Eli Zaretskii wrote: >> >> Thanks, but it isn't new. > > In my experience this kind of warning can appear and vanish almost at whim: > compiling at a different optimization levels, or changing compilation in > some other way, can cause GCC to omit or include the warning. So possibly > Richard wasn't getting the warning a day ago, but got the warning after > changing from -O2 to -O0 (or whatever), or by upgrading GCC. > > If it's a major annoyance we could disable --Wnull-dereference for indent.c. > I hope that's not needed, though. > > Come to think of it, last year I filed a GCC bug report about a similar > problem with lib-src/etags.c and -Wnull-dereference, here: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71157 > > The GCC bug is not fixed yet. etags.c confused GCC by using an assignment > inside an 'if' expression, which is contrary to the usual GNU style, and > last year I worked around the GCC bug by changing etags.c to use a cleaner > style. I notice that indent.c also has an assignment inside an 'if' > expression that is relevant to these warnings. Richard, does it help to > recode indent.c to use the usual GNU style, as in the attached patch? If so, > let's do that instead. (Perhaps we should do that anyway....) Thanks Eli and Paul. The warnings don't change with that patch. FWIW it seems to me that the results of the buffer_display_table calls in c_string_width and lisp_string_width are dereferenced unchecked, in character.c. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-05 20:08 ` Richard Copley @ 2017-10-05 22:44 ` Paul Eggert 0 siblings, 0 replies; 7+ messages in thread From: Paul Eggert @ 2017-10-05 22:44 UTC (permalink / raw) To: Richard Copley; +Cc: Eli Zaretskii, Emacs Development On 10/05/2017 01:08 PM, Richard Copley wrote: > FWIW it seems to me that the results of the buffer_display_table calls > in c_string_width and lisp_string_width are dereferenced unchecked, > in character.c. I don't see how those two functions dereference the results. Although they do pass results to char_width, char_width has a run-time check that prevents dereferencing the results when null. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-05 19:32 ` Paul Eggert 2017-10-05 20:08 ` Richard Copley @ 2017-10-06 15:16 ` Richard Stallman 2017-10-06 17:23 ` Richard Copley 1 sibling, 1 reply; 7+ messages in thread From: Richard Stallman @ 2017-10-06 15:16 UTC (permalink / raw) To: Paul Eggert; +Cc: rcopley, eliz, emacs-devel [[[ To any NSA and FBI agents reading my email: please consider ]]] [[[ whether defending the US Constitution against all enemies, ]]] [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > The GCC bug is not fixed yet. etags.c confused GCC by using an > assignment inside an 'if' expression, which is contrary to the usual GNU > style, and last year I worked around the GCC bug by changing etags.c to > use a cleaner style. I notice that indent.c also has an assignment > inside an 'if' expression that is relevant to these warnings. Richard, > does it help to recode indent.c to use the usual GNU style, as in the > attached patch? I don't understand the question. An assignment inside an if condition is somewhat ugly, and moving the assignment out of the code is in general a good thing for clarity. So by all means do it. If that works around a GCC bug, that is good -- but the bug still needs to be fixed. -- Dr Richard Stallman President, Free Software Foundation (gnu.org, fsf.org) Internet Hall-of-Famer (internethalloffame.org) Skype: No way! See stallman.org/skype.html. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: C warning in MSYS2 build 2017-10-06 15:16 ` Richard Stallman @ 2017-10-06 17:23 ` Richard Copley 0 siblings, 0 replies; 7+ messages in thread From: Richard Copley @ 2017-10-06 17:23 UTC (permalink / raw) To: Richard Stallman; +Cc: Eli Zaretskii, Paul Eggert, Emacs Development On 6 October 2017 at 16:16, Richard Stallman <rms@gnu.org> wrote: > [[[ To any NSA and FBI agents reading my email: please consider ]]] > [[[ whether defending the US Constitution against all enemies, ]]] > [[[ foreign or domestic, requires you to follow Snowden's example. ]]] > > > The GCC bug is not fixed yet. etags.c confused GCC by using an > > assignment inside an 'if' expression, which is contrary to the usual GNU > > style, and last year I worked around the GCC bug by changing etags.c to > > use a cleaner style. I notice that indent.c also has an assignment > > inside an 'if' expression that is relevant to these warnings. Richard, > > does it help to recode indent.c to use the usual GNU style, as in the > > attached patch? > > I don't understand the question. Paul was asking me to test the attached patch. > An assignment inside an if condition is somewhat ugly, > and moving the assignment out of the code is in general > a good thing for clarity. So by all means do it. That's not the proposal. You're free to read the patch if you wish. > If that works around a GCC bug, that is good -- but the bug > still needs to be fixed. Indeed, but fortunately the impact is low. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-10-06 17:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-05 17:44 C warning in MSYS2 build Richard Copley 2017-10-05 18:54 ` Eli Zaretskii 2017-10-05 19:32 ` Paul Eggert 2017-10-05 20:08 ` Richard Copley 2017-10-05 22:44 ` Paul Eggert 2017-10-06 15:16 ` Richard Stallman 2017-10-06 17:23 ` Richard Copley
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.