bit fields warning help, AS6.1 + Naggy

Log in or register to post comments
Go To Last Post
13 posts / 0 new
Author
Message
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
#1
Posted by midea31: Mon. Apr 8, 2013 - 03:34 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
3485 : cast to 'volatile bit_field *' from smaller integer type 'uint16_t' (aka 'unsigned short')

I have hundreds of this warning.

typedef struct 
{ 
uint8_t bit0 : 1, 
bit1 : 1, 
bit2 : 1, 
bit3 : 1, 
bit4 : 1, 
bit5 : 1, 
bit6 : 1, 
bit7 : 1; 
} bit_field;

#define	button1 (*(volatile bit_field *) (_SFR_ADDR(PINA))).bit0

the warnings point to every instance I use the defined ports

if (button1 == HIGH) {
...
}

led1 = LOW;

etc...

as far as I understand, it is warning me that I am casting to a larger data type from a smaller data type?
Or does the warning come from the fact I am casting to a pointer to a struct?
I know it is just a warning but some warnings are very useful and I don't want to ignore them.

Also, I am using AS6.1 with Naggy extension.
I am building this for m2560.
And these are the compile options (which are default, i think)

-funsigned-char -funsigned-bitfields -Os -ffunction-sections -fpack-struct -fshort-enums -g2 -Wall -mmcu=atmega2560 -c -std=gnu99 -MD -MP -MF "$(@:%.o=%.d)" -MT"$(@:%.o=%.d)" -MT"$(@:%.o=%.o)"

Also, I cannot recall if I have seen these errors when I have not yet installed Naggy. I don't know if it is Naggy who generates these.

Tags:

Tools, Atmel Studio (AVR-related), ATmega2560
Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Mon. Apr 8, 2013 - 05:56 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried to disable Naggy, and indeed the warnings disappeared.
What does this mean? Is Naggy wrong to point out those warnings?
Is avr-gcc wrong to NOT point them out?

Not knowing the context, is it really dangerous to cast a smaller variable size to a larger one?

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101574 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Mon. Apr 8, 2013 - 12:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You have commas instead of semicolons in your struct definition.

Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Tue. Apr 16, 2013 - 12:58 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I tried this one yesterday.

typedef struct 
{ 
uint8_t bit0 : 1;
uint8_t bit1 : 1;
uint8_t bit2 : 1;
uint8_t bit2 : 1;
uint8_t bit3 : 1;
uint8_t bit4 : 1;
uint8_t bit5 : 1;
uint8_t bit6 : 1;
} bit_field;

I don't know the difference of that with the original one.
The warnings still persist. And I am still no wiser about the warning message.

3485 : cast to 'volatile bit_field *' from smaller integer type 'uint16_t' (aka 'unsigned short')
Top
Levenkay
Levenkay's picture
Level: Resident
Joined: Thu. Dec 20, 2007
Posts: 966 View posts
Location: Portland, Oregon, USA
Posted by Levenkay: Tue. Apr 16, 2013 - 02:07 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bit fields are fine as long as you don't depend on their arrangement/allocation inside the larger datatypes. I like to use them, for example, to compact the storage of data structures used as elements of const data tables. The initializer list for such a table only needs to be written to match the bitfield-containing structure's declaration, regardless of whether the compiler likes to allocate bitfields in a little- or big-endian way. If you go trying to overlay (by casting - a disgusting practice in its own right) an "eight bitfield struct" onto a specific piece of hardware (like an eight-bit I/O port), though, you WILL be depending on the particular endianness of the compiler, and 'bang' goes portability.

Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Tue. Apr 16, 2013 - 02:17 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I found this particular code snippet here in the forums and I thought it was a cool way of giving an alias to a port.

I understand the issue about portability and endianness, but could you please explain the warning message. And also your comment about

Quote:
by casting - a disgusting practice in its own right

which is obviously what I am doing here.

Top
stevech
stevech's picture
Level: Raving Lunatic
Joined: Tue. Dec 18, 2001
Posts: 6119 View posts
Posted by stevech: Tue. Apr 16, 2013 - 03:10 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

For OP's original code... the struct contains constants. Better to use an enum, eh?

Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Tue. Apr 16, 2013 - 05:20 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

does it?
I think the struct means that I am creating eight 1bit-wide members.

Top
saaadhu
saaadhu's picture
Level: Hangaround
Joined: Sun. Jul 4, 2010
Posts: 279 View posts
Location: Chennai, India
Posted by saaadhu: Tue. Apr 16, 2013 - 02:29 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Author here - looks like an incorrect warning off the top of my head. Clang is likely emitting this warning because it thinks a pointer is bigger than uint16_t (true for other architectures like x86, not true for most AVRs). You can make Naggy shut up about this specific diagnostic by adding it to the blacklist (see https://github.com/saaadhu/naggy... )

Regards

Senthil

 

blog | website

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101574 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Tue. Apr 16, 2013 - 02:44 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry but why would you be using _SFR_ADDR(PINA)? The PINA macro itself is already define in terms of _SFR_ADDR().

Oh and when I copy the struct definition you show most recently Naggy (helpfully!) points out that there are two copies of bit2 in it!

The following has no Naggy or compiler/linker errors or warnings:

#include 

typedef struct
{
	uint8_t bit0 : 1;
	uint8_t bit1 : 1;
	uint8_t bit2 : 1;
	uint8_t bit3 : 1;
	uint8_t bit4 : 1;
	uint8_t bit5 : 1;
	uint8_t bit6 : 1;
} bit_field;

#define   button1 (*(volatile bit_field *) (&PINA)).bit0 
	
int main(void) {
	if (button1 == 1) {
		PORTB = 0x55;
	}
}

it generates:

0000009a 
:
	
int main(void) {
	if (button1 == 1) {
  9a:	c8 9b       	sbis	0x19, 0	; 25
  9c:	02 c0       	rjmp	.+4      	; 0xa2 
		PORTB = 0x55;
  9e:	85 e5       	ldi	r24, 0x55	; 85
  a0:	88 bb       	out	0x18, r24	; 24
	}
}
  a2:	80 e0       	ldi	r24, 0x00	; 0
  a4:	90 e0       	ldi	r25, 0x00	; 0
  a6:	08 95       	ret

Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Tue. Apr 16, 2013 - 03:16 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Oh and when I copy the struct definition you show most recently Naggy (helpfully!) points out that there are two copies of bit2 in it!

Aha! You are right of course. I typed that second bit manually. I know, I'm supposed to copy-paste the actual code (which i did in the OP, btw). But I was away from my computer at the time. Anyway sorry about that bit.

Quote:
Sorry but why would you be using _SFR_ADDR(PINA)? The PINA macro itself is already define in terms of _SFR_ADDR().

Hmmm. I have no idea. I just copied the code snippet somewhere here in the forums. Thought it was cool, tried to use it, it worked, no problem.
I will try this tomorrow when I get to my workstation.
Thanks for trying it out!

Quote:
Clang is likely emitting this warning because it thinks a pointer is bigger than uint16_t (true for other architectures like x86, not true for most AVRs).

This was my thought initially, but it would seem clawson has fixed it. I will report back tomorrow.
Thanks saaadhu for the link. And thank you for Naggy!

Top
Gold Markclawson
clawson's picture
Level: Moderator
Joined: Mon. Jul 18, 2005
Posts: 101574 View posts
Location: (using avr-gcc in) Finchingfield, Essex, England
Posted by clawson: Tue. Apr 16, 2013 - 03:23 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Hmmm. I have no idea. I just copied the code snippet somewhere here in the forums. Thought it was cool, tried to use it, it worked, no problem.
I will try this tomorrow when I get to my workstation.
Thanks for trying it out!

This is the problem with picking up code from the internet. You never really know if you are looking at a work of utter genius or a complete pile of shite. I'd tend to go by recommendations of others who either have experience of something they considered good or those that know genius when they see it. Personally I like anything by a contributor here called "danni" and would recommend his sbit.h which is to be found on page 5 (is it?) of the "bit manipulation 101" thread in tutorial forum. I won't recount the entire thing here but it starts..

#ifndef _sbit_h_
#define _sbit_h_

#include 

//                      Access bits like variables:
struct bits {
  uint8_t b0:1, b1:1, b2:1, b3:1, b4:1, b5:1, b6:1, b7:1;
} __attribute__((__packed__));
#define SBIT_(port,pin) ((*(volatile struct bits*)&port).b##pin)
#define SBIT(x,y)       SBIT_(x,y)

#ifdef PORTA

#define	PORT_A0		SBIT( PORTA, 0 )
#define	PORT_A1		SBIT( PORTA, 1 )
#define	PORT_A2		SBIT( PORTA, 2 )
etc.

You can then simply:

#define button1 PIN_A0

then 

int main(void) {
   if (button1 == 1) {
      PORTB = 0x55;
   }
}

as before. But by a rather circuitous route you've effectively re-invented this anyway ;-)

(oh and convention says it should be "BUTTON1" not "button1". The use of all upper case tells the reader to be on the lookout for a macro definition).

Top
midea31
midea31's picture
Level: Hangaround
Joined: Fri. Jul 22, 2011
Posts: 172 View posts
Posted by midea31: Thu. Apr 18, 2013 - 12:25 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yep. The extra _SFR_ADDR did cause the warnings.
Thank you for the help clawson!
Had I not used Naggy I would have never known about this.
Thanks saaadhu for a great extension.

Quote:
I'd tend to go by recommendations of others who either have experience of something they considered good or those that know genius when they see it.

And with that, I will be taking your recommendation and use danni's sbit.h :wink:

Top