sdd / kiddo

Kiddo
Apache License 2.0
79 stars 13 forks source link

Underflow subtraction #172

Open ktmattson-alion opened 1 month ago

ktmattson-alion commented 1 month ago

I have at least one point cloud (so far each one I've tried) that causes calc_pivot to underflow here. Unfortunately I'm not able to release them (plus they are very large,) so I'll try to characterize the problem as it occurs.

When we get a panic, chunk_length = 65 and shifted = 64. Thus, pivot = (65 + 64) >> 1 = 64. There is a catch for chunk_length being odd, but it fails to engage because shifted != 0.

Looking over this function, I see a series of conditions, but there are only two outcomes: pivot = pivot.next_power_of_two() or (pivot + 1).next_power_of_two(). And while there is a test on stem_index, it doesn't actually alter the logic: odd numbers need to be incremented after shifting to get the correct next power.

I tried this modification,

let mut pivot = chunk_length + shifted;  
 pivot = if pivot & 1 == 1 {  
   ((pivot >> 1) + 1).next_power_of_two()  
} else {  
    (pivot >> 1).next_power_of_two()  
};  
pivot -= shifted;  

but it still fails for values like chunk = 42, shifted = 75. At this point, I don't really understand quite what is supposed to happen. Like I can stabilize this function naively, with checked subtraction, but what's supposed to happen when shifted > pivot? plus later on there's another underflow problem if pivot > chunk_length.