search

MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.26k stars 19.23k forks source link

[BUG] M20 crashes when recursing SD Card #23321

Open Acrobot opened 2 years ago

Acrobot commented 2 years ago

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

I was trying to debug why OctoPrint was causing my whole RAMPS 1.4 clone board to crash (based on Atmega 2560) when requesting M20 command. Turns out that my SD card, which I just reused from a different device, has a lot of directories which Marlin recurses. There is already a comment in the code:

 * This method recurses to unlimited depth and lists all G-code
 * files within the given parent. If the hierarchy is very deep
 * this can blow up the stack, so a 'depth' parameter would be a
 * good addition.

but I don't see it addressed in any issue or in the bugfix branch. Could you please either make it non-recursive or add a parameter (the 'depth' parameter mentioned) which could limit how deep the listing should go?

Bug Timeline

No response

Expected behavior

I expected Marlin to display the directory listing with M20

Actual behavior

The whole board crashes, with no way to recover other than removing power from the board.

Steps to Reproduce

  1. Prepare an SD card with many directories and files
  2. Issue M20 command
  3. Observe crashed board

Version of Marlin Firmware

2.0.9.2

Printer model

Sunhokey Prusa i3

Electronics

RAMPS 1.4 clone

Add-ons

No response

Bed Leveling

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

descipher commented 2 years ago

The ATMEGA2560 has limited memory. How many files are we talking about?

Acrobot commented 2 years ago

161 directories, 158 files

It's not a must per-se (I just inserted a card from Android and it contained many files from Spotify cache), but it was hard to debug what crashed the board. I think changing this from recursive to iterative might solve the problem (or just at least display an ellipsis if there are more files in a directory than the configured limit).

descipher commented 2 years ago

I looked at the code, not a simple fix, even with a limit the memory will become too tight on an AVR. Too many elements are impacted. I think we need to emphasize in the docs that proper SD prep is required for reliable AVR operation.

thinkyhead commented 2 years ago

161 directories, 158 files

It's no surprise. There is no use of malloc in the embedded-C++ idiom, but stack frames are used. An SD card with that much stuff is going to either explode or only show you a random assortment of files. I would avoid running M20 with that card in any contemporaneous 3D printer. We'll do what we can to prevent a crash, but it's unlikely we'll ever be able to accommodate an unlimited number of files and folders.