search

conp-solutions / riss

Riss SAT Solver
GNU Lesser General Public License v2.1
8 stars 5 forks source link

fix double free #26

Closed AntonReinhard closed 2 years ago

AntonReinhard commented 2 years ago

Should fix issue #18

conp-solutions commented 2 years ago

Thanks, I will have a look.If i understand the diff correctly, this also adapts the code style. Is there a way to split code style changes from the functional change, so that I can easily spot the semantic change and reason about this?Many small commits work better for me than a big diff.

AntonReinhard commented 2 years ago

Oh sorry, the code style change was on accident

conp-solutions commented 2 years ago

Do you have a reproducer for this that indicates that the relevant line of code is actually the affected one? Given the destructor, it looks like it only calls free in valid cases. Because it is not obvious, I would like to better understand what is going wrong. Thanks!

AntonReinhard commented 2 years ago

For example I used ./coprocessor track1_004.mcc2020_cnf -search=0, which fails because of double free on master, and doesn't on this branch. You do check whether the pointers are null before freeing them, and then set them null after freeing them. But the state of the object is undefined after a destructor call, so there is no guarantee that on a second destructor call those pointers will still have the same value. In fact, the problem disappears when building for Debug, which I think happens because the compiler optimizes the pointer = 0 statements away in Release, since the object is in undefined state after the destructor call anyways.

Quote from cppref:

Note that calling a destructor directly for an ordinary object, such as a local variable, invokes undefined behavior when the destructor is called again, at the end of scope.