Question about handling of array messages

[anhosi]

I am not sure whether this is the correct place for this issue. So please direct me to a better location.

Together with @Karsten1987 we stumbled over

rmw_fastrtps/rmw_fastrtps_dynamic_cpp/include/rmw_fastrtps_dynamic_cpp/TypeSupport_impl.hpp

Line 279 in 8f6a13c

auto vector = reinterpret_cast<std::vector<unsigned char> *>(field);

.

I have only limited understanding about the whole rmw. As far as I understand a array message member that is originally represented as a std::vector<T> * is reinterpret_casted to a std::vector<unsigned char> * with a void * as intermediate. IMHO this is not guaranteed to work by the C++ standard as implementations are allowed to use different template specializations of std::vector for different types. There are only guarantees for the memory layout of a vectors data().
As far as I can tell the above code is only the symptom as the void * field here is part of what is passed into rmw_serialize as ros_message. Maybe it would be better to pass only the data() pointer of the vector at some point instead of the whole vector as void *?

[deeplearningrobotics]

IMHO, if field was not originally cast to void * from a std::vector<unsigned char> * then this code yields undefined behavior according to the type aliasing rules.

https://en.cppreference.com/w/cpp/language/reinterpret_cast -> Type aliasing

[anhosi]

@deeplearningrobotics exactly. From my understanding field might very well have been a std::vector<float> for a float32[] message member.

[sloretz]

@richiprosima may I ask you to look at what it would take to fix this?

[deeplearningrobotics]

@richiware: Could you take a look a this?

Or is this covered under the standard?

Chapter 3.10 (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf )

[richiware]

I will try to make the changes next week. ↪ 2018-11-28 mié 22:32, deeplearningrobotics <notifications@github.com>:

…

@richiware: Could you take a look a this? Or is this covered under the standard? Chapter 3.10 (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3690.pdf ) -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #219 (comment)

[deeplearningrobotics]

@richiware: There are a lot of reinterpret_casts and also casts to and from void * which are undocumented. So I think there is a generally bigger work packages to verify their correctness and somehow document that those do not exhibit UB.

[ivanpauno]

Closing in favor of #428.
#429 is fixing this.