* Warnings/errors related to possibly clobbered variables
@ 2015-01-13 11:32 Dmitry Antipov
2015-01-13 11:51 ` Andreas Schwab
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Antipov @ 2015-01-13 11:32 UTC (permalink / raw
To: eggert; +Cc: emacs-devel
With:
configure --prefix=/not/exists --enable-checking --enable-check-lisp-object-type --enable-gcc-warnings --with-x-toolkit=lucid
I got:
../../emacs/src/keyboard.c: In function ‘read_char’:
../../emacs/src/keyboard.c:2376:15: warning: variable ‘c’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
Lisp_Object c;
^
But with:
configure --prefix=/not/exists --enable-checking --enable-check-lisp-object-type --enable-gcc-warnings --with-x-toolkit=no
the same warning is treated as fatal error:
../../emacs/src/keyboard.c: In function ‘read_char’:
../../emacs/src/keyboard.c:2376:15: error: variable ‘c’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
Lisp_Object c;
^
cc1: all warnings being treated as errors
Why this is so?
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 11:32 Warnings/errors related to possibly clobbered variables Dmitry Antipov
@ 2015-01-13 11:51 ` Andreas Schwab
2015-01-13 12:12 ` Dmitry Antipov
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2015-01-13 11:51 UTC (permalink / raw
To: Dmitry Antipov; +Cc: eggert, emacs-devel
Dmitry Antipov <dmantipov@yandex.ru> writes:
> configure --prefix=/not/exists --enable-checking --enable-check-lisp-object-type --enable-gcc-warnings --with-x-toolkit=lucid
case $with_x_toolkit in
lucid | athena | motif)
# Old toolkits mishandle 'const'.
nw="$nw -Wwrite-strings"
;;
*)
gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
;;
esac
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 11:51 ` Andreas Schwab
@ 2015-01-13 12:12 ` Dmitry Antipov
2015-01-13 16:21 ` Eli Zaretskii
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Antipov @ 2015-01-13 12:12 UTC (permalink / raw
To: Andreas Schwab; +Cc: eggert, emacs-devel
On 01/13/2015 02:51 PM, Andreas Schwab wrote:
> case $with_x_toolkit in
> lucid | athena | motif)
> # Old toolkits mishandle 'const'.
> nw="$nw -Wwrite-strings"
> ;;
> *)
> gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
> ;;
> esac
Yes, but the question really was "how seriously we should treat
warnings about XXX which might be clobbered by YYY" :-).
Dmitry
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 12:12 ` Dmitry Antipov
@ 2015-01-13 16:21 ` Eli Zaretskii
2015-01-13 17:07 ` Paul Eggert
2015-01-13 19:43 ` Stefan Monnier
0 siblings, 2 replies; 9+ messages in thread
From: Eli Zaretskii @ 2015-01-13 16:21 UTC (permalink / raw
To: Dmitry Antipov; +Cc: schwab, eggert, emacs-devel
> Date: Tue, 13 Jan 2015 15:12:52 +0300
> From: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org
>
> On 01/13/2015 02:51 PM, Andreas Schwab wrote:
>
> > case $with_x_toolkit in
> > lucid | athena | motif)
> > # Old toolkits mishandle 'const'.
> > nw="$nw -Wwrite-strings"
> > ;;
> > *)
> > gl_WARN_ADD([-Werror], [WERROR_CFLAGS])
> > ;;
> > esac
>
> Yes, but the question really was "how seriously we should treat
> warnings about XXX which might be clobbered by YYY" :-).
Why not declare the offending variables 'volatile' and forget about
all this stuff?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 16:21 ` Eli Zaretskii
@ 2015-01-13 17:07 ` Paul Eggert
2015-01-13 19:43 ` Stefan Monnier
1 sibling, 0 replies; 9+ messages in thread
From: Paul Eggert @ 2015-01-13 17:07 UTC (permalink / raw
To: Eli Zaretskii, Dmitry Antipov; +Cc: emacs-devel
Eli Zaretskii wrote:
> Why not declare the offending variables 'volatile' and forget about
> all this stuff?
When this topic last came up, I vaguely recall Stefan saying that we shouldn't
declare local variables 'volatile' merely to work around GCC bugs, and that we
should get GCC fixed instead. I filed a bug report, but the bug has not been
fixed yet; see:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=48968
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 16:21 ` Eli Zaretskii
2015-01-13 17:07 ` Paul Eggert
@ 2015-01-13 19:43 ` Stefan Monnier
2015-01-13 20:10 ` Eli Zaretskii
1 sibling, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2015-01-13 19:43 UTC (permalink / raw
To: Eli Zaretskii; +Cc: schwab, Dmitry Antipov, emacs-devel, eggert
> Why not declare the offending variables 'volatile' and forget about
> all this stuff?
FWIW, `volatile' might not necessarily give us the behavior we
want/need. IOW if the warning is legit, we should investigate and
decide exactly how to fix the problem (which might involve using
`volatile' or it might on the contrary involve making a copy of the
current value).
If the warning is not legit, then it's a problem in GCC and we shouldn't
do anything on our side other than report a bug.
Stefan "-Werror is wrong"
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 19:43 ` Stefan Monnier
@ 2015-01-13 20:10 ` Eli Zaretskii
2015-01-13 20:55 ` Stefan Monnier
2015-01-13 21:31 ` Paul Eggert
0 siblings, 2 replies; 9+ messages in thread
From: Eli Zaretskii @ 2015-01-13 20:10 UTC (permalink / raw
To: Stefan Monnier; +Cc: schwab, dmantipov, emacs-devel, eggert
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>, schwab@suse.de, eggert@cs.ucla.edu, emacs-devel@gnu.org
> Date: Tue, 13 Jan 2015 14:43:11 -0500
>
> > Why not declare the offending variables 'volatile' and forget about
> > all this stuff?
>
> FWIW, `volatile' might not necessarily give us the behavior we
> want/need.
How so? Are you saying that qualifying an automatic variable as
'volatile' might change the semantics of the program?
> IOW if the warning is legit, we should investigate and
> decide exactly how to fix the problem (which might involve using
> `volatile' or it might on the contrary involve making a copy of the
> current value).
>
> If the warning is not legit, then it's a problem in GCC and we shouldn't
> do anything on our side other than report a bug.
The compiler is rarely wrong in these cases, especially if the warning
comes from several different versions of it. Programs are much better
than people in examining control flow. They don't understand the
hidden relations between the offending variable and other conditions,
so they can flag problems that don't really exist. But they rarely
flag something that is clearly wrong.
IME, adding 'volatile' is generally TRT in these cases. Plus, it's
very simple, and goes "by the book" (a.k.a. the C Standard).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 20:10 ` Eli Zaretskii
@ 2015-01-13 20:55 ` Stefan Monnier
2015-01-13 21:31 ` Paul Eggert
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Monnier @ 2015-01-13 20:55 UTC (permalink / raw
To: Eli Zaretskii; +Cc: schwab, dmantipov, emacs-devel, eggert
>> > Why not declare the offending variables 'volatile' and forget about
>> > all this stuff?
>> FWIW, `volatile' might not necessarily give us the behavior we
>> want/need.
> How so? Are you saying that qualifying an automatic variable as
> 'volatile' might change the semantics of the program?
GCC's warning basically points out that the semantics of the program is
unclear, because when we come back from a longjmp we could either see
the latest value of the variable, or a value it had earlier when we did
the setjmp.
So using `volatile' chooses one of the two behaviors, and it might not
be the one we need.
> IME, adding 'volatile' is generally TRT in these cases. Plus, it's
> very simple, and goes "by the book" (a.k.a. the C Standard).
FWIW, the last time I had to fight with this kind of interaction between
longjmp and local variables, I ended up installing the patch below to
fix the random crashes I was seeing: the problem is more subtle than
just "add `volatile'".
Stefan
diff --git a/src/bytecode.c b/src/bytecode.c
index 0ea646a..f1bdfd9 100644
--- a/src/bytecode.c
+++ b/src/bytecode.c
@@ -501,7 +501,6 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
Lisp_Object args_template, ptrdiff_t nargs, Lisp_Object *args)
{
ptrdiff_t count = SPECPDL_INDEX ();
- ptrdiff_t volatile count_volatile;
#ifdef BYTE_CODE_METER
int volatile this_op = 0;
int prev_op;
@@ -509,14 +508,12 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
int op;
/* Lisp_Object v1, v2; */
Lisp_Object *vectorp;
- Lisp_Object *volatile vectorp_volatile;
#ifdef BYTE_CODE_SAFE
- ptrdiff_t volatile const_length;
- Lisp_Object *volatile stacke;
- ptrdiff_t volatile bytestr_length;
+ ptrdiff_t const_length;
+ Lisp_Object *stacke;
+ ptrdiff_t bytestr_length;
#endif
struct byte_stack stack;
- struct byte_stack volatile stack_volatile;
Lisp_Object *top;
Lisp_Object result;
enum handlertype type;
@@ -1122,9 +1119,6 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
PUSH_HANDLER (c, tag, type);
c->bytecode_dest = dest;
c->bytecode_top = top;
- count_volatile = count;
- stack_volatile = stack;
- vectorp_volatile = vectorp;
if (sys_setjmp (c->jmp))
{
@@ -1135,12 +1129,11 @@ exec_byte_code (Lisp_Object bytestr, Lisp_Object vector, Lisp_Object maxdepth,
handlerlist = c->next;
PUSH (c->val);
CHECK_RANGE (dest);
- stack = stack_volatile;
+ /* Might have been re-set by longjmp! */
+ stack.byte_string_start = SDATA (stack.byte_string);
stack.pc = stack.byte_string_start + dest;
}
- count = count_volatile;
- vectorp = vectorp_volatile;
NEXT;
}
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: Warnings/errors related to possibly clobbered variables
2015-01-13 20:10 ` Eli Zaretskii
2015-01-13 20:55 ` Stefan Monnier
@ 2015-01-13 21:31 ` Paul Eggert
1 sibling, 0 replies; 9+ messages in thread
From: Paul Eggert @ 2015-01-13 21:31 UTC (permalink / raw
To: Eli Zaretskii, Stefan Monnier; +Cc: schwab, dmantipov, emacs-devel
On 01/13/2015 12:10 PM, Eli Zaretskii wrote:
> The compiler is rarely wrong in these cases
But it does appear to be wrong in this case. That's why I filed a
compiler bug, and the GCC maintainers seem to agree that it's a bug.
Long ago when I ran into this set of GCC bugs, I shrugged and added
'volatile's to pacify the buggy GCCs. Stefan removed some of them as
part of some of his other fixes, but I suspect that a few 'volatile's
are not necessary even today (except perhaps to pacify the GCC bug).
I say "set of GCC bugs" because I suspect there are more than one, some
of which have been fixed, and some of which remain.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-01-13 21:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-13 11:32 Warnings/errors related to possibly clobbered variables Dmitry Antipov
2015-01-13 11:51 ` Andreas Schwab
2015-01-13 12:12 ` Dmitry Antipov
2015-01-13 16:21 ` Eli Zaretskii
2015-01-13 17:07 ` Paul Eggert
2015-01-13 19:43 ` Stefan Monnier
2015-01-13 20:10 ` Eli Zaretskii
2015-01-13 20:55 ` Stefan Monnier
2015-01-13 21:31 ` Paul Eggert
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.