ParticleFx crashes frequently on 64x64 pixels

[softhack007]

What happened?

I often see crashes on my 64x64 matrix when changing between particle effects. It seems to be related to heap corruption.

What version/release of MM WLED?

dev

Which microcontroller/board are you seeing the problem on?

ESP32

Relevant log/trace output

*** Free heap: 56492    Largest free block: 12276 ***           Arduino min free stack: 4220   TCP min free stack: 6016 ***
*** Free heap: 56492    Largest free block: 12276 ***           Arduino min free stack: 4220   TCP min free stack: 6016 ***
 60 lps      16.6ms      61 fps

<<< This is the moment when I changed to another PS effect >>>

deserializeState(): strip is still drawing effects.
strip.waitUntilIdle(): strip idle after 12 ms. (task async_tcp with prio=3)
setUpMatrix 64 x 64
setupmatrix customMappingTable alloc 0 from 0


WLED error code = 34
CORRUPT HEAP: Bad tail at 0x3fffb04d. Expected 0xbaad5678 got 0x00000000

assert failed: multi_heap_free multi_heap_poisoning.c:253 (head != NULL)


Backtrace: 0x400869cd:0x3ffb1ff0 0x40090321:0x3ffb2010 0x40096cbd:0x3ffb2030 0x400968d3:0x3ffb2160 0x40086f2d:0x3ffb2180 0x40096ced:0x3ffb21a0 0x400f7a95:0x3ffb21c0 0x400feb42:0x3ffb21e0 0x400ff25e:0x3ffb2200 0x40141672:0x3ffb2230 0x40141c51:0x3ffb2270 0x4016b5e1:0x3ffb2290

  #0  0x400869cd:0x3ffb1ff0 in panic_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/panic.c:408
  #1  0x40090321:0x3ffb2010 in esp_system_abort at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/esp_system/esp_system.c:137
  #2  0x40096cbd:0x3ffb2030 in __assert_func at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/newlib/assert.c:85
  #3  0x400968d3:0x3ffb2160 in multi_heap_free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/heap/multi_heap_poisoning.c:253
      (inlined by) multi_heap_free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/heap/multi_heap_poisoning.c:245
  #4  0x40086f2d:0x3ffb2180 in heap_caps_free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/heap/heap_caps.c:361
  #5  0x40096ced:0x3ffb21a0 in free at /Users/ficeto/Desktop/ESP32/ESP32S2/esp-idf-public/components/newlib/heap.c:39
  #6  0x400f7a95:0x3ffb21c0 in Segment::deallocateData() at wled00/FX_fcn.cpp:258
  #7  0x400feb42:0x3ffb21e0 in Segment::resetIfRequired() at wled00/FX_fcn.cpp:276
      (inlined by) Segment::resetIfRequired() at wled00/FX_fcn.cpp:272
  #8  0x400ff25e:0x3ffb2200 in WS2812FX::service() at wled00/FX_fcn.cpp:1908 (discriminator 2)
  #9  0x40141672:0x3ffb2230 in WLED::loop() at wled00/wled.cpp:237
  #10 0x40141c51:0x3ffb2270 in loop() at C:/ARDUINO_Work/WORK/#ESP32/WLED_soundreactive-GitHub_DEV/WLED_MoonModules/WLED_mdev3_orig_clean/wled00/wled00.ino:98
  #11 0x4016b5e1:0x3ffb2290 in loopTask(void*) at C:/Users/frank/.platformio/packages/framework-arduinoespressif32/cores/esp32/main.cpp:50

ELF file SHA256: 9ea90e678a622b27

Rebooting...

Code of Conduct

• I agree to follow this project's Code of Conduct

[softhack007]

@coderabbitai I'm seeing a problem with several particle effects (wled00/FXparticleSystem.cpp, wled00/FXparticleSystem.h, and "PS" effect functions in FX.cpp). The crash occurs randomly when changing between particle effects.

The heap corruption message seems to indicate that a heap area was written out-of-bounds, or maybe used after free(). the abort in deallocateData() is most likely just a smoking gun, because the heap manager detects corrupted blocks late.

please scan the source code related to particle effects for possible write out-of-bounds and use-after-free problems. please indicate finding with high confidence separately from findings where you are not sure.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the particle system header file
cat wled00/FXparticleSystem.h

Length of output: 22874

---

🏁 Script executed:

#!/bin/bash
# Read the particle system implementation file
cat wled00/FXparticleSystem.cpp

Length of output: 99757

---

🏁 Script executed:

#!/bin/bash
# Find and extract PS effect functions from FX.cpp
# First, let's see the file structure
head -100 wled00/FX.cpp

Length of output: 3203

---

🏁 Script executed:

#!/bin/bash
# Search for all PS_ effect function definitions in FX.cpp
rg -n "uint16_t.*mode_[^(]*PS" wled00/FX.cpp

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Get the actual PS effect implementations with context
rg -n -A 50 "uint16_t.*mode_.*PS_" wled00/FX.cpp

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Search for ParticleSystem usage in FX.cpp
rg -n "ParticleSystem" wled00/FX.cpp | head -50

Length of output: 4263

---

🏁 Script executed:

#!/bin/bash
# Search for initParticleSystem calls
rg -n "initParticleSystem" wled00/FX.cpp | head -50

Length of output: 2698

---

🏁 Script executed:

#!/bin/bash
# Look for mode functions that might use particles
rg -n "mode_.*particle" -i wled00/FX.cpp | head -30

Length of output: 3340

---

🏁 Script executed:

#!/bin/bash
# Search for PartSys variable usage
rg -n "PartSys" wled00/FX.cpp | head -50

Length of output: 4030

---

🏁 Script executed:

#!/bin/bash
# Get a complete particle effect implementation
rg -n -A 100 "uint16_t mode_particlevortex" wled00/FX.cpp | head -150

Length of output: 4765

---

🏁 Script executed:

#!/bin/bash
# Look for array access patterns that could be out of bounds
# Check particles array access
rg -n "particles\[.*\]" wled00/FXparticleSystem.cpp | head -80

Length of output: 5779

---

🏁 Script executed:

#!/bin/bash
# Check framebuffer access patterns
rg -n "framebuffer\[" wled00/FXparticleSystem.cpp | head -50

Length of output: 997

---

🏁 Script executed:

#!/bin/bash
# Look for potential null pointer dereferences with advPartProps
rg -n -B2 -A2 "advPartProps\[" wled00/FXparticleSystem.cpp | head -100

Length of output: 4939

---

@softhack007, I've analyzed the particle system code for potential heap corruption issues. Here are my findings, separated by confidence level:

HIGH CONFIDENCE - Critical Issues

1. Framebuffer Out-of-Bounds Write During Wrapped Particle Rendering ⚠️

File: FXparticleSystem.cpp, Lines 640, 742, 778

The framebuffer is accessed using calculated indices like framebuffer[x + (maxYpixel - y) * (maxXpixel + 1)], but when wrapping is enabled and particles are rendered with advanced sizes, the wrapped coordinate validation is incomplete:

// Line 733-742: Wrapping logic for large particles
if (xfb > (uint32_t)maxXpixel) {
    if (wrapX) {
        if (xfb > (uint32_t)maxXpixel << 1)
            xfb = (maxXpixel + 1) + (int32_t)xfb;  // Cast from uint32 to int32!
        else
            xfb = xfb % (maxXpixel + 1);
    }
}
// Then: framebuffer[xfb + (maxYpixel - yfb) * (maxXpixel + 1)]

Problem: After wrapping, yfb coordinate wrapping occurs separately, but the final index xfb + (maxYpixel - yfb) * (maxXpixel + 1) is never validated to be within framebuffer bounds. If yfb wraps but maxYpixel - yfb becomes very large or negative, this writes beyond the framebuffer.

2. Use-After-Free During Effect Transitions ⚠️

File: FXparticleSystem.cpp, effect transitions in FX.cpp

The crash backtrace shows deallocateData() being called while the system is in WS2812FX::service() → Segment::resetIfRequired(). The pattern is:

1. Effect switch triggers SEGMENT.allocateData() which calls deallocateData() on old memory
2. New effect does PartSys = reinterpret_cast<ParticleSystem2D *>(SEGENV.data)
3. But WS2812FX::service() may still be iterating particles from the OLD effect

Problem: If a frame is mid-processing when an effect changes, particle arrays could be accessed after the memory is freed. The heap corruption at deallocateData() suggests corrupted heap metadata, likely from out-of-bounds writes that happened before deallocation.

3. Unchecked usedParticles vs numParticles Bounds ⚠️

File: FXparticleSystem.cpp, Lines 77, 583, and FX.cpp Line 9057

Loops iterate using usedParticles: for (uint32_t i = 0; i < usedParticles; i++), but usedParticles can be changed dynamically and there's no validation that it doesn't exceed numParticles (the actual allocated size).

In FX.cpp:

int partindex = (int)PartSys->usedParticles - (int)i;
if (partindex >= 0) {
    PartSys->particles[partindex].x = ...  // Only checks >= 0, not < numParticles!
}

Problem: If usedParticles is set larger than the allocated array during an effect parameter change, array access goes out of bounds.

4. Collision Detection with Dynamic usedParticles ⚠️

File: FXparticleSystem.cpp, Lines 811-843

The collision detection bins particles using pidx that wraps: if (pidx >= usedParticles) pidx = 0. But if setUsedParticles() reduces the count mid-collision-detection, pidx could point beyond the new limit before wrapping.

MEDIUM CONFIDENCE - Potential Issues

5. Advanced Properties Array Access Without Size Validation

File: FXparticleSystem.cpp, Lines 633, 677, 832

Advanced particle properties are accessed: advPartProps[particleindex].size with only null pointer checks, but no validation that particleindex < numParticles. If the particle index comes from a loop using usedParticles that exceeds numParticles, this is out of bounds.

6. Matrix Size Changes Don't Update In-Flight Particles

File: FXparticleSystem.cpp, updateSystem()

When setMatrixSize() is called, it updates maxX, maxY, and maxXpixel, maxYpixel, but particles already in flight keep their old positions. If the matrix shrinks (e.g., segment resize), particles outside the new bounds could write to invalid framebuffer locations.

7. Integer Overflow in Wrapped Coordinate Calculation

File: FXparticleSystem.cpp, Line 735

xfb = (maxXpixel + 1) + (int32_t)xfb;  // xfb is uint32_t, cast to int32_t

If xfb is very large (particle moving fast out of bounds), the cast from uint32_t to int32_t causes overflow, potentially creating a small positive or large negative value, leading to incorrect wrapped coordinates.

Recommendations

1. Add bounds checking before all framebuffer writes:

   uint32_t index = xfb + (maxYpixel - yfb) * (maxXpixel + 1);
   if (index < (maxXpixel + 1) * (maxYpixel + 1))
       fast_color_add(framebuffer[index], ...);

2. Add validation in setUsedParticles():

   usedParticles = min(usedParticles, numParticles);

3. Add frame synchronization during effect transitions to ensure old effect completes before deallocation.

4. Validate particleindex in all array accesses:

   if (particleindex >= numParticles) return;

Would you like me to generate patches for these issues, or investigate any specific finding in more detail?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and read the particle system files
echo "=== FXparticleSystem.h ==="
fd -t f "FXparticleSystem.h" --exec cat {}

echo -e "\n\n=== FXparticleSystem.cpp ==="
fd -t f "FXparticleSystem.cpp" --exec cat {}

echo -e "\n\n=== Searching for PS effect functions in FX.cpp ==="
# Look for functions that start with "PS_" or contain "Particle" in FX.cpp
rg -n -A 50 "uint16_t.*mode_PS.*\(" wled00/FX.cpp | head -500

Length of output: 122742

---

🏁 Script executed:

#!/bin/bash
# Search for more PS effect patterns and get more context
echo "=== Finding all PS-related effect functions in FX.cpp ==="
rg -n "mode_.*PS|particle|Particle" wled00/FX.cpp | head -100

echo -e "\n\n=== Looking for deallocateData usage ==="
rg -n -B5 -A5 "deallocateData\(\)" wled00/

echo -e "\n\n=== Looking for allocateData usage in particle context ==="
rg -n -B3 -A3 "allocateData.*particle|particle.*allocateData" wled00/

echo -e "\n\n=== Check FX_fcn.cpp for segment data management ==="
rg -n -B5 -A10 "deallocateData|resetIfRequired" wled00/FX_fcn.cpp | head -150

Length of output: 20232

---

@softhack007, I've completed a thorough analysis of the particle system code. Here are my findings:

HIGH CONFIDENCE Issues

1. Critical: Placement New Without Destructor Call ⚠️

Location: FXparticleSystem.cpp initialization functions, effect functions in FX.cpp

The particle system uses placement new to construct objects:

PartSys = new (SEGENV.data) ParticleSystem2D(...)

But when memory is freed in deallocateData() (line 258 of FX_fcn.cpp), it directly calls free(data) without invoking the destructor. While the current ParticleSystem classes don't have destructors, if any member variables had destructors or if cleanup logic were added later, this would cause resource leaks or undefined behavior.

2. Critical: Dangling Pointers After Reallocation ⚠️⚠️

Location: All particle effects + updatePSpointers() in FXparticleSystem.cpp

The Issue: The ParticleSystem stores internal pointers that are calculated once:

particles = reinterpret_cast<PSparticle *>(this + 1);
particleFlags = reinterpret_cast<PSparticleFlags *>(particles + numParticles);
sources = reinterpret_cast<PSsource *>(particleFlags + numParticles);
framebuffer = reinterpret_cast<CRGB *>(sources + numSources);

The typical effect pattern is:

if (SEGMENT.call == 0) {
  if (!initParticleSystem2D(PartSys, NUMBEROFSOURCES))
    return mode_static();
}
PartSys = reinterpret_cast<ParticleSystem2D *>(SEGENV.data);
PartSys->updateSystem();

The Problem: If SEGENV.data is reallocated between effect calls (during effect switching, segment resizing, or memory pressure), the memory block moves to a new location. The effect then casts the new SEGENV.data pointer, but all the internal pointers (particles, sources, framebuffer, etc.) still point to the OLD (now freed) memory location. Accessing these causes heap corruption.

Evidence from stack trace: The crash in deallocateData() with "Bad tail at 0x3fffb04d" suggests memory was corrupted by writing to freed memory.

3. Unsafe Array Access in renderParticle() with Advanced Rendering

Location: FXparticleSystem.cpp lines ~850-920 (2D), ~1250-1330 (1D)

When rendering particles with advPartProps and size > 1, a 10x10 renderbuffer is created on the stack. The code then calculates xfb_orig and yfb_orig:

uint32_t xfb_orig = x - (rendersize>>1) + 1 - offset;
uint32_t yfb_orig = y - (rendersize>>1) + 1 - offset;

The variables are unsigned, but the calculation can underflow (become very large). While there are wrap-around checks, the unsigned arithmetic combined with modulo operations could still produce out-of-bounds indices in edge cases, especially with particles near boundaries.

4. Missing Synchronization During Effect Switching

Location: WS2812FX::service() line 1908 + effect execution

The code flow is:

1. Main loop calls WS2812FX::service()
2. For each segment, it calls seg.resetIfRequired() (line 1908)
3. Then immediately runs the effect function

The Problem: If an effect switch occurs mid-frame:

• Thread 1: Effect is running, accessing particles[i], framebuffer[x], etc.
• Thread 2 (or same thread via interrupt): Calls resetIfRequired() → deallocateData() → free(data)
• Thread 1: Continues accessing now-freed memory → HEAP CORRUPTION

This is particularly problematic on ESP32 with multi-core or interrupt-driven operations.

---

MEDIUM CONFIDENCE Issues

5. Potential Bounds Issues in handleCollisions()

Location: FXparticleSystem.cpp ~lines 740-820 (2D), ~1450-1550 (1D)

The collision detection uses stack-allocated binIndices[maxBinParticles]. While there's overflow protection (if (binParticleCount >= maxBinParticles)), the assumption that "no more than half of particles are in same bin" could be violated in extreme cases (e.g., all particles at same position with collisions disabled initially, then enabled).

6. No Bounds Validation in blur2D()

Location: FXparticleSystem.cpp line ~955

The function accepts xstart, ystart, xsize, ysize but doesn't validate that xstart+xsize <= buffer_width or ystart+ysize <= buffer_height. Callers are responsible for this, but if miscalculated, it writes out of bounds.

7. Integer Overflow in Particle Coordinate Math

Location: Various particle movement and rendering functions

Operations like dx * dx + dy * dy (used for distance squared calculations) with 16-bit coordinates could overflow into negative numbers or wrap around, potentially causing incorrect collision detection or rendering at wrong locations.

---

Recommendations

Immediate fixes for high-confidence issues:

1. Add SEGENV.data validity checking: Before accessing particle system pointers, verify the memory hasn't been reallocated
2. Call updatePSpointers() more defensively: Check if SEGENV.data address changed since last frame
3. Add bounds validation in renderParticle(): Validate calculated indices before array access
4. Protect deallocateData() with a mutex or flag to prevent deallocation while effects are running
5. Consider adding idle frame counting: The code has MAX_MEMIDLE 10 defined but I don't see it used - implement delayed deallocation

Would you like me to generate specific code patches for any of these issues?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and read the particle system files
echo "=== FXparticleSystem.h ==="
fd -t f "FXparticleSystem.h" --exec cat {}
echo -e "\n\n=== FXparticleSystem.cpp ==="
fd -t f "FXparticleSystem.cpp" --exec cat {}

Length of output: 122689

---

🏁 Script executed:

#!/bin/bash
# Search for PS effect functions in FX.cpp that use particle system
echo "=== Searching for PS effect functions in FX.cpp ==="
rg -n "PS_" wled00/FX.cpp | head -50

echo -e "\n=== Looking for particle effect mode definitions ==="
rg -n "mode_.*PS|ParticleSystem" wled00/FX.cpp | head -30

echo -e "\n=== Check for any direct particle size or buffer accesses ==="
rg -nP "renderbuffer\[|advPartProps\[|particleindex" wled00/FXparticleSystem.cpp

Length of output: 13246

---

@softhack007, I've analyzed the particle system code. Here are my findings on potential heap corruption sources:

🔴 HIGH CONFIDENCE Issues

1. Critical: Memory allocation vs. pointer calculation mismatch during segment size changes

Location: FXparticleSystem.cpp Lines 924-928, 1013

The most likely cause of your heap corruption during effect transitions:

// Memory allocated based on dimensions at init time:
requiredmemory += sizeof(CRGB) * SEGMENT.virtualLength(); // Line 1013

// But pointers calculated in updatePSpointers based on CURRENT dimensions:
framebuffer = reinterpret_cast<CRGB *>(sources + numSources); // Line 924
uintptr_t p = reinterpret_cast<uintptr_t>(framebuffer + (maxXpixel+1)*(maxYpixel+1)); // Line 926

Problem: If the segment dimensions change between memory allocation and pointer updates (e.g., when switching effects or if segment configuration changes), the framebuffer pointer arithmetic will calculate positions beyond the actually allocated memory. Writing to framebuffer[x + y * width] will corrupt heap structures.

Happens when:

• Switching between particle effects with different segment sizes
• Segment reconfiguration between calls
• Matrix dimensions change dynamically

---

2. Stack overflow from collision detection VLA

Location: FXparticleSystem.cpp Line 797, similar at Line 1623 (1D)

uint32_t maxBinParticles = max((uint32_t)50, (usedParticles + 1) / 2); // Line 793
uint16_t binIndices[maxBinParticles]; // Line 797 - VLA on stack!

Problem: For 64x64 matrix with 2048 particles, maxBinParticles could be 1024, allocating 2KB on the stack (in handleCollisions). On ESP32 with 8KB stack, this is extremely risky, especially when combined with other function call overhead. Stack overflow corrupts heap memory adjacent to stack.

---

3. Unbounded array access in collision handling

Location: FXparticleSystem.cpp Lines 832-833

if (advPartProps) { 
  collDistSq = (particleHardRadius << 1) + 
    (((uint32_t)advPartProps[idx_i].size + (uint32_t)advPartProps[idx_j].size) >> 1);
}

Problem: idx_i and idx_j come from binIndices[] array. While filling logic appears sound, if there's any corruption in binIndices[] or usedParticles, these indices could exceed numParticles, causing out-of-bounds access to advPartProps[]. No explicit bounds checking before array access.

---

🟡 MEDIUM CONFIDENCE Issues

4. Collision pushing can move particles out of bounds

Location: FXparticleSystem.cpp Lines 895-922

The collision code pushes particles to prevent overlap, but doesn't validate the new position is within allocated memory before the next frame's access:

particle1.x += pushamount; // Line 914
particle1.y += pushamount; // Line 916

Comment on Line 918 acknowledges: "note: pushing may push particles out of frame". If these particles are then rendered before bounds checking, could write out of framebuffer.

---

5. Advanced particle property access patterns

Location: Multiple locations accessing advPartProps[particleindex]

While most accesses check if (advPartProps), the pattern throughout assumes particleindex is valid. During transition between effects or corruption, stale indices could be used.

---

🔍 Recommendations

Immediate fixes for high-confidence issues:

1. Add memory reallocation check in updateSystem():

   • Before calling updatePSpointers(), verify allocated memory size matches current segment dimensions
   • Or force reallocation if dimensions changed

2. Replace VLA with heap allocation or fixed size:

   • Use dynamic allocation for binIndices or limit to fixed reasonable size
   • Add overflow protection

3. Add bounds validation:

   • Validate idx_i < numParticles and idx_j < numParticles before accessing advPartProps[]

Would you like me to generate specific code fixes for any of these issues?

[softhack007]

@coderabbitai thank you for the detailed assessment.

WS2812FX::service() is calling all effects sequentially. There is no other thread that could initiate "resetIfRequired() → deallocateData() → free(data)" in parallel to the effect functions being active. deserializeSegment (json.cpp) is called from the web server thread, so this might possibly cause changes while an effect is "in flight". However there are interlocks in place: suspendStripService, strip.isServicing() and strip.waitUntilIdle(). Lock timeouts were not reported before the crash happened.

please repeat your analysis considering these constraints.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check how virtualLength is calculated and used in allocation
echo "=== Checking virtualLength usage ==="
rg -n "virtualLength\(\)" wled00/FXparticleSystem.cpp wled00/FX_fcn.cpp | head -20

echo -e "\n=== Looking at segment dimension properties ==="
rg -n "maxXpixel|maxYpixel|virtualWidth|virtualHeight" wled00/FXparticleSystem.cpp | head -30

echo -e "\n=== Check framebuffer allocation calculation ==="
rg -n -B5 -A5 "sizeof\(CRGB\).*virtualLength|framebuffer.*sizeof|requiredmemory.*CRGB" wled00/FXparticleSystem.cpp

echo -e "\n=== Check setMatrixSize and how dimensions are set ==="
rg -n -B3 -A10 "setMatrixSize" wled00/FXparticleSystem.cpp

echo -e "\n=== Look for framebuffer access patterns without bounds checking ==="
rg -n "framebuffer\[.*\]" wled00/FXparticleSystem.cpp | head -50

Length of output: 10018

---

🏁 Script executed:

#!/bin/bash
# Look at the specific wrapping and coordinate calculation code in detail
echo "=== Examining the wrapping coordinate calculation in renderParticle ==="
rg -n -B10 -A5 "xfb = \(maxXpixel \+ 1\) \+ \(int32_t\)xfb" wled00/FXparticleSystem.cpp

echo -e "\n=== Check VLA declaration and maxBinParticles calculation ==="
rg -n -B5 -A2 "binIndices\[maxBinParticles\]|maxBinParticles = max" wled00/FXparticleSystem.cpp

echo -e "\n=== Check collision detection and particle index usage ==="
rg -n -B3 -A8 "idx_i = binIndices\[|idx_j = binIndices\[" wled00/FXparticleSystem.cpp

echo -e "\n=== Look for any checks of framebuffer index before write ==="
rg -n -B2 "framebuffer\[.*\].*<" wled00/FXparticleSystem.cpp | head -30

Length of output: 6784

---

@softhack007, thank you for the clarification. With single-threaded execution and working interlocks, I can now focus the analysis on out-of-bounds writes rather than use-after-free scenarios. Here's my revised assessment:

🔴 HIGHEST CONFIDENCE - Primary Suspect

Stack Overflow from Variable Length Array (VLA) in Collision Detection

Location: FXparticleSystem.cpp Lines 796-798 (2D), Lines 1614-1616 (1D)

This is almost certainly your heap corruption culprit for 64x64 matrices:

uint32_t maxBinParticles = max((uint32_t)50, (usedParticles + 1) / 2);
uint16_t binIndices[maxBinParticles]; // VLA on stack!

The Math:

• 64×64 matrix = 4096 pixels
• Several particle effects allocate one particle per pixel: numberofParticles = SEGMENT.virtualLength() (Line 1777)
• maxBinParticles = max(50, (4096 + 1) / 2) = 2048
• VLA size = 2048 × sizeof(uint16_t) = 4096 bytes = 4KB

The Problem:

• ESP32 Arduino default stack size is 8KB
• Your VLA consumes 50% of the entire stack
• Add local variables, function parameters, return addresses, and nested call overhead
• Stack overflow corrupts heap structures adjacent to the stack
• The heap manager detects this corruption later when deallocateData() is called, producing your exact error: "CORRUPT HEAP: Bad tail at 0x3fffb04d"

Why it crashes during effect transitions:

• Collision detection is computationally expensive and happens mid-effect
• When switching between particle effects, both the new effect initialization AND collision handling may be active on the stack
• The combined stack usage pushes past the 8KB limit
• Corrupted heap metadata causes the crash to manifest when memory is next freed

---

🟡 HIGH CONFIDENCE - Secondary Issues

1. Missing Validation: usedParticles vs numParticles

Location: Throughout particle system, especially collision detection at Lines 827-836

for (uint32_t i = 0; i < binParticleCount; i++) {
  uint32_t idx_i = binIndices[i];
  // ...
  advPartProps[idx_i].size  // No bounds check that idx_i < numParticles

Problem: While binIndices filling has overflow protection (if (binParticleCount >= maxBinParticles)), there's no validation that:

1. Particle indices stored in binIndices[] are valid (< numParticles)
2. usedParticles never exceeds numParticles

If memory corruption from stack overflow corrupts usedParticles or the particle binning logic, invalid indices could be accessed.

---

2. Framebuffer Index Calculation Without Bounds Validation

Location: Lines 640, 742, 778 (2D), Lines 1508, 1574, 1597 (1D)

After wrapping arithmetic for coordinates, the final framebuffer index is calculated as:

framebuffer[xfb + (maxYpixel - yfb) * (maxXpixel + 1)]

While wrapping logic appears sound, there's no final bounds check before writing. If there's any bug in the wrapping arithmetic (especially with the unsigned-to-signed casts at Lines 722, 735), an out-of-bounds write could occur.

---

📋 Recommendations

Immediate Fix - Replace VLA with Heap Allocation:

// Instead of:
uint16_t binIndices[maxBinParticles];

// Use:
uint16_t *binIndices = nullptr;
if (maxBinParticles > 0) {
  binIndices = (uint16_t*)malloc(maxBinParticles * sizeof(uint16_t));
  if (!binIndices) {
    // Handle allocation failure - skip collision detection this frame
    return;
  }
}
// ... collision detection code ...
if (binIndices) free(binIndices);

Or cap the VLA to a safe size:

constexpr uint32_t MAX_SAFE_BIN_SIZE = 512; // 1KB on stack
uint32_t maxBinParticles = min(max((uint32_t)50, (usedParticles + 1) / 2), MAX_SAFE_BIN_SIZE);

Add Bounds Validation:

1. In setUsedParticles():

void setUsedParticles(uint32_t num) {
  usedParticles = min(num, numParticles);
}

2. Before array access in collision detection:

uint32_t idx_i = binIndices[i];
if (idx_i >= numParticles) continue; // Skip invalid index

3. Before framebuffer writes:

uint32_t fbIndex = xfb + (maxYpixel - yfb) * (maxXpixel + 1);
if (fbIndex < (maxXpixel + 1) * (maxYpixel + 1))
  fast_color_add(framebuffer[fbIndex], color, brightness);

Optional - Increase Stack Size:

For WLED, you could increase the Arduino loop task stack size in your main sketch file:

SET_LOOP_TASK_STACK_SIZE(16 * 1024); // 16KB instead of default 8KB

However, this only delays the problem - the VLA should still be replaced.

---

Would you like me to generate complete patch files for the VLA replacement and bounds validation fixes?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for initParticleSystem implementations and usage patterns
echo "=== Finding initParticleSystem definitions ==="
rg -n "bool.*initParticleSystem" wled00/

echo -e "\n=== Check how dimensions are used in allocation ==="
rg -n -B5 -A10 "sizeof\(CRGB\).*virtualLength|sizeof\(CRGB\).*\(maxX|maxY" wled00/FXparticleSystem.cpp

echo -e "\n=== Check updatePSpointers and updateSystem calls ==="
rg -n -B3 -A5 "updatePSpointers\(|updateSystem\(" wled00/FXparticleSystem.cpp

echo -e "\n=== Look for segment dimension changes ==="
rg -n "setMatrixSize|maxX|maxY" wled00/FXparticleSystem.cpp | head -50

echo -e "\n=== Check allocateData and deallocateData patterns ==="
rg -n -B5 -A10 "allocateData|deallocateData" wled00/FX_fcn.cpp | head -100

Length of output: 27693

---

🏁 Script executed:

#!/bin/bash
# Check how segment size changes are handled and if dimensions can differ
echo "=== Check SEGMENT.virtualLength() calculation ==="
rg -n "virtualLength\(\)|virtualWidth\(\)|virtualHeight\(\)" wled00/FX_fcn.cpp | head -30

echo -e "\n=== Check if segment dimensions can change without triggering full reallocation ==="
rg -n -B5 -A10 "if \(data && _dataLen >= len\)" wled00/FX_fcn.cpp

echo -e "\n=== Look for segment resize scenarios ==="
rg -n "setSize|resize|dimensions" wled00/FX_fcn.cpp | head -20

echo -e "\n=== Check what triggers allocateData ==="
rg -n -B3 "allocateData\(" wled00/FX.cpp | head -50

echo -e "\n=== Check ESP32 stack size configuration ==="
rg -n "STACK_SIZE|CONFIG_.*STACK" platformio.ini | head -20

Length of output: 6594

---

🏁 Script executed:

#!/bin/bash
# Trace through a typical particle effect initialization and updates
echo "=== Find a typical particle effect pattern ==="
rg -n -A30 "mode_particlefire\(\)" wled00/FX.cpp | head -50

echo -e "\n=== Check how segment dimensions can change dynamically ==="
rg -n "virtualWidth|virtualHeight|grouping|mirror|reverse" wled00/FX.h | head -40

echo -e "\n=== Check if dimensions stored vs calculated ==="
rg -n "calc_virtualWidth|calc_virtualHeight" wled00/FX_fcn.cpp | head -30

echo -e "\n=== Look at initParticleSystem2D more closely ==="
rg -n -B5 -A50 "^bool initParticleSystem2D" wled00/FXparticleSystem.cpp | head -80

echo -e "\n=== Check collision detection stack usage ==="
echo "VLA size calculation for 64x64 matrix with max particles:"
echo "For 64x64 = 4096 pixels, usedParticles could be ~2048"
echo "maxBinParticles = max(50, 2048/2) = 1024"
echo "binIndices[1024] = 2KB on stack"

Length of output: 6698

---

@softhack007, thank you for the clarification. With those threading constraints in mind, here's my revised analysis:

🔴 PRIMARY SUSPECT - High Confidence

Memory Allocation vs. Runtime Dimension Mismatch ⚠️⚠️⚠️

Location: FXparticleSystem.cpp Lines 1106-1131, 960-981

This is the most likely cause of your heap corruption on 64x64 matrices:

The Issue:

// Line 1106 - Memory allocated based on CURRENT segment dimensions:
requiredmemory += sizeof(CRGB) * SEGMENT.virtualLength(); // virtualLength = width × height

// Line 223 FX_fcn.cpp - Memory REUSED if large enough:
if (data && _dataLen >= len) {
    if ((call == 0) && (len > 0)) memset(data, 0, len);
    return true; // <-- No reallocation!
}

// Line 962 FXparticleSystem.cpp - updateSystem() called every frame:
setMatrixSize(SEGMENT.virtualWidth(), SEGMENT.virtualHeight()); // Updates maxXpixel, maxYpixel

// Line 981 - Pointer arithmetic assumes CURRENT dimensions:
uintptr_t p = reinterpret_cast<uintptr_t>(framebuffer + (maxXpixel+1)*(maxYpixel+1));
PSdataEnd = reinterpret_cast<uint8_t *>(p);

Critical Problem: SEGMENT.virtualWidth() and virtualHeight() are calculated dynamically from segment properties that can change at runtime:

// FX.h lines 708-721
inline uint16_t virtualWidth() const {
  uint16_t vWidth = width();
  vWidth = ((vWidth + groupLength() - 1) / groupLength()); // Affected by grouping & spacing
  if (mirror) vWidth = (vWidth + 1) /2;  // Affected by mirror flag
  return vWidth;
}

Crash Scenario for 64x64 Matrix:

1. Initial state: Segment configured with mirror=true, calculates to 32×64 virtual pixels = 2048 pixels

   • initParticleSystem2D() allocates: sizeof(CRGB) * 2048 = 6144 bytes for framebuffer

2. User changes settings via web API (deserializeSegment): mirror=false

   • New dimensions: 64×64 = 4096 pixels
   • Required framebuffer: sizeof(CRGB) * 4096 = 12288 bytes

3. Effect calls updateSystem():

   • setMatrixSize(64, 64) → sets maxXpixel=63, maxYpixel=63
   • updatePSpointers() calculates: framebuffer + (63+1)*(63+1) = framebuffer + 4096
   • This position is 6144 bytes beyond the actual allocation!

4. Out-of-bounds writes:

   • renderParticle() line 640: framebuffer[x + (maxYpixel - y) * (maxXpixel + 1)]
   • Writing to framebuffer[4000] when only 2048 elements allocated
   • Corrupts heap metadata structures

5. Delayed detection:

   • Corruption happens during rendering
   • Next effect switch triggers deallocateData()
   • Heap manager detects: "CORRUPT HEAP: Bad tail at 0x3fffb04d"

Why memory not reallocated: Line 223 if (data && _dataLen >= len) only checks if a buffer exists and is large enough for the NEW effect's requirements. But if the new effect needs LESS memory than currently allocated, it reuses the buffer. Then if segment dimensions GROW during execution, the framebuffer becomes too small.

---

🔴 SECONDARY ISSUE - High Confidence

Stack Overflow from Variable Length Array in Collision Detection

Location: FXparticleSystem.cpp Lines 793-797

uint32_t maxBinParticles = max((uint32_t)50, (usedParticles + 1) / 2);
uint16_t binIndices[maxBinParticles]; // VLA on stack

For 64×64 matrix:

• Maximum particles ≈ 2048 (from calculateNumberOfParticles2D)
• maxBinParticles = max(50, 1024) = 1024
• Stack allocation: 1024 × sizeof(uint16_t) = 2048 bytes
• ESP32 loop task stack: ~8-9KB (from platformio.ini CONFIG_ASYNC_TCP_TASK_STACK_SIZE)
• This VLA consumes 22-25% of available stack

Combined with other function call overhead (handleCollisions → updateSystem → service), this can cause stack overflow, which manifests as heap corruption when the stack grows into heap territory.

---

🟡 MEDIUM CONFIDENCE - Contributing Factor

advPartProps Bounds Not Validated After Dimension Changes

Location: FXparticleSystem.cpp Lines 633, 677

When updateSystem() changes dimensions but numParticles stays constant, loops using usedParticles could exceed the bounds if usedParticles wasn't adjusted proportionally to the new matrix size.

---

Recommendations

Immediate Fix #1 - Validate Framebuffer Size

Add validation in updatePSpointers():

void ParticleSystem2D::updatePSpointers(bool isadvanced, bool sizecontrol) {
  // ... existing pointer calculations ...
  
  // VALIDATION: Verify framebuffer fits in allocated memory
  uint32_t requiredFramebufferSize = (maxXpixel + 1) * (maxYpixel + 1);
  uintptr_t p = reinterpret_cast<uintptr_t>(framebuffer + requiredFramebufferSize);
  
  // Check if calculated end pointer exceeds SEGENV.data allocation
  uintptr_t dataEnd = reinterpret_cast<uintptr_t>(SEGENV.data) + SEGENV._dataLen;
  if (p > dataEnd) {
    // Framebuffer too large - force reallocation
    USER_PRINTLN(F("PS: Dimension mismatch detected, reallocating"));
    return; // Return early, effect should reinitialize
  }
  
  PSdataEnd = reinterpret_cast<uint8_t *>(p);
  // ... rest of function ...
}

Immediate Fix #2 - Replace VLA with Bounded Allocation

void ParticleSystem2D::handleCollisions() {
  const uint32_t MAX_BIN_PARTICLES = 256; // Conservative limit
  uint32_t maxBinParticles = min(MAX_BIN_PARTICLES, max((uint32_t)50, (usedParticles + 1) / 2));
  uint16_t binIndices[MAX_BIN_PARTICLES]; // Fixed size on stack
  // ... rest of function ...
}

Root Cause Fix - Force Reallocation on Dimension Changes

In initParticleSystem2D(), always check if dimensions changed:

bool initParticleSystem2D(ParticleSystem2D *&PartSys, ...) {
  uint32_t cols = SEGMENT.virtualWidth();
  uint32_t rows = SEGMENT.virtualHeight();
  uint32_t pixels = cols * rows;
  
  // Check if existing allocation has different dimensions
  if (SEGENV.data != nullptr) {
    ParticleSystem2D *existing = reinterpret_cast<ParticleSystem2D *>(SEGENV.data);
    uint32_t existingPixels = (existing->maxXpixel + 1) * (existing->maxYpixel + 1);
    if (existingPixels != pixels) {
      // Force deallocation if dimensions changed
      SEGMENT.deallocateData();
    }
  }
  
  // ... continue with allocation ...
}

Would you like me to generate complete patch files for these fixes?

[softhack007]

Thank you very much @coderabbitai. I will check your findings and i'll try out some fixes based on your suspects.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@softhack007, you're very welcome! I'm glad the analysis was helpful. The primary suspects I'd focus on first are:

1. Memory allocation vs. runtime dimension mismatch - especially if segments are reconfigured with mirror/grouping changes while effects are running
2. VLA stack overflow in collision detection for large matrices

If you need any clarification on the findings, help generating specific patches, or assistance validating the fixes, feel free to ping me. Good luck with the debugging! 🐰

    (\(\
    (='.')
    (")_(")
   [Happy to help!]

[softhack007]

Hallo @DedeHai,

I'm slowly finding the time to come back to wled and wled-MM.
The rabbit has identified some potential problem in your ParticleFX code (see above) - maybe you want to cross-check them because I assume that similar things could also happen in upstream WLED?

I'm using an esp32 with 64x64 HUB75 panel right now, which is "small" from wled-MM point of view ;-)

viele Grüße, Frank

[softhack007]

@coderabbitai how can I change your configuration, so that you will always report VLA when they appear as part of a PR under review? My intention is to find a way to highlight use of VLA early, and make sure they are duly justified by the PR author.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch CodeRabbit configuration schema and review instructions documentation
echo "=== Fetching configuration schema ==="
curl -s "https://coderabbit.ai/integrations/schema.v2.json" | jq -r '.' | head -300

echo -e "\n\n=== Fetching review instructions documentation ==="
curl -s "https://docs.coderabbit.ai/guides/review-instructions"

Length of output: 278485

---

@softhack007 Here are two complementary ways to make CodeRabbit always call out VLAs in PRs.

Option A — Quick, zero-setup signal via path-based instructions
Add this to your .coderabbit.yml so every C/C++ change gets a VLA justification request during review:

reviews:
  path_instructions:
    - path: "**/*.{c,cc,cpp,h,hpp}"
      instructions: |
        - If a Variable Length Array (VLA) is introduced, modified, or used inside a function or as a parameter, raise a dedicated comment asking the author to justify it.
        - Treat VLA usage as high risk on embedded targets (stack pressure, OOB risk). Prefer fixed-size arrays, std::array/std::vector (C++), or heap allocation with explicit bounds checks.
        - If a VLA is truly required, the PR must include: documented maximum size, bounds rationale, and tests on the largest expected size.

Pros: immediate signal. Cons: heuristic (relies on the reviewer model, not deterministic pattern matching).

Option B — Deterministic detection with AST rules (recommended)
Enable ast-grep rules to mechanically detect VLAs in both C and C++.

1. In .coderabbit.yml, enable ast-grep and point to your rules folder:

reviews:
  tools:
    ast-grep:
      essential_rules: true
      rule_dirs:
        - rules/c-family

2. Create rules/c-family/no-vla-c.yml

id: no-vla-c
language: C
severity: warning
message: >
  Variable Length Array (VLA) detected. On embedded targets this can cause stack overflows
  and undefined behavior. Prefer fixed-size arrays or heap allocation. If you must use a VLA,
  add a justification (max size, bounds rationale, tests).
rule:
  # Match declarations like: int buf[n];  float a[m + 2];  uint8_t x[width*height];
  pattern: $TYPE $NAME[$SIZE];
  all:
    - inside:
        # catch local/block-scope declarations (common place for VLAs)
        any:
          - kind: compound_statement
          - kind: parameter_list
  # Heuristic filter: skip obvious compile-time constants (decimal/hex literals or sizeof(...))
  not:
    any:
      - regex: '\[\s*(\d+|0x[0-9a-fA-F]+)\s*\]'
      - regex: '\[\s*sizeof\s*\([^)]+\)\s*\]'

3. Create rules/c-family/no-vla-cpp.yml

id: no-vla-cpp
language: Cpp
severity: warning
message: >
  Variable Length Array (GNU extension) detected in C++. This is non-standard and risky on embedded
  targets. Prefer std::array/std::vector or fixed-size arrays. If used intentionally, justify it.
rule:
  # Matches e.g.: int buf[n]; inside functions/params in C++
  pattern: $TYPE $NAME[$SIZE];
  all:
    - inside:
        any:
          - kind: compound_statement
          - kind: parameter_list
  not:
    any:
      - regex: '\[\s*(\d+|0x[0-9a-fA-F]+)\s*\]'
      - regex: '\[\s*sizeof\s*\([^)]+\)\s*\]'

Notes and tuning

• The regex “not” guards reduce false positives for obvious compile-time constants. If you use macros like NUM_LEDS that expand to constants, these rules may still flag them. You can narrow scope further (e.g., require presence of an identifier in the size expression) by replacing the second block with:
  • regex: '[\s*[A-Za-z_][A-Za-z0-9_]*' # requires an identifier in the dimension
• To escalate impact, change severity to error and adopt a policy that PRs with error-level ast-grep findings must be addressed or explicitly waived.
• To discourage regressions, keep Option A’s path_instructions alongside the AST rules; the natural-language comment asks authors for a written justification even when the rule fires.

Optional: add a waiver pattern
If you need rare, auditable exceptions, agree on a single-line waiver comment and add a “not” to ignore lines containing it:

• Team convention in code: // coderabbit: allow-vla (explain why, max size = 1024)
• Then add to each rule:
  not:
  any:
  - regex: 'coderabbit:\s*allow-vla'

Want me to open a PR that adds these rules and the .coderabbit.yml changes to your repo, or tailor the regex to your macro conventions (e.g., WIDTH, HEIGHT, NUM_LEDS)?

[softhack007]

@netmindz I'll leave that up to you. I like the idea of also adding a waiver pattern.