py/parse: Zero dangling parse tree pointer to fix possible leak. - #14136

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.39%. Comparing base (9d27183) to head (a6973ef).

❗ Current head a6973ef differs from pull request most recent head 71044a4. Consider uploading reports for the commit 71044a4 to get more accurate results

@@           Coverage Diff           @@
##           master   #14136   +/-   ##
=======================================
  Coverage   98.39%   98.39%           
=======================================
  Files         161      161           
  Lines       21199    21200    +1     
=======================================
+ Hits        20859    20860    +1     
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Code size report:

   bare-arm:    +4 +0.007% 
minimal x86:    +8 +0.004% 
   unix x64:   +24 +0.003% standard
      stm32:    +8 +0.002% PYBV10
     mimxrt:    +8 +0.002% TEENSY40
        rp2:    +0 +0.000% RPI_PICO
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS

I did a quick audit of other functions calling m_del in similar context and didn't find any obvious ones, however I think it's possible for something similar to happen with any normal C automatic variables:

1. Function A allocates a 16 byte stack frame, uses 3 words of it.
2. The highest word is a pointer to the heap which is freed.
3. Function A returns.
4. Function B is called, allocates a 16 byte stack frame (same address on the stack as A), only uses 2 words of it, calls into child function C.
5. The pointer from step 2 is still present in valid stack memory from now until function B returns.

I think the only way around this reliably would be a m_free_and_null(p) function that nulls out the argument pointer. May be worth considering?

This means if any Python object happens to allocate at the same address

It's not even "at" same address I realise, it only has to overlap the dangling pointer address.

Thank you!

I think the only way around this reliably would be a m_free_and_null(p) function that nulls out the argument pointer. May be worth considering?

Maybe...

I think the only way around this reliably would be a m_free_and_null(p) function that nulls out the argument pointer. May be worth considering?

That wouldn't have helped this case if it were written as:

    m_free_and_null(&chunk);

because chunk is a local variable. Instead it would have needed some more sophisticated code for the un-link list logic.

@dpgeorge good point, so yeah maybe it's OK to do it piecemeal if/when these things come up.