Fields2Cover / fields2cover_ros

A ROS interface of Fields2Cover
BSD 3-Clause "New" or "Revised" License
44 stars 19 forks source link

SegFault in ROS::conversor F2CCell to geometry_msgs::Polygon #5

Closed vinnnyr closed 1 year ago

vinnnyr commented 1 year ago

This might be a quirk of this branch https://github.com/Fields2Cover/fields2cover_ros/pull/4

but I get a segfault in the F2CCell to geometry_msgs::Polygon conversion here: https://github.com/Fields2Cover/fields2cover_ros/blob/3ff6ad499c30f6d4c52c2b46fe921ff079b5b3b1/include/ros/conversor.h#L26

note -- I don't actually have a good way of confirming that this exists on master since I don't have a ROS1 environment easily set up / unit testable.

This is the test I ran that causes the SEG fault:

  F2CLinearRing outer_ring{
    F2CPoint(0, 0), F2CPoint(2, 0), F2CPoint(2, 2), F2CPoint(0, 2), F2CPoint(0, 0)};
  F2CLinearRing inner_ring{
    F2CPoint(0.5, 0.5), F2CPoint(1.5, 0.5), F2CPoint(1.5, 1.5),
    F2CPoint(0.5, 1.5), F2CPoint(0.5, 0.5)};
  F2CCell cell;
  cell.addRing(outer_ring);
  cell.addRing(inner_ring);

  auto poly1 = conversor::GeometryMsgs::Polygon();
  auto poly2 = conversor::GeometryMsgs::Polygon();

  std::vector<conversor::GeometryMsgs::Polygon> polygons;
  polygons.push_back(poly1);
  polygons.push_back(poly2);

  conversor::ROS::to(cell, polygons);

This is the BT I get

Thread 1 "test_ros_conver" received signal SIGSEGV, Segmentation fault.
0x00007fb819125d5c in f2c::types::Geometry<OGRLinearRing, (OGRwkbGeometryType)101>::Geometry (this=<optimized out>, g=<optimized out>, this=<optimized out>, g=<optimized out>) at /ws/src/fields2cover/include/fields2cover/types/Geometry_impl.hpp:33
33      Geometry<T, R>::Geometry(const T* g) : data(static_cast<T*>(g->clone()),
(gdb) bt
#0  0x00007fb819125d5c in f2c::types::Geometry<OGRLinearRing, (OGRwkbGeometryType)101>::Geometry (this=<optimized out>, g=<optimized out>, this=<optimized out>, g=<optimized out>)
    at /ws/src/fields2cover/include/fields2cover/types/Geometry_impl.hpp:33
#1  0x00007fb819126ae2 in f2c::types::Geometries<f2c::types::LinearRing, OGRLinearRing, (OGRwkbGeometryType)101, f2c::types::Point>::Geometry (this=<optimized out>, this=<optimized out>)
    at /ws/src/fields2cover/include/fields2cover/types/Geometries.h:23
#2  f2c::types::LinearRing::Geometry (this=0x7ffcdd2cbcc0) at /ws/src/fields2cover/include/fields2cover/types/LinearRing.h:21
#3  f2c::types::Cell::getInteriorRing (this=<optimized out>, i=<optimized out>) at /ws/src/fields2cover/src/fields2cover/types/Cell.cpp:115
#4  0x00007fb8191ce815 in conversor::ROS::to (_poly=..., _ros_poly=std::vector of length 4, capacity 4 = {...}) at /ws/src/fields2cover_ros/src/ros/conversor.cpp:33 **<this line number will differ on `master`>**

From what I understand , this function is not actually being used even in the ROS1 branch with the provided node on master, so I think it is ok living with this functionality not working for now.

Gonzalo-Mier commented 1 year ago

Hi @vinnnyr, My fault. Solved at commit cacfad3c8822c548497cdf92c6f3e080fbd78290 Thanks for mention it.

Gonzalo-Mier commented 1 year ago

Other thing, the second argument of ´conversor::ROS::to(F2CCell, std::vector)´ is expected to be empty, and will be filled inside the function. The test expect a move on the elements (checks are made on ´poly1´ and ´poly2´, which are empty also after the function.

vinnnyr commented 1 year ago

Ah yea makes sense. I at the moment did not have access to GDB to investigate further. I see you fixed the test as well in the ros2 support branch