It's difficult to pin down the exact details as to why the problem it's manifesting exactly the way it is. But, in short, it's fair to say you're running out of memory.
First, just a note: The code I'm testing with (because it compiles) is the code out of your github repo, which used a char
type variable for linesInserted
and had a commented-out println(linesInserted);
, so it kind of seems like at one point you'd been trying to print the count as character, specifically non-printable control characters. So that might may have been confusing you. I uncommented it and added a , DEC)
parameter to it (rather than casting) to coerce println into treating it as an int.
I tested your code without the display library in a PC environment to allow me to run valgrind on it. It didn't find anything; I wasn't necessarily expecting it to. But, I did crudely instrument your code in this enviroment to see that you're calling malloc()
/new
124 times, for a total of 500 bytes, but importantly this does not include the structure heap overhead, which is significant because you're new
-ing tiny four-character chunks.
Then I ran your code as compiled for an AVR. Because I don't have your display, and wanted to match your environment as closely as possible, I left the display code in but NOPed out a few function calls that would have screwed up my test rig.
The primary finding is that you're dangerously close to "running out" of memory:
000005e0 | 01 00 02 04 00 01 00 02 - 00 04 00 03 01 03 05 04 | ........ ........
000005f0 | 00 01 03 02 03 04 00 01 - 06 02 06 04 00 00 00 00 | ........ ........
00000600 | 00 04 00 02 00 02 00 04 - 00 01 01 01 01 04 00 00 | ........ ........
00000610 | 02 00 02 04 00 02 02 02 - 02 04 00 01 03 01 03 04 | ........ ........
00000620 | 00 00 04 00 04 04 00 02 - 04 02 04 04 00 01 05 01 | ........ ........
00000630 | 05 04 00 00 06 00 06 04 - 00 02 06 02 06 00 00 00 | ........ ........
^^
[__brkval]
[SP]
VV
000006b0 | 00 00 00 00 00 00 b7 09 - 03 01 01 06 02 a5 02 00 | ........ ........
000006c0 | 00 00 00 01 00 57 05 06 - cc 00 03 8f 0f 01 00 00 | .....W.. ........
000006d0 | 00 01 00 00 03 00 03 00 - 00 00 07 02 a4 00 01 00 | ........ ........
000006e0 | 03 02 a4 00 05 f8 0d 06 - f6 00 03 00 00 08 c3 08 | ........ ........
000006f0 | de 00 01 00 01 80 1a ed - 02 04 00 41 a4 02 04 00 | ........ ...A....
There's 120 byte separation between the edge of the heap and the stack pointer. The implementation of the malloc
that I ran this under checks to see whether or the the stack pointer is within 128
(__malloc_margin
) bytes. As your code was building in my environment, without the counter-printing in, this managed to not-fail to allocate. However adding a single additional call to addNewLine
was enough to cause malloc()
via new
to fail. So you are indeed right on the edge. And you have no checks to see if malloc
or new
returning nullptr
.
You're making one allocation after another without deallocation anything, which is resulting in a fairly simple pattern of the heap edge advancing by the requested size of your allocation plus 2 bytes each time. So that's about 248 bytes waste in just heap structure.
I'm not sure on the exact cause of your counter being overwritten, assuming that's what's actually happening. In fact I was having a hard time finding an case where it would overwrite anything in memory with the code as you have it, but here's a good candidate:
lines[linesInserted - 1] = new char[4]{startX, startY, endX, endY};
With generated code looking like:
0x24c6 [addNewLine+244] ldi r25, 0x00 ; 0
0x24c8 [addNewLine+246] call 0x3830 ; 0x3830 [malloc]
0x24cc [addNewLine+250] movw r30, r24
0x24ce [addNewLine+252] st Z, r10
0x24d0 [addNewLine+254] std Z+1, r11 ; 0x01
0x24d2 [addNewLine+256] std Z+2, r17 ; 0x02
0x24d4 [addNewLine+258] std Z+3, r16 ; 0x03
So, if malloc()
fails, apparently the new
statement will just assume that it didn't and goes ahead and writes into the memory at 0x0000 (nullptr) anyway, ugh. I think this is because technically you're calling the normal new that is supposed to throw an exception on failure. But there are no exceptions with the way avr-g++ is set up for Arduino. As it exists in ArduinoCore-avr version 1.8.3, the new code just returns what malloc returns and the compiler thinks an exception will be in progress if it fails. Normally you would call the std::nothrow version of new[] that returns nullptr on failure, as in:
lines[linesInserted - 1] = new(std::nothrow) char[4]{startX, startY, endX, endY};
// check lines[linesInserted - 1] against nullptr here
However, there's no facility to do that in 1.8.3. The master as of writing appears to have addressed this, but that's not shipping yet.
The short answer to this is basically: Don't do that. That is:
Avoid dynamic allocation altogether where possible.
This was mentioned in the comments, but it's worth repeating, use flash where you can. Besides getting your character data into PROGMEM
, you also have some string literals that can be moved into flash by way of F()
.
To the extent that you're doing dynamic allocation requests:
Check to see that they're actually succeeding
Make as few requests as possible.
Do them up-front at the beginning of the code. The "up-front" part you're more or less already doing, so that much is good.
It seems if you're going to dynamically allocate under core version 1.8.3 and you're at all in danger of having an allocation failure, it is important that you not use new, since doing so may cause it to overwrite memory for the initialization/construction part after the allocation proper has failed. Alternately, seriously consider modifying your Arduino core's new.cpp file to do something like call abort()
if malloc()
returns nullptr
, at a minimum, if not something more sensible. Otherwise, this could drive you insane.
char** lines;
ever initialized?lines
to(char**) malloc(sizeof(char*) * Char.sizeOfLines);
. I'm in the process of converting this from astruct
to a class so I haven't gotten around to creating a constructor yet.pgm_read_*
functions to access the data. No need for any dynamic RAM allocation. No limit on the number of lines in a symbol (that count could be static and part of the symbol data). No messing with malloc.