Created attachment 36760 [details] Test case If the leak checker marks a block as "possibly reachable" and then upgrades that to "definitely reachable" when it finds an additional pointer then any indirect blocks which were found in between those two events are not upgraded. It's not entirely clear whether this is a regression in 3.5.0 or not - certainly I've never noticed this before so I'm inclined to think the old algorithm avoided this somehow, or at least made it much less likely to be triggered. A test case is attached.
Created attachment 36762 [details] Patch to rescan blocks on upgrade This patch makes the leak checker rescan blocks when it changes their status from Possible to Reachable.
I'll look at the patch in detail later, but on a cursory scan I see that sizeof(LC_Extra) has increased, which is unfortunate as there can be a lot of them. We could reduce the number of bits for indirect_szB by another 1, hopefully 512MB would be enough on 32-bit platforms? Hmm.
Ah sorry - I hadn't realised indirect_szB was part of the bitfield so I didn't think I was increasing the size... The problem was that when you upgrade a block it might already be on the stack so without that flag you wind up pushing it again - in my original attempt I actually wound up overflowing the stack because I was pushing the same block multiple times and the stack is sized to the total number of blocks.
Created attachment 36763 [details] Patch to rescan blocks on upgrade Updated patch that reduces width of indirect_szB to avoid making LC_Extra bigger and also avoids an infinite loop by not repushing an already reachable block if we see it a second time.
(In reply to comment #2) > I'll look at the patch in detail later, but on a cursory scan I see that > sizeof(LC_Extra) has increased, which is unfortunate as there can be a lot of > them. We could reduce the number of bits for indirect_szB by another 1, > hopefully 512MB would be enough on 32-bit platforms? Hmm. Is it really so bad for LC_Extra to be 2 words instead of 1 ? IIUC the number of them is equal to the number of unfreed blocks at exit, which is going to be fairly small for mostly-well-behaved programs, in the range of a few hundred or a few thousand. I don't think I've ever seen a program which has (eg) a million blocks unfreed at exit, and even in that case on a 32-bit platform, that'd only use 8MB in total for 2 words. Any program exiting with millions of blocks unfreed is probably hosed w.r.t. space management anyway. I'd prefer to not impose a 512MB block size limit on the leak checker on 32-bit processes. It could come back and bite us as some tedious boundary case in the future, in which case we'd have to revisit this yet again. Instead let's take a bit of a space hit in the worst case. How about this: ULong size:60 UInt state:2 UInt pending:1 UInt spare:1 That gives no size limit on 32-bit or 64-bit and is only 8 bytes on both platforms.
I believe the 512Mb limit is on the amount of indirect memory reachable from a given block, not on the total amount of allocated memory. Even if that value overflows I think all that happens is that the statistics displayed for the amount of memory in various classes will be inaccurate.
*** Bug 252100 has been marked as a duplicate of this bug. ***
Please see my patch from Bug 252100.
Your patch is a strict subset of my one which is attached to this bug already.
My patch is just incorrect because it is possible to add the same block to the stack twice and so lc_markstack array can be overflowed. Thank you!
Patch was used at work on a big application, doing a lot of malloc/free. It works properly and has reduced significantly the nr of false possibly leaked. Note that the forest of false possibly leaked was hiding some trees of real leaks :).
Ok, let's get this landed. Tom, can you do it?
Committed as r11395.