Skip to content

PBs fix heap buffer overflow

Vasileios Angelidakis requested to merge PBs_fix_heap_buffer_overflow into master

Hi, in this commit I solve issue #125 (closed).

Although BlockGen has a built-in code to remove redundant planes from the blocks, it kept planes with one or two vertices attached to the particle (which do not correspond to actual faces).

The problematic code was duplicated in two different places inside the BlockGen.cpp, while I had recently used part of it in PotentialBlock.cpp, to calculate the particle centroid, volume and inertia. To resolve this, I deleted the duplicate code from the BlockGen, where the centroid, volume and inertia are now calculated in the shape class, calling PotentialBlock::postLoad(PotentialBlock&). Then, I fixed the issue in the PotentialBlock.cpp, by removing these redundant faces, which interfered with the calculation of volume/inertia and created the heap-buffer-overflow Anton detected.

While fixing this in PotentialBlock.cpp, I had to save the vertices associated with each plane, so it became easy to introduce an attribute named connectivity, to record the ordered vertices lying on each plane in an ordered manner (clockwise or counter-clockwise), which actually gives us a triangulation of each face. I will use this soon to visualise the PBs in qt, without needing to involve CGAL (i.e. making the Gl1_PotentialBlock.cpp script significantly lighter).

In addition:

  • in PotentialBlock.cpp, I renamed all iterators named "k" to "kk" to prevent conflicts, since "k" already exists as an attribute of the shape class.
  • to translate the blocks generated by BlockGen to their actual position, after calculating their characteristics in the shape class, I introduced an attribute named position in PotentialBlock.cpp, recording the eccentricity of a particle, if defined eccentrically.

@gladk thank you once more for finding the heap-buffer-overflow!! It would have taken me ages to find this critical bug on my own, since the checkBlockGen.py script did not detect it by sheer luck.

EDIT: @jduriez good find in !318 (closed). I have added your correction in this MR as well, since I wanted to check it didn't break my check tests.

All the best, Vasileios

Edited by Vasileios Angelidakis

Merge request reports