local variable inside empty function must be assigned a value to avoid exception

[snarkysparky]

Atmel studio 7 with SAMD20G15 as target.

void ProcessSercom0Word(unsigned int Sercom0Word)
{
   PrtCntrlBrdCommands PrtCntrCmd;
   
   
   PrtCntrCmd = (PrtCntrlBrdCommands)(( Sercom0Word & 0xf000) >> 12);   // if commented out program will hang ...

/*   ..................   everything commented out after this to final brace.....

   switch(PrtCntrCmd)
   {
      case WRITE_BUFF_LOWER_12_BITS:  // write SPI receive buff lower 12 bits
......

*/
}

Can anyone explain what is happening here. 

[mcovington]

And the symptom is the program hanging, or an error message, or what?

Does this function in any way interact with anything that is timing-critical?

Does the compiler refrain from compiling the function if it has nothing in its body?  Try changing the one and only statement to something even simpler and see what happens.

[vidi]

Hi,

1) Please paste here a disassembly dump of the ProcessSercom0Word function.
( arm-none-eabi-objdump --disassemble ProcessSercom0Word object_file.o ) or ( arm-none-eabi-objdump -S object_file.o ).
It is a CortexM0+ architecture so C/to/assembly should be pretty straightforward.

2) Try to decrease optimization level to -O0, if is not already set.

[SiliconWizard]

If you comment out the assignment to PrtCntrCmd, at the switch(PrtCntrCmd) statement what do you

• want it to do?
• think it will do?
• think the compiler is at liberty to do?

Same question: unless there is some missing code assigning PrtCntrCmd a value between the declaration and the 'switch', what exactly is this piece of code supposed to mean?

[Nominal Animal]

My innate telepathy tells me OP is dealing with a much more mundane problem: that C-style comments (/* ... */) do not nest.

Simply put,

Code: [Select]

    /*
        I want this piece to be commented out

        /*
            and I want this piece to be commented out too
        */
    */
will abort the compilation on the second */ because comments do not nest.  Within C-style comments, /* is ignored, and the first */ ends the comment.

[snarkysparky]

OK,  thanks for all the replies.   I was taking time to make sure i had a repeatable situation.

in the code below ,  dummyglobal was created solely to get the compiler to link in an empty function.  It  appears nowhere else.
If the assignment to dummyglobal is commented out my program hangs since my 1ms interrupt stops toggling a debug output.

The function has no executable statements enabled other than the empty switch cases.
I changed optimization to O0.

Answering what I expect question..

I expect it to not be necessary to call an empty function to prevent a program hang...   I could code around it but I think I need to understand what is going on.

Thanks

Code: [Select]

void ProcessSercom0Word(unsigned int Sercom0Word)
{


PrtCntrlBrdCommands PrtCntrCmd;
PrtCntrCmd = (PrtCntrlBrdCommands)(( Sercom0Word & 0xf000) >> 12); 

    dummyglobal = PrtCntrCmd;   // added to get the compiler to include empty function
                                // if commented out the program hangs somewhere.
switch(PrtCntrCmd)
{
   case WRITE_BUFF_LOWER_12_BITS:  // write SPI receive buff lower 12 bits
   {
//  SpiRecieveBufferWord &= 0x00fff000;    // clear lower 12 bits
//  SpiRecieveBufferWord |= (Sercom0Word & 0x0fff);
        //  Debug16BitWordOutPin15(SpiRecieveBufferWord);  // its good here
  break;
   }
   
   case WRITE_BUFF_UPPER_12_BITS:  // write SPI recieve buff upper 12 bits
   {
//   SpiRecieveBufferWord &= 0x00000fff;    // clear upper 12 bits
//   SpiRecieveBufferWord |= ((Sercom0Word & 0x0fff )<< 12);
   break;
   }  
   
   case CLEAR_BUFF:
   {
//   SpiRecieveBufferWord = 0;
   break;
   }    
   
   case CYCLE_CUTTER:
   {
    //  CutterCycleBegin();
   break;
   }    
 
   case STEP_RUN:
   {
  //SpiRecieveBufferWord = 1200;  //  works here
   
//   if(SpiRecieveBufferWord &  (1 << 23))  // 24th bit is set for a negative move.  use remaining 23 bits for num steps
//   {
//    StepMotorDrive((SpiRecieveBufferWord & 0x007fffff), 0);   // dir != 1 is reverse
//   }
//   else
//   {
//   StepMotorDrive((SpiRecieveBufferWord & 0x007fffff), 1);   // fwd
//   }
//  Debug16BitWordOutPin15(SpiRecieveBufferWord);  corrupt here
   break; 
   }

   case LOAD_ERROR_INTO_BUFF:
   {
//   LoadWordIntoSPI_TXBuff(GlobalErr);
   
   break;
   }

   case LOAD_DEBUG_WORD_INTO_BUFF:
   {
   
//   LoadWordIntoSPI_TXBuff(DebugWordArray[Sercom0Word & 0x0000000f]);
   break;
   }

   case SEVEN:
   {
   break;
   }

   case EIGHT:
   {
   break;
   }

   case NINE:
   {
   break;
   }

   case TEN:
   {
   break;
   }

   case ELEVEN:
   {
   break;
   }

   case TWELVE:
   {
   break;
   }

   case THIRTEEN:
   {
   break;
   }
   case FOURTEEN:
   {
   break;
   }

   default:
   {
   
   
   }
   
}
 
}







[newbrain]

It is still not very clear to me which cases are working (if any) and which are not.

If the assignment to PrtCntrCmd is removed, and the switch statement is left in place, the behaviour is undefined as you are trying to use the value of an automatic storage duration variable while still indeterminate.
Doubly so with the assignment to the global variable.
Hanging is very possible*.

Could you please reduce the code to the minimum working case and the minimum failing one?
What do you mean "to get the compiler to include empty function"?
Is the function called at all?

I think Atmel Studio has an integrated debugger - can you see where the code is hanging (probably a default handler) and the calls in the stack?

* Nasal daemons are another possibility, though rarely seen in practice.

[snarkysparky]

Newbrain.   None of the cases are enabled.  The program should at least run the 1ms interrupt routine.

tggzzz.   The original problem statement was a little off.  If you look at the latest code post I am always assigning PrtCntrCmd based on the function passed argument.
              Since all the case contents are commented out the compiler did not link in the function call.  There is a switch present but none of the cases have any work to do.  I would guess its smart enough
              to just not call the function.  But not smart enough to avoid calling it based on assignment of a global variable within the function.

I did something that fixed the issue.  (It should not have fixed it though ) .   There were sercom interrupt flags being set for interrupts that were not enabled  ( so I didn't bother to clear int sources that were not enabled )  but i put in
 REG_SERCOM0_SPI_INTFLAG = 0xff;   // clear all interrupts    that is executed in the sercom0 ISR regardless of interrupt source.   It fixed the hanging.

gawd this is a lot of fun.

thanks all.    I am still interested in any theories as to why the original issue happened that  function with no external effect that I can see needs to be called to avoid hang.

[SiliconWizard]

It's still not quite clear to me what exactly you did and what exactly was happening and what exactly you were expecting, but with the info I gathered here, I'd venture this could be some case where qualifying "volatile" one of your variables could be the answer to your problem. (But this is just based on the little I've gotten.)

[Nominal Animal]

Let's dissect the code:

    void ProcessSercom0Word(unsigned int Sercom0Word)
    {
        PrtCntrlBrdCommands PrtCntrCmd;
        PrtCntrCmd = (PrtCntrlBrdCommands)(( Sercom0Word & 0xf000) >> 12);

That fourth line is not a function call, but a cast.  The part in the right-side parentheses takes bits 12..15 of Sercom0Word and shifts them down 12 bits, then casts the result to PrtCntrlBrdCommands type (values 0 to 15, inclusive).

If the rest of the code in the function does not use PrtCntrCmd, the compiler is free to omit the variable and the computation altogether.
        dummyglobal = PrtCntrCmd;
i.e. assigning its value to a global variable is just one way to tell the compiler the value is required.

If the rest of the function is basically
        switch (PrtCntrCmd) {
        case  0: break;
        case  1: break;
        case  2: break;
        case  3: break;
        case  4: break;
        case  5: break;
        case  6: break;
        case  7: break;
        case  8: break;
        case  9: break;
        case 10: break;
        case 11: break;
        case 12: break;
        case 13: break;
        case 14: break;
        case 15: break;
        }
    }
it does nothing, and definitely does not cause the program to hang.

However, it looks like this has something to do with SPI and interrupts.  They don't just magically work, with "doing something" "fixing issues".  No, the problematic version of the function simply did not do what you intended it to do.

Edited: I forgot, this is SAMD20.  So:

On SAMD20, REG_SERCOM0_SPI_INTFLAG refers to the SPI interrupt status flag and clear register.  A function handling the SPI interrupt must clear the status flag, by writing a value with bits set for the interrupt causes it has handled (see chapter 25.6.2.6 in the SAM D20 Family Data Sheet).  If it does not do so, the same interrupt is raised immediately after the function returns.  Then, the only thing running on the microcontroller is that interrupt handler function (and any higher-priority interrupts that might occur at the same time).  The SAMD20 is not stuck or hung; it is doing exactly what you have instructed it to do, repeatedly calling that interrupt handler without allowing anything else to run (except other interrupts with same or higher priority).

[snarkysparky]

""it is doing exactly what you have instructed it to do, repeatedly calling that interrupt handler without allowing anything else to run""

for interrupts i DO NOT have enabled?

I though I was clear.  The flags are set regardless of interrupt being enabled.   I definitely clear the ones I handled bitwise.

Compiler isn't acting right.  Thinking of copying the source code elsewhere and creating a new project.   Does this ever help things ?

[Nominal Animal]

for interrupts i DO NOT have enabled?

They might get enabled by libraries or auto-included code.  If you are using Arduino, that is almost certain.  A lot of stuff is done "behind your back", in the hopes that it lowers the barriers of entry into microcontroller programming.

Compiler isn't acting right.  Thinking of copying the source code elsewhere and creating a new project.   Does this ever help things ?

It's not the compiler.  You need to be in full control of the project, or programming becomes uttering magical incantations in the hope of desirable effect.

I always start from fresh, testing individual features in essentially separate projects, and build the final project by adding each tested feature, and re-testing the combined project.

When developing an individual feature, I limit the space of possible issues, thus making development more efficient.  When adding an individually tested feature, any bugs that crop up must be due to interaction between the new and existing features; for example, namespace clashes (functions or variables with the same name in the same namespace; often global functions of variables).  This too makes me more efficient.  I like that a lot, because I'm utterly lazy.

How and what you have done thus far, is a mystery to us, but yeah, it does sound like you probably should start with a fresh project, instead of throwing code at a proverbial wall (of an existing project) and seeing what sticks.

[bson]

Could be a timing problem, where when the function is optimized out of existence enough delay is removed to cause the program to fail elsewhere.

[snarkysparky]

I found the issue.   Problem in this code snippet..

Code: [Select]


#define ANALOG_READ_LIST_LEN 6
unsigned int  AnalogReadList[ANALOG_READ_LIST_LEN];


AnalogReadIndex++;
if(AnalogReadIndex > ANALOG_READ_LIST_LEN )AnalogReadIndex = 0;

AnalogChannelValues[AnalogReadList[AnalogReadIndex]] = REG_ADC_RESULT ;


See the problem ??   

[madires]

There's no AnalogReadList[6]! If you have an array with six elements, they are counted from zero to five. The code line dealing with the index overflow resets AnalogReadIndex when AnalogReadIndex is larger than 6, i.e. you end up with AnalogReadList[6]. You can change the ">" into a ">=" or ANALOG_READ_LIST_LEN into (ANALOG_READ_LIST_LEN - 1) to limit AnalogReadIndex to five. Also be aware that AnalogReadIndex should start with zero, since AnalogReadList[0] is the array's first element.

[newbrain]

Undefined Behaviour is usually very reliable.

(to mess up one's code).

[Nominal Animal]

I found the issue.

Good ol' off-by-one!

Also, a good example of how overwriting arbitrary memory (whatever followed AnalogReadList in memory) can produce really, really odd symptoms.

And thus, an excellent example of why one should always create a Minimal, Complete, and Verifiable example (one that has the minimum amount of code that exhibits the problem, but is compilable and testable by itself without other code, and reproduces the exact same problem), when asking for help with a bug.

You got less than helpful answers, because the actual problem was hiding elsewhere.  It was akin to an user asking a sparky to fix a light fixture, when the real issue is a miswired fuse box -- but only bringing the light fixture for the sparky to fix.

In any case, finding and fixing the problem is always a