Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in translate_enode_to_exprnode #340

Open
cgd8d opened this issue Apr 21, 2017 · 2 comments
Open

Memory leak in translate_enode_to_exprnode #340

cgd8d opened this issue Apr 21, 2017 · 2 comments
Assignees

Comments

@cgd8d
Copy link

cgd8d commented Apr 21, 2017

There is a memory leak in translate_enode_to_exprnode. I believe it exhibits itself in many different ways, but to be concrete, imagine the expression (+ 1 2) is being processed. This will reduce to the following pseudocode:

return &(*translate_enode_to_exprnode(“1”) + *translate_enode_to_exprnode(“2”));

which is essentially equivalent to (again, pseudocode):

return &ibex::ExprAdd::new_(ibex::ExprConstant::new_scalar([1,1]),
                            ibex::ExprConstant::new_scalar([2,2]))

For this particular realization, the memory leak is that ibex::ExprConstant::new_scalar allocates memory, ExprAdd takes ownership of that memory (as members “left” and “right”), but when the ExprAdd is deleted, it has no non-default destructor so it does not destroy those objects.

More generally, ibex::ExprBinaryOp has two references, but does not delete those references, so if it does have ownership then the memory is leaked.

In principle, since ibex::ExprBinaryOp exposes its members, a user program like dreal3 could work very hard to manage the memory itself. Thus, I submit this bug report to dreal3 even though I suspect it should be pushed upstream to ibex.

One complication I see with fixing the bug in ibex is that it is possible for functions to create an ExprBinaryOp where they do not want ExprBinaryOp to take ownership of those references, eg. someone could call ExprAdd::new_ with arguments they want to own. Maybe ibex does not intend to permit that kind of use, but it is not currently forbidden by the interface. If that is acceptable, then the easy fix in ibex may be to add the following destructor to ExprBinaryOp:

virtual ~ExprBinaryOp() {
    delete &left;
    delete &right;
}

At any rate, I have very limited familiarity with ibex, so I would be happy to submit a bug report there or to leave this bug in your hands, whatever you suggest.

@soonho-tri
Copy link
Member

Thanks for the report. As far as I know this is a known-issue in the ibex team. Last time I asked it, they gave me a workaround but I forget the details now. I'll check things up this weekend.

@cgd8d
Copy link
Author

cgd8d commented Apr 21, 2017

Thanks, much appreciated!

@soonho-tri soonho-tri self-assigned this Apr 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants