Strange expression execution/compiler issue with gcc

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

Hi !

Being aware that I have an old environment (avoiding new as long as feasible, since something always breaks when getting new) I still would

like to check some.

 

I have an error that took me long time to find since I held it for totally inprobable that that it could go wrong....

So then Im doubting whether this is an "old good known bug" or my environment is somehow broken.  mingw 4.8.1

 

Its a failure of the compiler to get -1 X 1 to negativ value (!!)   //edit

 

In the code gcc compiled and downloaded onto a atmega8 the statement marked A, at end of code, is ok executed giving the printout, while B never happens

even though they are semantically identical, and should result in same execution. kind of unbelivable

 

#define F_CPU 8000000
#include <avr/io.h>
#include <avr/interrupt.h>
#include <util/delay.h>
#include <limits.h>
#define TRUE 1
#define FALSE 0

#define LED        PB0
#define LED_PORT   PORTB
#define LED_DDR    DDRB  

#define PWR_PIN     PB1
#define PWR_PORT   PORTB
#define PWR_DDR    DDRB 

#include "uart_main.h"

void      thermo_init();
uint16_t  thermo_read(); 

 // koefficient for up temp regulation and down reg resp, one per 100 degrees
int16_t temp;       // read from MAX6675
int32_t  ave;        // average temp value
int32_t  ave_old;
int32_t  der;
int32_t  temp_pred;
int32_t  temp_target;
int16_t  Terr;
int16_t  Pcorr;
int16_t  Pcurr;     // the current power control setting
int16_t  Prev;
int16_t  P;

char CR=13; char LF=10;
char outstring[40];
uint8_t I;

uint16_t baud=103;      // 4800 vid 8Mhz clock

uint16_t intrvl_temp_shift, intrvl_temp_max, intrvl_temp_min; 

void stat_init();
void stat(int16_t temp, int16_t cnt);

int main() {
//********************************************************************************

uint8_t  i;

   USART_Init(baud);            /* Set up transmitt UART */
   USART_Init_rcv();            /* set up UART reception with interrupt */ 

   sei();

   USART_Transmit(CR); USART_Transmit(LF);
   USART_Trsm_string("***** fk *****");
   USART_Transmit(CR); USART_Transmit(LF); 

   LED_DDR   |= ( 1 << LED  );  

   stat_init(); 

   while(1) {

      _delay_ms (1000);

     if(temp < 0) temp = 1; else temp = -1;   

     stat(temp,i); // gather info

  } // ** while

} // *** main()

void stat_init() {

   intrvl_temp_shift= -1;  // start on low side
   intrvl_temp_max  = SHRT_MIN;
   intrvl_temp_min  = SHRT_MAX;
}

void stat(int16_t t, int16_t cnt) {
    // performace info gathering for regulation algo
    int16_t fuckComp,FK,FF;

   fuckComp = (t - (int16_t)temp_target);

   FK = fuckComp;

   FF = intrvl_temp_shift * FK ;

   if( FF < 0 ) USART_Trsm_string(" fuuu   ");                             // <<<<<<<<<<<<<<<<**** A

   if( intrvl_temp_shift * fuckComp < 0 ) USART_Trsm_string(" ckkk   ");   // <<<<<<<<<<<<<<<<**** B

      itoa(fuckComp,outstring,10); USART_Trsm_string(outstring );USART_Trsm_string("." );
      itoa(intrvl_temp_shift,outstring,10); USART_Trsm_string(outstring );USART_Trsm_string("." );
      itoa((t - (int16_t)temp_target) ,outstring,10); USART_Trsm_string(outstring );USART_Trsm_string("!" );
      itoa(intrvl_temp_shift * fuckComp  ,outstring,10); USART_Trsm_string(outstring );USART_Trsm_string("!      " );

}

 

 

 

This topic has a solution.

Last Edited: Mon. Jan 3, 2022 - 09:52 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

gechxx wrote:

In the code gcc compiled and downloaded onto a atmega8 the statement marked A, at end of code, is ok executed giving the printout, while B never happens

even though they are semantically identical, and should result in same execution. kind of unbelivable

 

No, they are not semantically identical. It is a silly beaten-to-death well-known error, triggered by thoughtless mixing of signed and unsigned arithmetic.

 

(I assume that your platform uses 16-bit `int` type, meaning that integral promotions are not applicable to 16-bit values.)

 

Expression in statement B multiples signed and unsigned values of the same width, i.e. it multiplies `int16_t` and `uint16_t`. By rules of C and C++, the multiplication is performed in the domain of unsigned (!) type. I.e. the result of the multiplication is `uint16_t`. It cannot possibly be less than 0. Many compilers will actually issue a warning for your B, since that `if` is nonsensical: the condition is guaranteed to always be false. (For example, AVR GCC with `-Wextra`: https://godbolt.org/z/oxnx1M5jG)

 

Expression right before statement A does the same multiplication and produces the same unsigned result. However, then it forces that unsigned result into a signed (!) variable `FF`. If the result does not fit into the positive range of `FF` it wraps around and becomes negative. That `if` in statement A sees that negative value in `FF` and executes accordingly.

 

Every time you perform arithmetic on mixed signed and unsigned values, you have to keep such things in mind.

 

 

Dessine-moi un mouton

Last Edited: Mon. Jan 3, 2022 - 09:22 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

did you try:

if(  ( intrvl_temp_shift * fuckComp ) < 0 ) USART_Trsm_string(" ckkk   ");   // <<<<<<<<<<<<<<<<**** B

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:

did you try:

if(  ( intrvl_temp_shift * fuckComp ) < 0 ) USART_Trsm_string(" ckkk   ");   // <<<<<<<<<<<<<<<<**** B

 

 

This is not going to make any difference.

Dessine-moi un mouton

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Indeed not.

 

As you say:

 

gechxx wrote:

uint16_t intrvl_temp_shift, intrvl_temp_max, intrvl_temp_min; 

So intrvl_temp_shift is unsigned

gechxx wrote:

   intrvl_temp_shift= -1;  // start on low side

So that's not going to work!

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

AndreyT wrote:
Many compilers will actually issue a warning for your B

gechxx wrote:
avoiding new as long as feasible, since something always breaks when getting new

This might be a case where a more recent compiler would have given you that extra warning.

 

Might be worth looking at the warning options you have - maybe your current compiler could give that warning, if asked ... ?

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK, Your explanation seems feasible, however the implementation then is whack.  (gosh, I do feel like waisting my time...)

 

But then, I was under the impression that I had two signed, what you say is that the "C spec" says that they should be treated unsigned ?!

 

well, will it help if I go over to int8_t for both, will the compiler then compute    -1 * 1 as -1 ?

or will I have to make my own xor, using single if's .....

 

 

 

 

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

Ohhh gosh. Im blind as a bat. Sorry, again and again. Thanks very much for your help. I thought I had checked over and over......

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

Here's a summary of the promotion rules: https://en.cppreference.com/w/c/language/conversion . Thankfully compilers remember them better than we do.

 

BTW: The rule that bit you is:

  • If the unsigned type has conversion rank greater than or equal to the rank of the signed type, then the operand with the signed type is implicitly converted to the unsigned type.

 

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

gechxx wrote:
I thought I had checked over and over......

This is where creating a minimum but complete example can help you - with everything else cut out, you might have spotted yourself that you were trying to use an unsigned for a negative value ...

 

Also, do check out increasing your warning level.

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

awneil wrote:
Also, do check out increasing your warning level.
... or operating a linter.

Gimpel On-Line Message Reference Manual

570

Loss of sign (Context) (Type to Type) -- An assignment (or
      implied assignment, see Context) is being made from a negative
      constant into an unsigned quantity.  Casting the constant to
      unsigned will remove the diagnostic but is this what you want.
      If you are assigning all ones to an unsigned, remember that ~0
      represents all ones and is more portable than -1.

PC-lint Plus Online Demo - Gimpel Software - The Leader in Static Analysis for C and C++ with PC-lint Plus

"Dare to be naïve." - Buckminster Fuller

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

>If you are assigning all ones to an unsigned, remember that ~0  represents all ones and is more portable than -1.

 

I'm not criticising lint in general, but in this case I'm not convinced the above statement is true.

I thought it would be the other way round.

At least, assigning the value -1 to any unsigned type is guaranteed to result in the maximum value that the unsigned type can hold. Is there any case where that isn't all ones?

What would you get if assigning ~0 to an unsigned type on an implementation using say ones' complement?

 

 

 

 

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

In applications where you have to use PC-Lint (so most probably in safety-critical applications) you always have to know the exact size of every variables in your project, and this way you can always apply the proper constant "all-one" value.

PCLint cannot and should not distinguish the case when you assign a signed value to an unsigned variable by mistake, from another case when you use this (otherwise cool) trick intentionally.

edited: typo

Last Edited: Sat. Jan 8, 2022 - 07:28 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Menahem wrote:
In applications where you have to use PC-Lint (so most probably in safety-critical applications) ...
Any application; linters can be nearly ubiquitous.

fyi, PC-lint is legacy ... the ones at Microchip Technology added a linter plug-in to MPLAB X v6.00

 

Adding Automatic Debugging to Firmware for Embedded Systems by Jack Ganssle

[near bottom]

... (you do use Lint, don't you? It's an essential part of any development environment) ...

Software Safety: The death of Gimpel Lint. Do you read your license agreements?

Come Join Us (MPLAB Now Supports AVRs) | Page 8 | AVR Freaks

 

"Dare to be naïve." - Buckminster Fuller

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

While MISRA advises against typecasts,  they can be useful both to the compiler and your code maintenance engineer to know exactly what your intentions were. 

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

Wow.

IIRC there was a license agreement like that on Legends of Tomorrow.

Moderation in all things. -- ancient proverb

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

There was a manual for I think one of the early C compilers that included a page saying send in this page for a $500 payment... nobody did.

 

Neil

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


Ha ha!  Reminds me of this - currently doing the rounds on the twitter:

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

barnacle wrote:
There was a manual for I think one of the early C compilers that included a page saying send in this page for a $500 payment... nobody did.
A company did somewhat the same thing with its bills.

Management objected to some state-mandated enclosures and wanted to demonstrate that nobody read them.

 

A guy at Edward Jones told me I was the first person to read the agreement before signing it.

I believe he meant the first of his clients.

Moderation in all things. -- ancient proverb

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

That is one reason I so hate the poke-it-and-see-what-it-does method of figuring out software.

I'm currently going through that with gappa.

Moderation in all things. -- ancient proverb