Pages: 1 2 [3]   Go Down
Author Topic: String corrupts the heap and crashes  (Read 9128 times)
0 Members and 1 Guest are viewing this topic.
0
Offline Offline
Edison Member
*
Karma: 44
Posts: 1481
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This malloc has problems for schedulers and RTOSes.

http://arduino.cc/forum/index.php/topic,138392.0.html
Logged

Offline Offline
Full Member
***
Karma: 3
Posts: 104
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Paul Stoffhegan made  a post a while back about patches he made to the String library to fix these problems that were never adopted by the 'team'.  Perhaps he would be willing to share those changes?

The original contribution I tried to make is still on Arduino's issue tracker.  It was a complete rewrite of String which I created for Teensyduino.  It had the fixes to the memory allocator.  I didn't want anyone using a Teensy board to suffer a buggy String library.

Unfortunately, the Arduino Team didn't use all of my code.  They made many changes, some small, but some that really hurt efficiency.  They also didn't use my fixes for the memory allocator, which is probably why this crashes on Arduino.  They ignored the special compiler options, despite numerous messages I wrote on the developer mail list to explain what a tremendous improvement they provide.

If you want my latest code, it's available as a free download from my website.  Here's the exact link:

http://www.pjrc.com/teensy/td_download.html

Just run the installer, and then grab the files from hardware/teensy/cores/teensy.  You don't need to buy a Teensy board..... but of course I would prefer if you do.  Sales of Teensy are what's what funds all my work and the many contributions I (try to) make back to Arduino.

I've been running the example code on a Teensy board for the last 30 minutes without any problems.  It's reached "Done" many times.  I changed the delay to only 5ms, so it runs the entire test quickly.  This bug absolutely does not happen with my code in Teensyduino.

The installer patches your Arduino IDE, but I'm very careful to never change behavior for non-Teensy boards.  The modified compiler settings and other stuff are only used when you upload to a Teensy board.  The source code for those changes is installed to a "src" directory within your arduino directory.

Quote
Sure, we know about fragmentation, but an egregious bug is something else.

Indeed.  I completely rewrote String in Fall/Winter 2010.  While developing this, I used lots of code to log all malloc/realloc/free usage.  I spent MANY long hours carefully analyzing and tweaking so realloc would be used to best effect (depends on a couple compiler options, which they never accepted into Compiler.java).  I found and fixed these terrible memory allocator bugs, almost TWO YEARS AGO.

They simply didn't want to use my code as-is.  They decided to accept it piece-wise, making changes.  Some parts, like the special compiler option to elide constructors (saves you from all sorts of memory fragmentation by avoiding unnecessary copies) were never used.  That's really a shame, and one that really sours my attitude about contributing to arduino, because I spent so many long hours carefully studying disassembly of the generated contructor/destrutor code and very long logs of every malloc/realloc/free operation for so many test cases.

They never used the memory allocator fix (which was more-or-less just code I lifted from a newer version of avr-libc).  That's probably why this code crashes on Arduino.  The result is a bad combination of inefficient runtime performance, which only hastens the inevitable crash due to the allocator bugs.  It's really very sad, when I put so much work into avoiding those problems.

I'm still very unhappy at how poorly String turned out for all non-Teensy users.  I tried to contribute a good String implementation back to all Arduino users.  Really, I tried.  I wrote lots of messages explaining the issues.  Much discussion was held.  In particular, Alan Burlison was very unhappy with me and my code (he had planned to do a String rewrite, but never did).  Much heated discussion occurred.  Nobody seemed to appreciate my effort.  The entire process of trying to contribute this back to Arduino was incredibly difficult and painful.  The code sat unused for many months (but of course I shipped it for Teensy boards).  When it finally was partially used, all my suggestions were pretty much ignored.

I fixed all these problems, and if you use a Teensy board you'll get my code as it should be.  If you have a Teensy board, please try running any problematic String examples.  I'm sure you'll see it works quite well (unless you run out of memory, in which case you get empty strings but never a crash).

There's just nothing I can do about Arduino's buggy code.  I really, really did try.  Sorry.




wanted to verify that we only need to copy the "WString.cpp" and "WString.h" files
Logged

New Jersey
Offline Offline
Sr. Member
****
Karma: 1
Posts: 482
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This example, which crashes before printing "Done", demonstrates a serious bug in String.
Code:
#include <MemoryFree.h>
void setup() {
  Serial.begin(9600);
}
void loop() {
  int n = 0;
  while(n <=100) {
    int u = 20*n;
    int v = 30*n;
    String str = String(n) + ", " + String(u) + ", " + String(v) + ", " + String(freeMemory());
    Serial.println(str);
    delay(500);
    n++;
  }
  Serial.println("Done"); 
}
The output looks like this and the value of freeMemory() in the last column indicates a corrupt heap.
Quote
0, 0, 0, 1740
1, 20, 30, 1738
2, 40, 60, 13048
3, 60, 90, 24358
4, 80, 120, -29870
5, 100, 150, -17536
6, 120, 180, -3921
7, 140, 210, 10462
8, 160, 240, 23054
9, 180, 270, -29122
-----crash and restart here---

The bug is from a larger program here http://arduino.cc/forum/index.php/topic,115451.0.html

I just tried the above code with Arduino IDE 1.0.4 on my Uno and it works now. 
Logged

0
Offline Offline
God Member
*****
Karma: 24
Posts: 587
Always making something...
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Yes.  1.0.4 includes the malloc/realloc/free fix.
Logged

Pages: 1 2 [3]   Go Up
Jump to: