Hi Stefan, I just found it during code reading.
- Alex On Tue, Oct 20, 2020 at 5:58 AM Dinkelacker, Stefan < s.dinkelac...@dkfz-heidelberg.de> wrote: > Thanks for the report. I also guess this is a typo. But I am curious if > this really caused any trouble for you or if it was a finding during code > reading? > > > Best, > > Stefan > ------------------------------ > *Von:* amelv...@umich.edu <amelv...@umich.edu> > *Gesendet:* Montag, 19. Oktober 2020 18:04 > *An:* mitk-users@lists.sourceforge.net > *Betreff:* [mitk-users] Possible Undefined behavior in mitkPlanarEllipse, > return by value function's return value stored in reference > > > If you prefer to look at a github issue, feel free to take a look at this > issue in our fork: > > > > https://github.com/carthurs/MITK/issues/3 > > > > I copy/pasted the content from the issue below, editing the line numbers > to match what’s in MITK’s master branch. > > > > This call to `GetControlPoint` stores the return value in a reference > > > > > https://github.com/MITK/MITK/blob/0f33cec9e310c3f69b6c46795f29c026f7e90dc6/Modules/PlanarFigure/src/DataManagement/mitkPlanarEllipse.cpp#L63 > > > > But `GetControlPoint` does not return a `Point2D&`, it returns a `Point2D` > > > > > https://github.com/MITK/MITK/blob/0f33cec9e310c3f69b6c46795f29c026f7e90dc6/Modules/PlanarFigure/src/DataManagement/mitkPlanarFigure.cpp#L240 > > > > I would take that to mean that the well defined lifetime of `centerPoint` > is a single line. > > > > MSVC may have a compiler extension allowing this, but I am not sure if > MITK has enabled this or not, and even if this option was enabled, this is > platform specific behavior and may not work the same on Mac, Linux, etc. > > > > There is a note about const refs extending the liftetime of temporary > objects, > > 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). > > > > But, reading the extensions linked in “Reference initialization” > > 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. > > > ... > > > > So a const ref *would be* except, if not for the fact that we're taking a > ref to a return value. > > > > This sounds like UB to me, but I’m not 100% sure if itkFixedArray has > something to mitigate this. Let me know what you think. > > > > Hope that helps, > > > > - Alex > >
_______________________________________________ mitk-users mailing list mitk-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mitk-users
