WTF?!! GCC bug or what?

Go To Last Post
16 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay. I'm going insane. Can anyone please explain me, why

#define F_CPU 4000000

int usart_init(struct usart_config *uc)
{
unsigned long ubrr = F_CPU / (16 * uc->baud) -1;

this does not work? sets ubrr to long_max, 2^32-1;

but this does:

ubrr = F_CPU;
ubrr /= 16;
ubrr /= uc->baud;
ubrr -= 1;

axos88

axos88

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

How do we know if it's a gcc bug or not without seeing what gcc generates for your code snippet? Or the version of gcc that you are using? ...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm using the latest version, the manual says it was built on 20071221, I don't know how to extract the exact data, or the code snippet either? Should I just copy&paste it from the disassembler, or what?

axos88

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Try this:

#define F_CPU 4000000UL

int usart_init(struct usart_config *uc) 
{ 
unsigned long ubrr = F_CPU / (16UL * uc->baud) -1; 

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

axos88, You need to add a "UL" suffix to your F_CPU value. Otherwise, your arithmetic will be at 16-bits in your initial equation.

#define F_CPU 4000000UL
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Isn't 'or what' the correct answer to the OP's question?

Smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Smiley: I don't know what your problem is, I wasn't asking in a bad tone, or something.

Actually the #define F_CPU 4000000 has been done by the AVR Studio from command line, but I will try to add the UL to 16, maybe that helps...

Btw, what does OP stand for? I realised that it will be the one asking the question, but what does it actually stnad for?

Thx

axos88

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If it 'bugs' the OP that gcc didn't automatically make F_CPU a UL, then its a 'bug'.

If someone who been on this forum a while says they found a bug in 'you name it', I would be about 10% inclined to believe them, otherwise I give any 'bug' talk about a 0% chance.

No offense is meant to the original poster (OP), its just that whenever there is talk of a 'bug', 99.99% of the time, its operator error.

My version of avr studio puts in the UL-

avr-gcc.exe  -mmcu=atmega88 -mmcu=atmega88 -Wall -gdwarf-2     -DF_CPU=8000000UL -Os -fsigned-char -MD -MP -MT usart_test.o -MF dep/usart_test.o.d  -x assembler-with-cpp -Wa,-gdwarf2 -c  ../usart_test.S

I would also consider skipping making your own define for F_CPU, and let avr studio put it in the makefile (which it does when you define the frequency in the project options), then you have only 1 number in 1 place to worry about.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
(16 * uc->baud)

I would guess is the expression that must be evaluated with 32 bits so try

(16UL * uc->baud)

/Lars

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can easily play around with various combos to see what happens-

#include 

//F_CPU already defined in makefile as 8000000UL

//define baud rate
#define baud1 38400

//variable baud rate
uint16_t baud2 = 38400;

//test variables (just assuming ubrrl only)
uint8_t test1,test2,test3,test4,test5,test6;

int main(void){
//this works, preprocessor does it all, as
//baud1 is a define
test1 = F_CPU / (16 * baud1) -1;

//this doesn't work, result of
//16 * baud2 is kept in an uint16_t
//the 16 * baud2 is the same as baud2 <<= 4;
test2 = F_CPU / (16 * baud2) -1;

//this works
test3 = F_CPU / (16 * (uint32_t)baud2) -1;

//this works
test4 = F_CPU / (16UL * baud2) -1;

//this works (preprocessor does the F_CPU/16)
test5 = F_CPU / 16 / baud2 - 1;

while(1);
}

so it appears in your case, its the '16 * uc->baud' that's causing your grief.

Usually you don't see baud rates stored in variables, so its normally just a preprocessor calculation.

looks like gcc is playing by the rules-
http://c-faq.com/expr/intoverflo...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

curtwm, Yeah, btw, I really think gcc *should* know that 4000000 is UL, because it cannot represent such a number with a lower integer type...

axos88

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

axos88 wrote:
curtwm, Yeah, btw, I really think gcc *should* know that 4000000 is UL, because it cannot represent such a number with a lower integer type...

I don't think computers be making any assumptions about anything.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
curtwm, Yeah, btw, I really think gcc *should* know that 4000000 is UL, because it cannot represent such a number with a lower integer type...

But as curtvm said, 4000000 is not your problem, it is 16 * an integer variable. There is no way that the compiler can see before runtime that this will overflow. It is up to you to make sure that the result will fit.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

> There is no way that the compiler can see before runtime
> that this will overflow.

Even if, the overflow might have been intended.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Actually if you remove the UL from the F_CPU, it will still work (at least it does for me). I don't know all the C rules for all of this, so I just try compiling different 'tests' to see what happens.

So make a define like '#define MY_CPU 4000000' (no UL), then replace F_CPU with MY_CPU in my little test code, and see what happens. You should see 0x3D0900 being used in the lss listing if the 4000000 stays a long (it does for me).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The type of integer constants is initially taken as `int', but then
automatically extended to longer types if the constant would otherwise
not fit (`unsigned int' being next, followed by `signed long' etc.).

The issue is really the subexpression of multiplying the baud rate
with 16. Both arguments of this subexpression conveniently fit into
`int', so the type of the entire expression is also `int'. Not only
that this can overflow, it could even produce a negative result
upon overflowing. So using 16UL or (uint32_t)16 is the correct way
to force that subexpression into the unsigned long/uint32_t domain,
which will in turn make the entire expression being evaluated with
that type.

Jörg Wunsch

Please don't send me PMs, use email if you want to approach me personally.