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
>