bugAVR C Runtime Library - Bugs: bug #22163, Everytime...

 
 

bug #22163: Everytime ATOMIC_BLOCK(ATOMIC_RESTORESTATE) C++ compiler generates warning - unused variable 'sreg_save'

Submitted by:  Tomasz Francuz <tfrancuz>
Submitted on:  Wed 30 Jan 2008 05:17:06 AM UTC  
 
Category: HeaderSeverity: 4 - Important
Priority: 9 - ImmediateItem Group: libc code
Status: NonePercent Complete: 0%
Assigned to: NoneOpen/Closed: Open
Release: AnyFixed Release: None

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Mon 03 Feb 2014 01:25:44 AM UTC, comment #5:

Can't reproduce this with:
- avr-g++ (GCC) 4.8.1
- avr-libc (debian) 1:1.8.0-4

This bug might have been fixed in avr-gcc as avr-libc haven't changed in time passed.

Just thought it might be good to clean obsolete bugs, thus tested and commented about my resulsts.

Jaakko Kantojärvi <raphendyr>
Thu 31 Jan 2008 10:16:58 AM UTC, comment #4:

I've just tried this code section, and there is a good reason for the "unused variable 'sreg_save'" warning - looking at the generated assembly, the variable is unused. In other words, the SREG is not restored after the block, as it is in the C version.

Could it be that the _attribute_((_cleanup_)) attribute is not working correctly for avr c++, at least when used in this way? If that's the case, then this is definitely a bug.

I've noticed another issue with the atomic.h functions - I don't think the memory blocks (asm("":::"memory")) are in the right places. It's important to ensure that any changes to memory are written out before interrupts are enabled - thus the block should appear before the sei() instructions, and before restoring SREG. The block is unnecessary before disabling interrupts. I could be wrong, but I think that's the correct ordering.

In C++ it is normally considered better to use a class and RAII for something like this. You should have a class such as:

class CriticalSection {
public:
CriticalSection() : _sreg( SREG ) { cli(); }
~CriticalSection() { asm("" ::: "memory"); SREG = _sreg; }
private:
uint8_t _sreg;
};

Then you simply declare a "CriticalSection" variable in the block you want to protect. Similar classes can be defined for non-critical sections, and for when you want to restore the old interrupt enable state, or to force the flag on or off.

As a workaround for the bug blocking cleanup functions, it's possible to use a RAII class within the same framework as the C-style atomic blocks. It's a little messy, but gives identical optimal assembly code (in my brief testing, anyway). I haven't gone through the details of patching this into atomic.h (I'm not sure what the policy of including c++ specific stuff is), but it can be added on at the end of atomic.h as it re-defines some macros:

#ifdef __cplusplus

// Override these macros:
#undef ATOMIC_RESTORESTATE
#undef ATOMIC_FORCEON
#undef NONATOMIC_RESTORESTATE
#undef NONATOMIC_FORCEOFF

class RestoreState {
public:
RestoreState() : _x( SREG ), _restore( true ) {} // For keeping sreg
RestoreState(uint8_t x) : _x( x ), _restore( false ) {} // For __ToDo loop hack
~RestoreState() { if (_restore) SREG = _x; }
operator bool() const { return _x; }; // For testing __ToDo
private:
uint8_t _x;
bool _restore;
};

class ForceOn {
public:
ForceOn() : _x( 0 ), _restore( true ) {} // For ignoring sreg
ForceOn(uint8_t x) : _x( x ), _restore( false ) {} // For __ToDo loop hack
~ForceOn() { if (_restore) sei(); }
operator bool() const { return _x; }; // For testing __ToDo
private:
uint8_t _x;
bool _restore;
};

class ForceOff {
public:
ForceOff() : _x( 0 ), _restore( true ) {} // For ignoring sreg
ForceOff(uint8_t x) : _x( x ), _restore( false ) {} // For __ToDo loop hack
~ForceOff() { if (_restore) cli(); }
operator bool() const { return _x; }; // For testing __ToDo
private:
uint8_t _x;
bool _restore;
};

#define ATOMIC_RESTORESTATE RestoreState sreg_save
#define ATOMIC_FORCEON ForceOn sreg_save
#define NONATOMIC_RESTORESTATE RestoreState sreg_save
#define NONATOMIC_FORCEOFF ForceOff sreg_save

#endif

David Brown <davidbrown>
Thu 31 Jan 2008 02:56:23 AM UTC, comment #3:

Sorry, but mistake I didn't mention that I'm using avr-g++ compiler. When using avr-gcc everything is working perfectly, but in g++ the following code produces warning:
#include <util/atomic.h>

int main()
{
volatile int b,a;
a=0;
b=0;
ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
{
if(b==0) a++;
}
}

I don't know if you consider it as a bug, but it would be great if ATOMIC_BLOCK could be used in c++.

Tomasz Francuz <tfrancuz>
Wed 30 Jan 2008 04:14:44 PM UTC, comment #2:

I have not seen this warning despite having used this macro in several projects. Are you using C++ Tomasz (I only have C projects)?

Lars Jonsson <lajon>
Wed 30 Jan 2008 03:20:35 PM UTC, comment #1:

Tomasz,
Could you please post a small test case that gives this warning?

Eric Weddington <arcanum>
Project Administrator
Wed 30 Jan 2008 05:17:06 AM UTC, original submission:

Everytime when using ATOMIC_BLOCK(ATOMIC_RESTORESTATE) macro I have got compiler warning - warning: unused variable 'sreg_save'. I'm using winavr-20071221.

Tomasz Francuz <tfrancuz>

 

(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 raphendyr (Posted a comment)
  • -unavailable- added by joerg_wunsch (Updated the item)
  • -unavailable- added by davidbrown (Posted a comment)
  • -unavailable- added by lajon (Posted a comment)
  • -unavailable- added by arcanum (Posted a comment)
  • -unavailable- added by tfrancuz (Submitted the item)
  • -unavailable- added by tfrancuz
  •  

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

     

     

    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri 16 Nov 2012 06:39:41 PM UTCarcanumPriority5 - Normal=>9 - Immediate
    Tue 22 Feb 2011 08:58:00 PM UTCarcanumPriority7 - High=>5 - Normal
      SummaryEverytime ATOMIC_BLOCK(ATOMIC_RESTORESTATE) compiler generates warning - unused variable \'sreg_save\'=>Everytime ATOMIC_BLOCK(ATOMIC_RESTORESTATE) C++ compiler generates warning - unused variable 'sreg_save'
    Wed 09 Jun 2010 08:24:07 AM UTCjoerg_wunschPriority3 - Low=>7 - High
    Wed 09 Jun 2010 08:24:06 AM UTCjoerg_wunschSeverity3 - Normal=>4 - Important
    Mon 11 Aug 2008 03:01:46 AM UTCarcanumPriority5 - Normal=>3 - Low
      ReleaseNone=>Any
    Mon 17 Mar 2008 04:36:16 PM UTCarcanumCategoryNone=>Header
    Wed 30 Jan 2008 05:17:07 AM UTCtfrancuzCarbon-Copy-=>Added tfrancuz

    Back to the top


    Powered by Savane 3.1-cleanup