UCL / STIR

Software for Tomographic Image Reconstruction
http://stir.sourceforge.net/
Other
111 stars 93 forks source link

Core dump while trying to access Scanner object created for an unknown scanner #1520

Open robbietuk opened 3 days ago

robbietuk commented 3 days ago

Found this issue in an code base. I can replicate the issue with a modification of using_installed_STIR C++ example https://github.com/UCL/STIR/tree/master/examples/C%2B%2B/using_installed_STIR

Setup

Fresh STIR clone, build and install, most options like ROOT, Python, OpenMP are disabled.

$cd ${STIR_REPO}/examples/C++/using_installed_STIR 
$cmake . -DSTIR_DIR=${STIR_REPO}/install/lib/cmake/STIR-6.2 # No change in output between `CMAKE_BUILD_TYPE` $configurations
$make
$./demo_create_image

This all works fine, create the expected output and test interfiles are created.


Modify the demo_create_image.cxx file to

#include "stir/Scanner.h"

int
main()
{
  std::cout << "Running demo_create_image" << std::endl;
  auto stir_scanner = stir::Scanner::get_scanner_from_name("D690");
  std::cout << "Constructed Scanner" << std::endl;
  // Basic test to ensure accessible scanner properties
  std::cout << stir_scanner->get_num_axial_blocks() << std::endl;
  std::cout << "Done" << std::endl;
  return EXIT_SUCCESS;
}
$ make && ./demo_create_image
Consolidate compiler generated dependencies of target demo_create_image
[100%] Built target demo_create_image
Running demo_create_image
Constructed Scanner
Floating point exception (core dumped)

It produces the Floating point exception (core dumped) during stir_scanner->get_num_axial_blocks()


I have a setup with a windows install and I don't get this issue. I am not sure what I am missing...

robbietuk commented 2 days ago

So the issue is with the stir::Scanner::get_scanner_from_name("D690"); The name is not valid and returns new Scanner(Unknown_scanner) but with no warning. Access of this "Unknown_scanner" object is weird (I'm not looking into it) and leads to the Floating point exception (core dumped) message and error state.

KrisThielemans commented 2 days ago

Obviously, the initialisation with an Unknown_scanner shouldn't cause seg-faults later on. Sigh.

Note that you now do have to call set_up() after creating a Scanner (not checked sufficiently yet).

auto stir_scanner = stir::Scanner::get_scanner_from_name("D690");

Note that this currently causes a memory leak in your example, as it returns a bare pointer. It really should return a std::unique_ptr, but then we'll have to add a SWIG work-around as that's only supported very recently.

Looks like Scanner needs some work.

KrisThielemans commented 2 days ago

Reopening as we didn't really fix this