avr-libc 1.6.4 dynamic memory (malloc, free) bug

[GoogleCodeExporter]

Arduino 1.0 uses WinAVR-20081205 that comes with avr-libc 1.6.4.

There is a documented bug on avr-libc (#27235, #28135) related with malloc() 
and free() that affects the current avr-libc version used by arduino; 
WinAVR-20100110's avr-libc 1.6.7 is still affected. Proven not to be affected 
is Atmel's native libc from AVR Studio 5.1.208 and wiring's avr-libc 1.7.1.

The solution would be to update arduino's avr-libc from 1.6.4 to the latest 1.7 
branch.

Attached is a program to prove the bug existence, when run it outputs 
used/free/large after each allocation call, then repeats the test when doing 
de-allocation. The expected behavior is at the end of the program the amount of 
used/free/large to be the same as at the start.. which is not the case using 
arduino's avr-libc 1.6.4. The original code comes from Andy Brown 
(http://j.mp/yGnTwT) I have just ported it to compile under Arduino's IDE.

Links:
- bug #27235: malloc: Several things go wrong (http://j.mp/xq8Xk8)
- bug #28135: malloc(): expand the last free chunk when expanding __brkval 
(http://j.mp/zQfOWN)

Forum topic which lead to this bug report:
- malloc(), realloc().. the dark side powerful it is (http://j.mp/y3YKF6)

Note: Nick Gammon mentioned that since Strings uses malloc() and free() this 
could be the explanation for some people complaining about random program crash 
when using the library.

Original issue reported on code.google.com by jbrazio on 12 Mar 2012 at 1:31

Attachments:

• malloc_test.zip

[GoogleCodeExporter]

Just spent the guts of 4 days trying to chase down a problem with system 
resets. Was guided to look at Strings. I found a corruption of the freelist 
created by using a simple Sting function. Now it appears that the problem may 
be even deeper - not with Strings per se but with the deallocation of memory in 
general!. Its so fundamental.

1 Please bump up the priority on this.
2 Please put a severe Government health warning on any use of Strings on the 
Arduino main reference pages, or remove Strings completely from the reference - 
there must be hundreds of people being sucked into using them by advertising 
Strings there. If this is not your territory, please point me to the publisher 
of the Reference.

Thanks

Original comment by ke...@devons.orangehome.co.uk on 27 Jun 2012 at 10:55

[GoogleCodeExporter]

I think we should try grabbing the malloc.c from the Teensy code base and 
seeing if that helps.  It should be a version of the 1.7.0 malloc() modified 
to, I think, compile from a user application (rather than inside the avr-libc 
codebase).

Original comment by dmel...@gmail.com on 29 Jun 2012 at 3:51

[GoogleCodeExporter]

You can grab the malloc.c from Teensyduino.  It's open source.  Just run the 
installer and then help yourself to the code, which you'll find in 
hardware/teensy/cores/teensy.

If a fixed malloc/realloc/free is all you need, the exact same code I've been 
using in Teensyduino for the last 2 years can also be found on issue #468.  
Just copy that malloc.c to hardware/arduino/cores/arduino and your String 
crashing problems will (probably) be solved.

There's also a tiny patch on issue #468, which was never applied (but it's been 
in Teensyduino for nearly 2 years and working very nicely).  The patch simply 
enables compiler optimizations to elide constructors and leverage new C++ move 
semantics, both of which greatly improve String's efficiency.

Original comment by paul.sto...@gmail.com on 26 Jul 2012 at 9:41

[GoogleCodeExporter]

This is getting embarrassing. Practically every day on the forum we get a 
complaint that a sketch crashes (which uses the supplied String class) or even 
today that analogRead() doesn't work. Except it turned out that the test sketch 
used String to display the results.

And every day, we say: "Don't use String, it has a bug".
Them: "What bug?"
Us: "In memory allocation."
Them: "How long has this been around?"
Us: "Ages."
Them: "Why isn't it fixed?"
Us: "Don't know."

The bug basically means a core supplied library, the String library, is 
virtually useless. It also impacts other things, such as anyone who tries to do 
malloc/free for other purposes, or use the Standard Template Library.

Please, upgrade this bug to high priority, and take action to fix it in the 
next release. Thank you very much.

Original comment by n...@gammon.com.au on 26 Jul 2012 at 11:59

[GoogleCodeExporter]

Could you please give more details how to temporarily fix it? I wrote some code 
that heavily relies on Strings and now got stacked by this bug :(

Original comment by dzii...@gmail.com on 27 Jul 2012 at 3:58

[GoogleCodeExporter]

So we have a known bug,

we have a known fix, 

what else has to happen to ge tthe bug fixed ?

Original comment by andrew6...@gmail.com on 27 Jul 2012 at 5:55

[GoogleCodeExporter]

This workaround seems to work:
http://www.arduino.cc/cgi-bin/yabb2/YaBB.pl?num=1294392281
Looking forward to have it fixed in next release :)

Original comment by dzii...@gmail.com on 27 Jul 2012 at 6:00

[GoogleCodeExporter]

Or......

1: go to issue #468
2: download malloc.c
3: copy malloc.c to arduino-1.0.1/hardware/arduino/cores/arduino
4: (optional) post a detailed message here about what sketches you tested and 
how this helped.

5: (really optional) manually copy of Compiler.diff changes into the Arduino 
IDE source code in Compiler.java, compile with JDK+ant, and run.  This will 
make String much more efficient.

Original comment by paul.sto...@gmail.com on 27 Jul 2012 at 6:08

[GoogleCodeExporter]

Original comment by dmel...@gmail.com on 31 Jul 2012 at 12:50

• Added labels: Component-Build, Component-Core, Milestone-1.0.2

[GoogleCodeExporter]

Federico, can you look into this one?  That is, find some examples of sketches 
that are crashing currently, try adding the malloc.c to the Arduino core 
folder, re-test the sketches, etc.  

Original comment by dmel...@gmail.com on 31 Jul 2012 at 12:56

[GoogleCodeExporter]

Original comment by dmel...@gmail.com on 5 Sep 2012 at 1:00

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

I tried with some sketches that makes use of strings allocate statically and 
dynamically but the free ram after every loop is always the same. I'm using the 
IDE 1.0.1

Can you please provide me a sketch that makes crash the Arduino?

Original comment by f.vanz...@gmail.com on 7 Sep 2012 at 5:26

[GoogleCodeExporter]

I got String to crash on the Mega with the code posted here: 
http://arduino.cc/forum/index.php/topic,121797.0.html, although apparently that 
same code does not crash on the Uno.  This crash is due to the memory 
allocation issue described.

Original comment by DireSpume on 15 Sep 2012 at 9:00

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

If you can't confirm the bug, you can't confirm that your changes fix it, 
either. Complaining won't help us update the bug, helping us test it will.

Original comment by dmel...@gmail.com on 16 Sep 2012 at 2:04

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

thanks paul, after some clueless hours I finally found this bug entry. I added 
malloc and my sketch using AndOSC Library was gone 
(http://neophob.com/2012/09/arduino-free-memory/).

Thanks!!

Original comment by michudr...@gmail.com on 17 Sep 2012 at 9:00

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I tried to create a test file to show the bug - and failed miserable.

However I tagged my project to show the error, you need the ArdOSC library 
(https://github.com/neophob/ArdOSC/tree/1.0) and the main application 
(https://github.com/neophob/SkyInvaders/tree/Arduino-Bug-857-Tag). You need an 
Ethernet Board, I tested it with an IBoard (duemillanove compatile). I hope 
that helps.

Original comment by michudr...@gmail.com on 20 Sep 2012 at 7:23

[GoogleCodeExporter]

When I rewrote String, I created numerous tests in the Aduino Test Suite.  
Several are based on Tom's examples which were having trouble running.  There's 
a huge "mega" test, which is also broken into 5 parts for compatibility with 
Uno's limited RAM.

I haven't touched String in a long time, but I'm pretty sure some of these 
tests expose this bug.  Right now I don't have time spend on this (I probably 
shouldn't even be writing this message....)  If anyone wants to give it a try, 
just download the latest Arduino Test Suite from github, copy it to your 
libraries directory, and try running the example sketches until you find 
something that fails or crashes.

https://github.com/arduino/Tests

Original comment by paul.sto...@gmail.com on 20 Sep 2012 at 7:34

[GoogleCodeExporter]

On a Uno, version 1.0.1 of the IDE this sketch crashes after about 3 iterations:

void setup() 
{
  Serial.begin(115200);
}
void loop() 
{
  int n = 0;
  while(n <= 100) {
    int u = 20*n;
    int v = 30*n;
    String str = String(n) + ", " + String(u) + ", " + String(v);
    Serial.println(str);
    delay(500);
    n++;
  }
  Serial.println("Done");  
}

Original comment by jainh...@gammon.com.au on 30 Sep 2012 at 1:30

[GoogleCodeExporter]

Version 1.0.2 still crashes after 3 iterations of the test code above with 
version 1.0.2.

Can we please fix the free() bug? Time after time on the forum people post code 
that mysteriously crashes, because they have used the String library.

Original comment by n...@gammon.com.au on 7 Nov 2012 at 6:27

[GoogleCodeExporter]

Yes, this is a serious SERIOUS bug that is wasting countless hours and days of 
peoples time when they unknowingly use the String class.  What gives?  I can't 
imagine there are many (or any) other bugs that are higher priority than this 
one.  Either remove the String class and warn against memory allocation in the 
Reference, or fix the bug!  Thanks!

Original comment by DireSpume on 7 Nov 2012 at 10:43

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I tried copying malloc.c from issue 468 into 
arduino-1.0.1/hardware/arduino/cores/arduino but I got errors telling me new 
and malloc have already been defined. Where is this definition?

Original comment by gabrield...@gmail.com on 5 Dec 2012 at 4:05

[GoogleCodeExporter]

Unfortunately i'm not that great C-specialist and ran into the same Issue 
mentioned by gabrield and all the others above.
I can't understand why absolutely nobody, not even one of these 
"Arduiono-C-Authoritys", is interested in this vexatious affair.

KNOCK-KNOCK-KNOCK ----- ANYBODY AROUND ???????????????????????????

Original comment by tombroe...@gmail.com on 14 Dec 2012 at 12:17

[GoogleCodeExporter]

When will this be addressed?  I just wasted 2 hours trying to work out the 
cause of a crashing issue and stumbled upon this malloc/free issue.  I can't 
believe that something so fundamentally broken got shipped!

*grumble* *grumble*

I'd take a look into it myself but I've got too many existing deadlines.

Original comment by dominicc...@gmail.com on 14 Dec 2012 at 12:58

[GoogleCodeExporter]

Just copy the malloc.c I provided on issue #468 to 
hardware/arduino/cores/arduino

So quick and easy, it shouldn't set your deadlines back more than a few seconds.

Original comment by paul.sto...@gmail.com on 14 Dec 2012 at 8:05

[GoogleCodeExporter]

Done. Thank you.

https://github.com/arduino/Arduino/commit/d457332664730fde14146649169b1fdfe22095
14

Please check the fix.

Original comment by c.mag...@bug.st on 17 Dec 2012 at 8:45

• Changed state: Fixed

[GoogleCodeExporter]

I can confirm this is still an issue with Arduino 1.0.3.  It doesnt affect me 
however since I use Atmel Studio 6 to build my code, which doesnt have the 
problem.
I can recommend this environment its so much better in many ways.
I think it doesnt exhibit the problem because it uses a newer toolchain: 
(AVR_8_bit_GNU_Toolchain_3.4.1_830  4.6.2), but thats just a guess.

Original comment by frank.e...@gmail.com on 2 Jan 2013 at 10:34

[GoogleCodeExporter]

The fix went into the github repository after Arduino 1.0.3 was published, so 
of course the problem is still present in version 1.0.3.

This bug was fixed in avr-libc version 1.7.0 or 1.7.1.  Newer versions of 
avr-gcc (eg, 4.6.2) are typically packaged with newer versions of avr-libc.  
It's avr-libc that matters for this bug, not the compiler itself.

While newer versions of the toolchain have this bug fixed, they also have 
several incompatible changes.  The lack of backwards compatibility creates 
quite a barrier to upgrading.  Newer isn't necessarily better.  Any 
improvements also come with the pain of breaking several Arduino libraries and 
sketches.  Perhaps the Arduno Team will make the decision to do that someday?  
It's their call.

If you're running on a newer toolchain and using Arduino libraries, your 
arguement for the Arduino Team to upgrage could be much more compelling if you 
made a detailed list of which libaries you've actually tested, on which boards, 
which which versions of avr-gcc, avr-libs, and binutils-avr.

Original comment by paul.sto...@gmail.com on 2 Jan 2013 at 10:56

[GoogleCodeExporter]

I experienced very similar problems as expressed above, however resolved it 
slightly differently. If the root cause is the same I am uncertain, but adding 
my experience just in case it helps someone else.

I'm using String objects and specifically indexOf() function a lot in my code. 
At one point the program started crashing when adding another indexOf() call. 
The odd part was the program was crashing at an earlier code point than the 
added call to the indexOf() function. These calls looked as follows:
inputString.indexOf('Date: ');

The problem was the single quotation marks, which Arduino sees as char 
definition, but defining multi-character within. This I only learned when 
turning on verbose output and compiler warnings pointed it out. When I changed 
the code to:
inputString.indexOf("Date: ");

suddenly the program flew through perfectly. 

Original comment by corneill...@gmail.com on 8 Jan 2013 at 11:47

[GoogleCodeExporter]

I'm experiencing a strange issue with this fix on 1.0.4 (the problem exists 
also in 1.5.2 I guess, but not tested yet):

compiling any example of the SdFat library gives the following error:

/home/megabug/Software/arduino-1.0.4/hardware/tools/avr/bin/../lib/gcc/avr/4.3.2
/../../../avr/lib/avr51/libc.a(malloc.o): In function `malloc':
/home/paul/avr-libc-1.6.4/avr/lib/avr51/../../../libc/stdlib/malloc.c:78: 
multiple definition of `malloc'
core.a(malloc.c.o):/home/megabug/Software/arduino-1.0.4/hardware/arduino/cores/a
rduino/malloc.c:281: first defined here
/home/megabug/Software/arduino-1.0.4/hardware/tools/avr/bin/../lib/gcc/avr/4.3.2
/../../../avr/lib/avr51/libc.a(malloc.o): In function `free':
/home/paul/avr-libc-1.6.4/avr/lib/avr51/../../../libc/stdlib/malloc.c:195: 
multiple definition of `free'
core.a(malloc.c.o):/home/megabug/Software/arduino-1.0.4/hardware/arduino/cores/a
rduino/malloc.c:281: first defined here

this happens only with SdFat library (so far....). I didn't see any malloc() or 
free() re-definitions inside the library.
Any idea?

C

Original comment by c.mag...@arduino.cc on 12 Mar 2013 at 1:35

[GoogleCodeExporter]

Ok, found the answer myself:

http://code.google.com/p/beta-lib/issues/detail?id=9

sorry for the noise
C

Original comment by c.mag...@arduino.cc on 12 Mar 2013 at 1:42

[GoogleCodeExporter]

This has been on my to-do list.  I did not know you were about to release 
1.0.4, so it was a lower priority.  :-(

The self-contained malloc.c I made to fix the String class does not have some 
of the tuning parameters which are present in the C library copy.  If a program 
attempts to use those tuning variables, the linker tries to include both copies 
into the compiled code.

I've been planning to add the tuning parameters to the self-contained malloc.c. 
 That will fix the immediate problem with SdFat.  Sadly, 1.0.4 released without 
any notice (at least any I could see), so that will have to wait for 1.0.5.

It seems several libraries use a variety of techniques, to attempt to guess the 
amount of available memory.  Just yesterday I found one from Adafruit which has 
other dependencies.  Perhaps Arduino should provide an official API for a free 
memory estimate?

Original comment by paul.sto...@gmail.com on 12 Mar 2013 at 1:53

[GoogleCodeExporter]

I have planned the 1.0.5 release for next week, we have the time to fix it 
properly.

There are other way to get the free memory (that doesn't trigger the bug)?
Maybe its simpler to change SdFat instead of changing malloc.c? 
What is the library from Adafruit you are talking about?

C

Original comment by c.mag...@arduino.cc on 12 Mar 2013 at 2:43

[GoogleCodeExporter]

While sorting out this one can I please put in a plea to include the solution 
to issue1007 in the 1.0.5 release. Currently the definition of INADDR_NONE in 
IPaddress.h and its use by Dns.cpp locks up unnecessary SRAM. (see the 
originators and my oen contributions)

Original comment by brucedur...@btinternet.com on 12 Mar 2013 at 3:27

[GoogleCodeExporter]

Wow, another release in only 1 week.  Moving so very fast!!

I'm happy to make the edits to malloc.c.  It's not very difficult, but of 
course there is always a little apprehension touching something as sensitive as 
malloc/realloc/free.  This has been on my to-do list for about a month.  I will 
do it eventually.  If you want it for 1.0.5, I'm happy to raise its priority 
and do it this week.

The Adafruit library is ST7565.  It works fine with this malloc, but it can't 
compile on Arduino Due because of assumptions about AVR malloc.  It looks like 
SdFat has lots of #ifdefs for many different boards.  So will other libraries, 
if they want to use tricks to guess the amount of memory available.  It's up to 
you if a free memory estimate should be part of Arduino's official API.  Not a 
lot of libraries need this, but those that attempt it become compatibility 
headaches.

Original comment by paul.sto...@gmail.com on 12 Mar 2013 at 11:36

[GoogleCodeExporter]

Paul,

do it if it doesn't take too much of your time, its not so high priority in the 
end. 
This issue is like weed :-)

I see no problems adding a getFreeMemory() function in the Arduino API. The 
only concern is what amount of memory is returned? The longest block of 
contiguous memory? The sum of all fragments?
Maybe this is worth a discussion on the dev list.

C

Original comment by c.mag...@arduino.cc on 13 Mar 2013 at 6:27

• Changed state: Accepted

[GoogleCodeExporter]

For what it's worth, I'd like to see both types of API call: one for largest 
contiguous, and one for sum of all fragments.

Original comment by christop...@gmail.com on 13 Mar 2013 at 6:33

[GoogleCodeExporter]

I'll work on a patch for the AVR tunable stuff.  It impacts very few users, but 
the compiler's errors are so unhelpful that it causes a lot of pain.

For a free memory estimate, I believe the most useful would be the largest 
integer that can be used on the next call to malloc().  That's slightly less 
than the largest contiguous, due to overhead and the stack growth safety margin.

The main usage case seems to be an estimate to warn the user if their program 
is running dangerously low on memory.  The authors of these libraries know 
their code needs big buffers.  Users don't know, unless they read the 
documentation, but who does that?!  So the library author adds code to guess 
the free memory and print a message at the beginning of their examples.  When 
someone tries to run it on an old '186-based Arduino, or if they add another 
library with big buffers or use lots of memory themselves, the example gives 
them some warning of possible trouble, instead of mysteriously crashing.

Original comment by paul.sto...@gmail.com on 13 Mar 2013 at 7:01

[GoogleCodeExporter]

The proper thing to do is to basically tare out the broken malloc 
implementation in GLIBC and in place of that, use what ever is the fixed one. I 
have to take a closer look at the one in 1.5.2's core area, but I think you 
should be able to just drop it in and be done. No need to have two versions of 
malloc and friends floating around, since that is going to cause massive 
linking conflicts.

Original comment by xxx...@gmail.com on 15 Mar 2013 at 2:07

[GoogleCodeExporter]

xxxajk, I see that the original malloc.c is compiled into libc.a, we should 
rebuild the core.a for all OS with the replaced malloc.c. I think its more easy 
to put the updated malloc.c inside the arduino core, the linker should be smart 
enough to avoid linking problem (and choose the right malloc).

I have some free time now, I'll give a try with avr-libc 1.8.0's malloc.c.

Original comment by c.mag...@arduino.cc on 23 Mar 2013 at 7:14

[GoogleCodeExporter]

Ok, it wasn't so difficult, just brutally copied the files from the standard 
1.8.0 as xxxajk suggested:

https://github.com/arduino/Arduino/pull/1329

I've tried some string tests and runs ok,
SdFat and xmem compiles as well (but not tested)

does anyone see any issues?

Original comment by c.mag...@arduino.cc on 23 Mar 2013 at 8:50