Serious bug in multiplication - mikroC PRO for AVR 2011 4.60

[ajsn]

I'm trying to use a.m. compiler for AVR and found serious mistake in multiplication. I made simple test code for demonstration:

unsigned long adc_print (unsigned int adc_int, char adc_k)
{
char volt_i, volt_f;
unsigned long adc_t = 0x0;
adc_t = adc_k * adc_int;
return (adc_t--);
}

int adc1=0x3FF;
char adc2=0xFF;
unsigned long adc3 = 0x0;

void main()
{
adc3 = adc1 * adc2;
adc1--;
adc2++;
adc1++;
adc2--;
adc3 = adc_print (adc1, adc2);
}


In both cases (main and subroutine) the result of multiplication is the same - '*' working with first two bytes part only (int format). Everyone can test the multiplication yourself. By the way, there are no special function mikroC PRO libraries for those operation.

So there are 2 questions:
- how and who can correct compiler (I believe to write special subroutine myself for multiply of two long will be very unusable and comical taken into account mikroC PRO compiler exist)?
- is someone found same fundamental bugs in the compiler?

Thanks in advance.

[secure]

Hi,

There's nothing wrong with the compiler.

There's a big difference between a signed and an unsigned char.

You might think the value of adc2 = 255. But since it's a signed char the value is not what you really expect it to be unless you've already take it under consideration.
If you change adc2 to an unsigned char then you would probably get the expected result you were looking for.

Let me know if that works for you.

[filip]

Hi,

From the Standard Conversion chapter of the Help file :

When using arithmetic expression, such as a + b, where a and b are of different arithmetic types, the mikroC PRO for AVR performs implicit type conversions before the expression is evaluated.
These standard conversions include promotions of “lower” types to “higher” types in the interests.

As I can see from your piece of code, you are multiplying adc1 (which is of int type) with adc2 (which is of char type), and the multiplication result exceeds the int limits, so you must use typecast operator to get the correct result, like this :

Code: Select all

adc3 = (unsigned long)adc1 * adc2;

Please, read the Standard Conversion and Explicit Typecasting chapters in the Help file for more details on this matter.

Regards,
Filip.

[ajsn]

Hi,
Please, read the Standard Conversion and Explicit Typecasting chapters in the Help file for more details on this matter.

Thank you very much for explanation. The code working exactly.
My sincerely apologizes!
Of course,

Code: Select all

adc3 = (unsigned long)adc1 * adc2;

is working exactly.

By the way,
in Standard Conversion is wrote:
These standard conversions include promotions of “lower” types to “higher” types in the interests of accuracy and consistency.

So I belive the result in the code

Code: Select all

int adc1=0x3FF;
char adc2=0xFF;
unsigned long adc3 = 0x0;
...
adc3 = adc1 * adc2;

will be convert to unsigned long

[ajsn]

The same "problem".
Not working correct operator:

Code: Select all

signed int  aa;
...
if (aa < 0xFF60) aa = 0xFF60;

and working correctly:

Code: Select all

signed int  aa;
...
if (aa < (signed int) 0xFF60) aa = 0xFF60;

The compiler is not take into account definition of variable 'aa'.
I think is more right (as in previous "Serious bug in multiplication" sample) to consider all parts of operator, not operands only, but the type of result also.
It will be fully conform to Standard Conversion issue.

[one25]

Hi all,

I have a similar problem.
I receive a string from USART which finishes in \n \r, so after every \r, i reset a counter and I fill an array with next string.
After this, I have to convert string in a number, so I have for example space, space, digit, digit, digit, decimal point, digit, digit, space.
I read this array and I multiply for a coefficient every digit (by 10000, by 1000, 100, 10, 1 respectively) and after, I eventually divide by 100.

Code: Select all

unsigned long multiply_coeff[9] = {0, 0, 10000, 1000, 100, 0, 10, 1, 0}; // also tried unsigned int
unsigned short weight_reading[9];

//.............

void buffer_reading() {
                      short l;
                      unsigned long new_weight = 0;

                      if (weight_reading[0] != 0x20) {
                                                     weight_error = 1;
                                                     }
                         else {
                              for (l=0;l<9;l++) {
                                                  if ((weight_reading[l]>47)&&(weight_reading[l]<58)) {      // if is a character from "0" to "9"
                                                                              new_weight = new_weight + (weight_reading[l]-48)*multiply_coeff[l];
                                                                                                      }
                                                 }
                              weight = new_weight;
                              weight_error = 0;
                              }

At the end, I should have the variable weight refreshed, but I am missing something on the multiplying.

Where I am wrong?

[one25]

Meanwhile I got this function working...

Code: Select all

new_weight = new_weight + ((unsigned long)(weight_reading[k]-0x30))*multiply_coeff[k];