Project

General

Profile

Bug #5396

fix longjmp clobbering errors

Added by Gary Mills over 5 years ago. Updated almost 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
tools - gate/build tools
Start date:
2014-12-05
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

When building illumos with gcc 4.8.2, these errors appear:


input.c:66:12: error: variable 'savesig' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
dmake: Warning: Command failed for target `input.o'
display.c:390:12: error: variable 'savesig' might be clobbered by 'longjmp' or 'vfork' [-Werror=clobbered]
dmake: Warning: Command failed for target `display.o'
dmake: Warning: Target `install' not remade because of errors
The following command caused the error:
dmake: Warning: Command failed for target `cscope-fast'

These are both in usr/src/tools/cscope-fast . When I look at the code in the mygetch() function, I can't determine how it's supposed to work. Maybe somebody who knows more about signal handling than I can tell us. My workaround, which only suppresses the error, consists of this addition to the Makefile:


CERRWARN += -_gcc=-Wno-clobbered

Note that many other errors have also been suppressed in a similar manner in the Makefile .

History

#1

Updated by Igor Kozhukhov over 5 years ago

i'm using gcc-4.8.3 with ported patches from gcc-4.4.4-illumos and i have no this issue.
also i tried build my dilos-illumos on SPARC by patched gcc-4.8.3 and special updates, but my build failed on boot - need time investigate it and update some modules. build was started, but failed on some loads of modules, but it's another story.

#2

Updated by Rich Lowe over 5 years ago

For the clobbering warnings, it's probably nearly always safe to just make everything that gets jumped back over 'volatile' like it should be.

The only gap I can think of is if some other aspect of the state before/after the jmp hasn't been considered properly, so it's still worth thinking about it, I suppose. But mostly, adding the 'volatile' these things should have is probably better than gagging the warning, and as easy.

#3

Updated by Igor Kozhukhov over 5 years ago

Igor Kozhukhov wrote:

i'm using gcc-4.8.3 with ported patches from gcc-4.4.4-illumos and i have no this issue.
also i tried build my dilos-illumos on SPARC by patched gcc-4.8.3 and special updates, but my build failed on boot - need time investigate it and update some modules. build was started, but failed on some loads of modules, but it's another story.

sorry - i have checked my usr/src/tools/cscope-fast/Makefile and i confirm: i have rule with:
CERRWARN += -_gcc=-Wno-clobbered

all warnings rules are:
CERRWARN += -_gcc=-Wno-parentheses
CERRWARN += -_gcc=-Wno-implicit-function-declaration
CERRWARN += -_gcc=-Wno-unused
CERRWARN += -_gcc=-Wno-uninitialized
CERRWARN += -_gcc=-Wno-clobbered

i didn't work yet on investigation of it - how to resolve it.

#4

Updated by Gary Mills over 5 years ago

Here's the function in question:


63 int
64 mygetch(void)
65 {
-------------------------------------------------------------------------------
66 SIGTYPE (*savesig)(); /* old value of signal */
-------------------------------------------------------------------------------
67 int c;
68
69 /* change an interrupt signal to a break key character */
70 if (setjmp(env) == 0) {
71 savesig = signal(SIGINT, catchint);
72 (void) refresh(); /* update the display */
73 reinitmouse(); /* curses can change the menu number */
74 if (prevchar) {
75 c = prevchar;
76 prevchar = 0;
77 } else {
78 c = getch(); /* get a character from the terminal */
79 }
80 } else { /* longjmp to here from signal handler */
81 c = KEY_BREAK;
82 }
83 (void) signal(SIGINT, savesig);
84 return (c);
85 }

Yes, I was aware that adding `volatile' generally corrects the problem. However, in this case I'm not sure that was adequate. How does `savesig' get set correctly for the second `signal()'? Does the signal handling expect SysV or BSD signal behavior? Does signal handling actually work? I gave up looking and suppressed the error.

#5

Updated by Rich Lowe over 5 years ago

The uncertain nature of 'savesig's value in this code is exactly what the warning is telling you about.

If it were 'volatile' you'd be fine, since its value would certainly be in memory in the stack frame, and you'd certainly reload it when it was referenced after the jump.

#6

Updated by Gary Mills over 5 years ago

But, `savesig' is an uninitialized automatic variable. It's value is unknown initially. What is its value when setjmp() returns non-zero? It can't be the return value from the first signal() because that's in the other branch of the `if' statement. It seems to me that the second signal() will get garbage. Am I reading it wrong?

#7

Updated by Rich Lowe over 5 years ago

The first branch of the 'if' executes when we setjmp, on our stack frame.

When we longjmp, the registers all get restored from the jmpbuf, including the pc and stack pointer. We're at 'setjmp' again, only we return 1. memory, including our stack frame is unaffected. We take the 'else' branch this time, but 'sendsig' -- were it volatile, to make sure we reloaded it from the stack, rather than trusting any register we may think it's in -- would have the value it gained in the other branch of the if.

It'd be more readable if we did the signal() prior to the setjmp, but 'sendsig' would still need to be volatile, and the result would still be the same.

#8

Updated by Gary Mills over 5 years ago

  • Status changed from New to Feedback

Thanks for the explanation. I understand how it works now: it's an interrupt. That's how interrupts work.

There are two files that need the `volatile' addition: usr/src/tools/cscope-fast/display.c usr/src/tools/cscope-fast/input.c .

#9

Updated by Gary Mills over 5 years ago

There are actually four locations that exhibit this bug:


usr/src/tools/cscope-fast
usr/src/lib/libcurses
usr/src/lib/libdtrace
usr/src/lib/gss_mechs/mech_krb5

#10

Updated by Rich Lowe almost 5 years ago

  • Subject changed from gcc 4.8.2 longjmp errors for cscope-fast to fix longjmp clobbering errors
#11

Updated by Rich Lowe almost 5 years ago

  • Subject changed from fix longjmp clobbering errors to fix longjmp clobbering errors
#12

Updated by Electric Monk almost 5 years ago

  • Status changed from Feedback to Closed
  • % Done changed from 0 to 100

git commit 67a4bb8f9ad4c49e9aa9e21e2114a7c093c3a73a

commit  67a4bb8f9ad4c49e9aa9e21e2114a7c093c3a73a
Author: Gary Mills <gary_mills@fastmail.fm>
Date:   2015-05-30T20:11:55.000Z

    5396 fix longjmp clobbering errors
    Reviewed by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
    Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF