bugAVR C Runtime Library - Bugs: bug #35003, wdt_enable() has a wrong code...

 
 

bug #35003: wdt_enable() has a wrong code sequence for many parts

Submitted by:  Luca Matteini <loucypher>
Submitted on:  Wed 07 Dec 2011 02:29:44 PM UTC  
 
Category: LibrarySeverity: 3 - Normal
Priority: 5 - NormalItem Group: Header files
Status: InvalidPercent Complete: 0%
Assigned to: NoneOpen/Closed: Closed
Release: 1.7.*Fixed Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Wed 07 Dec 2011 04:01:36 PM UTC, comment #5:

I admit I trusted quality of the compiler for a long time, then!

I don't change my mind anyway on finding questionable the fact how coding style changes every few lines, of the same source file, based on a feature only partially documented by silicon manufacturer.
s/bug/critical review/

Luca Matteini <loucypher>
Wed 07 Dec 2011 03:20:51 PM UTC, comment #4:

You'd be surprised to learn that the compiler has been
generating code like that for much more than 10 years now
(in every function prologue/epilogue when handling stack
frame creation/destruction).

Joerg Wunsch <joerg_wunsch>
Project Administrator
Wed 07 Dec 2011 03:03:11 PM UTC, comment #3:

Well, I wasn't aware of that!
I guess it's an extension to what happens with SEI instruction.
Note how in this last case all is clearly stated, as if the SEI instruction decoding, in itself, is to force execution of the following instruction, more than setting I flag (...)

IMHO even if that works, brings in some confusion in the sources, being the only cases relying on a feature poorly(?) documented (if any) -- all other cases are handled as in wdt_disable(), following all examples on Atmel data sheets.

Luca Matteini <loucypher>
Wed 07 Dec 2011 02:39:52 PM UTC, comment #2:

I-bit, of course

Georg-Johann Lay <gjlayde>
Wed 07 Dec 2011 02:39:16 PM UTC, comment #1:

AFAIK, setting the I-big for non-Xmega devices has a latency of 1 instruction, so the code looks fine.

Georg-Johann Lay <gjlayde>
Wed 07 Dec 2011 02:29:44 PM UTC, original submission:

While debugging my code I found a bug/race condition in avr/wdt.h that left me astonished (since that code is in effect for a long time now).

Assembly code (ATtiny, ATmega, AT90 parts) for the inline wdt_enable() possibly re-enables interrupts before the last write access to WDTCSR.
The bug is present both for STS/OUT instructions code.

Note in fact how for STS code we have
the OUT to SREG happens right before the last STS to WDTCSR, making the code non-atomic: the last STS to WDTCSR could be delayed (by an interrupt handler) for many cycles, voiding its effect (has to happen within 4 cycles from the precedent STS to WDTCSR).

The same happens for OUT instruction mode in wdt_enable(), while wdt_disable() is always correct (OUT to SREG is last instruction).

For ATxmega parts code in wdt_enable() is correct, but I can't see a wdt_disable() defined anywhere: am I right? Is that intentional?

~Lou

Luca Matteini <loucypher>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by joerg_wunsch (Posted a comment)
  • -unavailable- added by gjlayde (Posted a comment)
  • -unavailable- added by loucypher (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 07 Dec 2011 03:20:51 PM UTCjoerg_wunschStatusNone=>Invalid
      Open/ClosedOpen=>Closed

    Back to the top


    Powered by Savane 3.1-cleanup1