Compiler optimization Bug or is it me

Have verified this code works on all of the non AVR boards I have ESP32 and ESP8266 8 diferent varieties

Arduino UNO WiFi Rev 2 Totally different result will produce expected result regardless of code.

The code does not run correctly the following boards unless there are two calls to inc_Buffer & inc_low in the sketch
Arduino UNO
Arduino Mega2560
Elegoo Mega2560
Generic Mega2560
Generic UNO 3 diferent
Generic NANO
So I asume all 8 bit AVR board but these are the only units I have on hand

Can someone who works with C++ run thing on non Arduino IDE to see if it compiles and runs?

char *Buffer[] = {
// 0123456789  
  "ack 100099",
  "nak ======",
  "cmd ======",
  "sta ======",  
};
void setup() {
  Serial.begin(38400);
  Serial.println("ASCII Buffer Inc test");
//                Comment out following line inc_Buffer does not work ******************
  inc_Buffer(Buffer[0], 9, 3);          // Should rollover to 100
  Serial.println(Buffer[0]);
}
  uint32_t tmp = 0;
  
void loop() {
  inc_Buffer(Buffer[0], 9, 1);         // Just inc the least sig byte
 if(tmp++ < 15) { Serial.println(Buffer[0]); } // Just do 15
}
void inc_Buffer(char p_Buf[], uint8_t pos, uint8_t len){
    while (inc_low(p_Buf, pos--) && --len ); 
    inc_low(p_Buf, 4); // This is required to get inc_low to work **********************
                       // for some reason two calls to inc_Buffer and inc_low in
                       // sketch are required for routine to work
}    
bool inc_low (char p_Buf[], uint8_t pos){ // true = overflow
    if (++p_Buf[pos] < 58) {    // Get and inc 9 out of 10 times nothing else needed
      return false;  }          // indicate were done = no overflow
    p_Buf[pos] = 48;            // take care of over flow
    return true;                // indicate overflow
}

This is the expected output

Buffer Inc test
========10
========11
========12
========13
========14
========15
========16
========17
========18
========19
========20
========21
========22
========23
========24
========25

This is output with setup inc_Buffer commented out

Buffer Inc test
==========
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10
========10

For reference I cant say 100% but 99% as long as there are two or more calls to inc_Buffer in the compiled program it produces desired output which is why I think it may have to do with optimization.
Having just the main loop call and a call in a unused function does not produce the desired output.
And again I am at a total loss as to why comment out incrementing the rollover digit stops the inc of the primary digit making me think I am somehow misusing the by ref aspect of the function call.

it looks like you defined an array of char ptrs instead of an array of char and
you only need to pass the array not Buffer [0]

//                 0123456789
char *Buffer[] = {"=========="};

void setup () {
    Serial.begin (38400);
    Serial.println ("Buffer Inc test");
    ///*            Comment out following line inc_Buffer does not work
    inc_Buffer (Buffer[0],9);
    //*/
    Serial.println (Buffer[0]);
}

void loop () {
    inc_Buffer (Buffer[0],9);                       // Inc Pos 9 & 8 by 1 Asci
    Serial.println (Buffer[0]);
    delay (100);
}

void inc_Buffer (char p_Buf[], uint8_t pos){
    uint8_t i = ++p_Buf[pos]; // Get the Value
    if (i > 57) {
        p_Buf[pos] = '0';
        ///*  Comment out remaing lines single digit inc does not work
        i = ++p_Buf[pos-1];
        if (i > 57) {
            p_Buf[pos-1] = '1';
        }

        //*/
    }

}

If you want a pointer to an indexed array element, you have to use the address operator:

&Buffer[3]

aarg:
If you want a pointer to an indexed array element, you have to use the address operator:

&Buffer[3]

Pointer arithmetic is more concise:

Buffer + 3

Perhaps you meant something like:

//               0123456789
char Buffer[] = "==========";


void setup()
{
  Serial.begin(115200);
  Serial.println("Buffer Inc test");
  inc_Buffer(Buffer);
  Serial.println(Buffer);
}


void loop()
{
  inc_Buffer(Buffer);                      // Inc Pos 9 & 8 by 1 Asci
  Serial.println(Buffer);
  delay (100);
}


void inc_digit(char *buffer, size_t index)
{
  if (buffer[index] < '0' || buffer[index] > '9')
    buffer[index] = '0';
    
  buffer[index]++;
  
  if (buffer[index] > '9')
  {
    buffer[index] = '0';
    inc_digit(buffer, index-1);
  }
}


void inc_Buffer(char *buffer)
{
  inc_digit(buffer, strlen(buffer) - 1);
}

Output:

=========1
=========2
=========3
=========4
=========5
=========6
=========7
=========8
=========9
========10
========11
========12
========13
========14
========15

First this is a bug on my install of the Compiler as this exact code compiles and runs with expected results on a "WeMos D1 R1" (my target micro) regardless of what I comment out. Other possibility is that it is a "FEATURE" of the WeMos compiler.

Please can someone copy and run the code on a NANO(Old bootloader) IDE 1.8.13 and verify a positive result then comment out the setup call to inc_Buffer and see if resultant output is as expected. Running it on a new NANO would also be helpful to isolate. I will test and post which boards this does and does not work on.

In response to the anticipated question if it runs on my target micro why do I care?
Much faster to compile and burn on NANO than WeMos and 50 years of coding tells me debug and test small blocks of code not programs.

Thanks for the feedback but I am more interested in why this code works when there are two active calls.
Yes I wish to declare a array of pointers as I said it is part of a larger program I just reduced it to the smallest possible lines of code to debug the problem. Again the code works fine as long a I have two calls to inc_Buffer() in the sketch. I have included the actual routine again part of much larger program.

Also note my next step is to put in a while to allow variable number of digits.

char *Buffer[] = {
// 0123456789  
  "ack 100099",
  "nak ======",
  "cmd ======",
  "sta ======",  
};
void setup() {
  Serial.begin(38400);
  Serial.println("ASCII Buffer Inc test");
//                Comment out following line inc_Buffer does not work ******************
  inc_Buffer(Buffer[0], 9, 3);          // Should rollover to 100
  Serial.println(Buffer[0]);
}
  uint32_t tmp = 0;
  
void loop() {
  inc_Buffer(Buffer[0], 9, 1);         // Just inc the least sig byte
 if(tmp++ < 15) { Serial.println(Buffer[0]); } // Just do 15
}
void inc_Buffer(char p_Buf[], uint8_t pos, uint8_t len){
    while (inc_low(p_Buf, pos--) && --len ); 
    inc_low(p_Buf, 4); // This is required to get inc_low to work **********************
                       // for some reason two calls to inc_Buffer and inc_low in
                       // sketch are required for routine to work
}    
bool inc_low (char p_Buf[], uint8_t pos){ // true = overflow
    if (++p_Buf[pos] < 58) {    // Get and inc 9 out of 10 times nothing else needed
      return false;  }          // indicate were done = no overflow
    p_Buf[pos] = 48;            // take care of over flow
    return true;                // indicate overflow
}

As far as the actual Inc I selected the method to speed up the process as one of my UDP application modules is doing waveform analysis and needs all the speed and memory i can get.

Why does this always happen to me this code should have taken 15 min to write test and debug I am at almost 20 hours and to say the least frustrated. I wrote and debugged the main programs five hundred or so lines of code last week in less time than it has take me to white a lousy ASCII inc routine.

Neither of these makes much sense to me. What pattern of numbers did you want?

Arduino UNO with the line in setup() NOT commented:

ASCII Buffer Inc test
ack 200100
ack 300101
ack 400102
ack 500103
ack 600104
ack 700105
ack 800106
ack 900107
ack 000108
ack 100109
ack 200100

Arduino UNO with the line in setup() commented out:

ASCII Buffer Inc test
ack 100099
ack 200090
ack 300091
ack 400092
ack 500093
ack 600094
ack 700095
ack 800096
ack 900097
ack 000098
ack 100099
ack 200090
ack 300091

jrpjim001:
First this is a bug on my install of the Compiler

Many people claim that the strange behavior they're seeing is caused by a compiler bug. Very few of them turn out to be right.

What you're seeing is almost definitely the result of undefined behavior in your code. Undefined behavior means that your code is incorrect, so the compiler is not required to output a valid program.

The main source of undefined behavior in your program is probably writing to string literals, but there may be others.
Please enable your compiler warnings, I'm pretty sure it'll warn you about that: The strings in your buffer are string literals, which are read-only. You're casting away const, and then writing to them, which is not allowed.

If you call your function once or multiple times, the compiler might decide to inline it, or not, so that could cause different outcomes when the code invokes UB.

Pieter

Edit: As expected, the compiler does warn about it:

warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]

Posted by johnwasser - Today at 07:46 pmQuote
Neither of these makes much sense to me. What pattern of numbers did you want?

Thanks for the help
Both are the anticipated output the inc_Buffer in setup changes the 99 to 100 because it is set to do 3 digit the inc buffer in loop just inc the least sig digit.
If you change the inc_Buffer in Loop it may make more sence

inc_Buffer(Buffer[0], 9, 1); // Just inc the least sig byte

to
inc_Buffer(Buffer[0], 9, 2); // Just inc the last 2 bytes will give you 0-99 inc
or
inc_Buffer(Buffer[0], 9, 3); // Just inc the last 2 bytes will give you 0-999 inc

What board and IDE are you running. I just tested all my other boards and all the avr boards give bad results all the ESP boards work fine.
(Resolved sorry) New to forum is there a way to reply to your reply vrs the way I am replying

jrpjim001:
New to forum is there a way to reply to your reply vrs the way I am replying

There are forum usage guides in the sticky threads at the top of the forum.

The compiler does appear to have a problem with you modifing what it considers to be a const value. If you change the definition of the strings to the follow the output is different:

char text0[] = "ack 100099";
char text1[] = "nak ======";
char text2[] = "cmd ======";
char text3[] = "sta ======";
char *Buffer[] = {
  text0,
  text1,
  text2,
  text3,
};

Note that you will get the following warning message with your code if you have all compiler warnings turned on, because the compiler wants the definition to be const char *

warning: ISO C++ forbids converting a string constant to 'char*' [-Wwrite-strings]

I tested your original code on a Nano Every, and the output never changes, because the const values are automatically stored in flash memory on the atmega4809 and cannot be altered at run-time.

PieterP:
Many people claim that the strange behavior they're seeing is caused by a compiler bug. Very few of them turn out to be right.

To be fair I'm a old dog 55 years ASM programming having problems learning new tricks because I am new to C++. From my point of view if what I type once works it should always work regardless of the machine and I know that's what the compiler does problem is I dont know how to tel it to compile statically. So I apologize when I call a inconsistency a bug maybe I should say a Compiler "Bug me". I realize it did what I told it I just have to figure out how to gt it to do what I want.

To me "0",'0' and 48, &H30, b0011-000 are all inter changeable and the default should be all variables are stored in ram and once the address of the ram block is known it should never be changed so unless I tell the compiler otherwise to compile for heap/stack garbage collection.
char Buffer[1] = '12345' at compile time should allocate 5 or 6(nul) bytes of SRAM not heap or stack allocated.

david_2018:

char text0[] = "ack 100099";

char text1[] = "nak ======";
char text2[] = "cmd ======";
char text3[] = "sta ======";
char *Buffer[] = {
  text0,
  text1,
  text2,
  text3,
};

Thanks this does seem to solve the problem I am not sure that it is going to solve in the long run because I added 10 character to all the Buffer's and it only added 6 to my Variable usage. Then I added two extra Buffers and variable usage was unchanged but program usage when up by 6. Translate my comment since it did not consume SRAM not sure it is always going to be in a known place to be able to be modified.

The compiler "Bug_me" is why do we have to do the static allocation in two steps there is plenty of processing power to do the conversion obviously the compiler is capable because it did it and even warned us about it why not add some feature to add the feature to allow 8 byte string to character conversion.

jrpjim001:
Thanks this does seem to solve the problem I am not sure that it is going to solve in the long run because I added 10 character to all the Buffer's and it only added 6 to my Variable usage. Then I added two extra Buffers and variable usage was unchanged but program usage when up by 6. Translate my comment since it did not consume SRAM not sure it is always going to be in a known place to be able to be modified.

You have to be careful when checking memory usage, particularly on small test sketches. The compiler is very good at optimizing away things that it determines are unused. Since your test code only uses Buffer[0], the compiler can eliminate the remainder of the array and unused strings completely, If you use a variable for the array index, and set it to a random number at the start of the code, then you will see a substantial increase in memory usage because the compiler needs to keep the entire array.

jrpjim001:
From my point of view if what I type once works it should always work regardless of the machine and I know that's what the compiler does problem is I dont know how to tel it to compile statically. So I apologize when I call a inconsistency a bug maybe I should say a Compiler "Bug me".

You should say "I'm doing something that results in undefined behavior per the language specification, and yet (for some reason) I'm surprised that the resultant undefined behavior is different across different platforms".

gfvalvo:
You should say "I'm doing something that results in undefined behavior per the language specification, and yet (for some reason) I'm surprised that the resultant undefined behavior is different across different platforms".

You must be in politics! But good view point.

But you do bring up a point does the C++ language specification cover optimization because the more I learn on this issue its a optimization that was causing the variation of behavior.

david_2018:
If you use a variable for the array index, and set it to a random number at the start of the code, then you will see a substantial increase in memory usage because the compiler needs to keep the entire array.

Thanks again that did the trick I now have faith in SRAM available and therefor faith in direct Buffer modification.

Now I am being lazy with this question as I know I can learn the answer My code needs to handle two level deep interrupts(No call only inline) two levels of time slice (threads) with 1-16bit param and 1-8 bit return each and 3 levels of procedures averaging 3-16 bit param and 1 8bit return any one for a best guess of amount of local variables capacity I need to leave.

Note compiler will not be able to ascertain function nesting depth due to the use of callback/dispatch arrays in the time slice and PLC portions of the code.

jrpjim001:
But you do bring up a point does the C++ language specification cover optimization because the more I learn on this issue its a optimization that was causing the variation of behavior.

It has nothing to do with optimization. You tried to modify the value of a string literal using a pointer. That's not allowed. Any variation of behavior that you see while attempting an illegal operation is on you.

gfvalvo:
It has nothing to do with optimization. You tried to modify the value of a string literal using a pointer. That's not allowed. Any variation of behavior that you see while attempting an illegal operation is on you.

You comment takes me back to being confused my desire is.
Create Array of 4 pointers stored in contiguous location of known size
Create 4 Arrays of 25 bytes each of contiguous data

Modify a specific Buffer's specific byte.

my use of a string literal was only to pre initialize instead of filling it at run time.

If you are or have any ASM experience this is what I need. Example is 8 bytes but you get the gist.

RSEG: ORIG &h100

ptr0: DB 48,48,48,48,48,48,48,00
ptr1: DB 48,48,48,48,48,48,48,00
ptr2: DB 48,48,48,48,48,48,48,00
ptr3: DB 48,48,48,48,48,48,48,00

Buffer: DW ptr0, ptr1, ptr2, ptr3

Reminder my C++ experience is limited to about a week vrs 55 years ASM

The way to do that in C is:

byte ptr0[] = {48,48,48,48,48,48,48,00};
byte ptr1[] = {48,48,48,48,48,48,48,00};
byte ptr2[] = {48,48,48,48,48,48,48,00};
byte ptr3[] = {48,48,48,48,48,48,48,00};

byte *Buffer[] = {ptr0, ptr1, ptr2, ptr3};

You aren't guaranteed that the arrays are consecutive in memory. If that is required:

byte data = {
48,48,48,48,48,48,48,00, 
48,48,48,48,48,48,48,00, 
48,48,48,48,48,48,48,00,
48,48,48,48,48,48,48,00};

byte *Buffer[] = {data, &data[8],  &data[16],  &data[24]};

Or unsigned char?

Do you specifically need the additional array of pointers? I'd just use a 2-dimension array, the compiler will treat Buffer[ x ] as a pointer in the function call.

char Buffer[4][25] = {
// 0123456789012345678901234
  "ack 100099              ",
  "nak ======              ",
  "cmd ======              ",
  "sta ======              "
};