UnixOSProcessPlugin and accessing C array out of bounds
Hi Esteban, while building packages for OBS and going the compile warnings (one of the nice things of newer compilers is the increased diagnostic) I noticed this: [ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4525:20: warning: iteration 64u invokes undefined behavior [-Waggressive-loop-optimizations] [ 196s] if ((semaIndices[sigNum]) > 0) { [ 196s] ^ [ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4524:9: note: containing loop [ 196s] while (sigNum <= (signalArraySize())) { while (sigNum <= (signalArraySize())) { if ((semaIndices[sigNum]) > 0) { setSignalNumberhandler(sigNum, (originalSignalHandlers())[sigNum]); } sigNum += 1; } semaIndices has signalArraySize()/NSIG number of entries and is zero based, it is being accessed with sigNum==NSIG which means one entry beyond the memory of the array? Can you confirm this? restoreDefaultSignalHandlers "Restore signal handlers to their original behaviors." | sigNum | <returnTypeC: 'void'> semaIndices = nil "nil if in interpreter simulation" ifFalse: [sigNum := 1. [sigNum <= self signalArraySize] whileTrue: [((semaIndices at: sigNum) > 0) ifTrue: [self setSignalNumber: sigNum handler: (self originalSignalHandlers at: sigNum).]. sigNum := sigNum + 1]] So it is one based but the array doesn't have an extra element to make it work? How is this normally handled in slang code? holger
Holger, Good catch, thank you. There were two arrays dimensioned wrong in OSProcessPlugin in the Smalltalk slang source. I fixed this, and the warning should be gone after the sources are next generated. Esteban, I fixed the oscog branch also. I could not easily test it, but it is a small change so hopefully should be no problem when you next generate the plugin. Dave On Wed, Feb 22, 2017 at 04:25:05PM +0700, Holger Freyther wrote:
Hi Esteban,
while building packages for OBS and going the compile warnings (one of the nice things of newer compilers is the increased diagnostic) I noticed this:
[ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4525:20: warning: iteration 64u invokes undefined behavior [-Waggressive-loop-optimizations] [ 196s] if ((semaIndices[sigNum]) > 0) { [ 196s] ^ [ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4524:9: note: containing loop [ 196s] while (sigNum <= (signalArraySize())) {
while (sigNum <= (signalArraySize())) { if ((semaIndices[sigNum]) > 0) { setSignalNumberhandler(sigNum, (originalSignalHandlers())[sigNum]); } sigNum += 1; }
semaIndices has signalArraySize()/NSIG number of entries and is zero based, it is being accessed with sigNum==NSIG which means one entry beyond the memory of the array? Can you confirm this?
restoreDefaultSignalHandlers "Restore signal handlers to their original behaviors."
| sigNum | <returnTypeC: 'void'> semaIndices = nil "nil if in interpreter simulation" ifFalse: [sigNum := 1. [sigNum <= self signalArraySize] whileTrue: [((semaIndices at: sigNum) > 0) ifTrue: [self setSignalNumber: sigNum handler: (self originalSignalHandlers at: sigNum).]. sigNum := sigNum + 1]]
So it is one based but the array doesn't have an extra element to make it work? How is this normally handled in slang code?
holger
Thanks david! On Thu, 23 Feb 2017 03:00:08 +0100, David T. Lewis <lewis@mail.msen.com> wrote:
Holger,
Good catch, thank you. There were two arrays dimensioned wrong in OSProcessPlugin in the Smalltalk slang source. I fixed this, and the warning should be gone after the sources are next generated.
Esteban,
I fixed the oscog branch also. I could not easily test it, but it is a small change so hopefully should be no problem when you next generate the plugin.
Dave
On Wed, Feb 22, 2017 at 04:25:05PM +0700, Holger Freyther wrote:
Hi Esteban,
while building packages for OBS and going the compile warnings (one of the nice things of newer compilers is the increased diagnostic) I noticed this:
[ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4525:20: warning: iteration 64u invokes undefined behavior [-Waggressive-loop-optimizations] [ 196s] if ((semaIndices[sigNum]) > 0) { [ 196s] ^ [ 196s] /usr/src/packages/BUILD/src/plugins/UnixOSProcessPlugin/UnixOSProcessPlugin.c:4524:9: note: containing loop [ 196s] while (sigNum <= (signalArraySize())) {
while (sigNum <= (signalArraySize())) { if ((semaIndices[sigNum]) > 0) { setSignalNumberhandler(sigNum, (originalSignalHandlers())[sigNum]); } sigNum += 1; }
semaIndices has signalArraySize()/NSIG number of entries and is zero based, it is being accessed with sigNum==NSIG which means one entry beyond the memory of the array? Can you confirm this?
restoreDefaultSignalHandlers "Restore signal handlers to their original behaviors."
| sigNum | <returnTypeC: 'void'> semaIndices = nil "nil if in interpreter simulation" ifFalse: [sigNum := 1. [sigNum <= self signalArraySize] whileTrue: [((semaIndices at: sigNum) > 0) ifTrue: [self setSignalNumber: sigNum handler: (self originalSignalHandlers at: sigNum).]. sigNum := sigNum + 1]]
So it is one based but the array doesn't have an extra element to make it work? How is this normally handled in slang code?
holger
-- Using Opera's mail client: http://www.opera.com/mail/
participants (3)
-
David T. Lewis -
Holger Freyther -
stepharong