UB: mitkPlanarEllipse, return by value function's return value stored in non-const ref

[amelvill-umich]

This call to GetControlPoint stores the return value in a reference

https://github.com/carthurs/MITK/blob/4e084d2d790cac945757ead66b2898ca70e5cd28/Modules/PlanarFigure/src/DataManagement/mitkPlanarEllipse.cpp#L68

But GetControlPoint does not return a Point2D&, it returns a Point2D

https://github.com/carthurs/MITK/blob/4e084d2d790cac945757ead66b2898ca70e5cd28/Modules/PlanarFigure/src/DataManagement/mitkPlanarFigure.cpp#L253

I am pretty sure that this a typo on MITK's part.

From "Lifetime of a temporary" https://en.cppreference.com/w/cpp/language/reference_initialization

Whenever a reference is bound to a temporary or to a subobject thereof, the lifetime of the temporary is extended to match the lifetime of the reference, with the following exceptions:

• a temporary bound to a return value of a function in a return statement is not extended: it is destroyed immediately at the end of the return expression. Such function always returns a dangling reference. ...

I would take that to mean that the true lifetime of centerPoint is a single line.

https://github.com/carthurs/MITK/blob/4e084d2d790cac945757ead66b2898ca70e5cd28/Modules/PlanarFigure/src/DataManagement/mitkPlanarEllipse.cpp#L68-L107

There is an exception for const references,:

Bottom of: https://en.cppreference.com/w/cpp/language/reference#Lvalue_references

Note that rvalue references and lvalue references to const extend the lifetimes of temporary objects (see Reference initialization for rules and exceptions).

• MSVC may have a compiler extension allowing this, but I am not sure if MITK has enabled this or not.

Even if they did, in theory MITK should work on Linux, Mac, and Windows, so this may be platform specific behavior.

[amelvill-umich]

It depends on how you want to read the documentation,

Note that rvalue references and lvalue references to const extend the lifetimes of temporary objects (see Reference initialization for rules and exceptions).

Re-reading that again, the exceptions are,

a temporary bound to a return value of a function in a return statement is not extended: it is destroyed immediately at the end of the return expression. Such function always returns a dangling reference.

So a const ref would extend the lifetime of the temporary, if not for the fact that the temporary is taking a ref to a return value.

I'm going to say this is UB.

This issue is still present in MITK's master branch.

[amelvill-umich]

Reported to MITK mailing list 10/20/2020