Project

General

Profile

Actions

Bug #3324

closed

lp::Packet::wireEncode() has nonsensical return type

Added by Davide Pesavento almost 9 years ago. Updated almost 9 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Base
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Actions #1

Updated by Davide Pesavento almost 9 years ago

  • Description updated (diff)
Actions #2

Updated by Davide Pesavento almost 9 years ago

  • Description updated (diff)
Actions #3

Updated by Eric Newberry almost 9 years ago

  • Status changed from New to In Progress

After looking at this again, I agree that it makes no sense. I believe that removing the const works better. I tried making this change and all of the test cases passed. When changing it to const Block&, the test cases encountered a memory access violation.

Actions #4

Updated by Davide Pesavento almost 9 years ago

What test cases?

Actions #5

Updated by Eric Newberry almost 9 years ago

Davide Pesavento wrote:

What test cases?

The ndn-cxx link protocol test cases.

Actions #6

Updated by Eric Newberry almost 9 years ago

  • Status changed from In Progress to Code review
Actions #7

Updated by Davide Pesavento almost 9 years ago

Eric Newberry wrote:

When changing it to const Block&, the test cases encountered a memory access violation.

It crashes with const Block& because Packet::wireEncode returns a reference to an object that is about to be destructed:

  Block::element_container elements = m_wire.elements();
  if (elements.size() == 1 && elements.front().type() == FragmentField::TlvType::value) {
    elements.front().parse();
    elements.front().elements().front().parse();
    return elements.front().elements().front();
  }

For the record, this is what valgrind says about LpPacket/DecodeBareNetworkLayerPacket:

==4802== Invalid read of size 8
==4802==    at 0x47DBEE: operator= (shared_ptr_base.h:867)
==4802==    by 0x47DBEE: operator= (shared_ptr.h:93)
==4802==    by 0x47DBEE: ndn::Block::operator=(ndn::Block const&) (block.hpp:43)
==4802==    by 0x51C2CC: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket::test_method() (packet.t.cpp:323)
==4802==    by 0x51C784: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket_invoker() (packet.t.cpp:307)
==4802==    by 0x477192: invoke<void (*)()> (callback.hpp:56)
==4802==    by 0x477192: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==4802==    by 0x5BA5CD0: ??? (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B858B5: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B860D2: boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BA5E01: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B8CFBD: boost::unit_test::framework_impl::visit(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BC348A: boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&, boost::unit_test::test_tree_visitor&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BC348A: boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&, boost::unit_test::test_tree_visitor&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B88915: boost::unit_test::framework::run(unsigned long, bool) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==  Address 0xa3654f0 is 0 bytes inside a block of size 88 free'd
==4802==    at 0x4C2D28B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4802==    by 0x500A21D: deallocate (new_allocator.h:110)
==4802==    by 0x500A21D: deallocate (alloc_traits.h:386)
==4802==    by 0x500A21D: _M_deallocate (stl_vector.h:178)
==4802==    by 0x500A21D: ~_Vector_base (stl_vector.h:160)
==4802==    by 0x500A21D: ~vector (stl_vector.h:425)
==4802==    by 0x500A21D: ~Block (block.hpp:43)
==4802==    by 0x500A21D: _Destroy<ndn::Block> (stl_construct.h:93)
==4802==    by 0x500A21D: __destroy<ndn::Block*> (stl_construct.h:103)
==4802==    by 0x500A21D: _Destroy<ndn::Block*> (stl_construct.h:126)
==4802==    by 0x500A21D: _Destroy<ndn::Block*, ndn::Block> (stl_construct.h:151)
==4802==    by 0x500A21D: ~vector (stl_vector.h:424)
==4802==    by 0x500A21D: ndn::lp::Packet::wireEncode() const (packet.cpp:74)
==4802==    by 0x51C2C0: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket::test_method() (packet.t.cpp:323)
==4802==    by 0x51C784: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket_invoker() (packet.t.cpp:307)
==4802==    by 0x477192: invoke<void (*)()> (callback.hpp:56)
==4802==    by 0x477192: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==4802==    by 0x5BA5CD0: ??? (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B858B5: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B860D2: boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BA5E01: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B8CFBD: boost::unit_test::framework_impl::visit(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BC348A: boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&, boost::unit_test::test_tree_visitor&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BC348A: boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&, boost::unit_test::test_tree_visitor&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==  Block was alloc'd at
==4802==    at 0x4C2C12F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4802==    by 0x4FC8D4C: allocate (new_allocator.h:104)
==4802==    by 0x4FC8D4C: allocate (alloc_traits.h:360)
==4802==    by 0x4FC8D4C: _M_allocate (stl_vector.h:170)
==4802==    by 0x4FC8D4C: void std::vector<ndn::Block, std::allocator<ndn::Block> >::_M_emplace_back_aux<ndn::Block>(ndn::Block&&) (vector.tcc:412)
==4802==    by 0x4FC9114: void std::vector<ndn::Block, std::allocator<ndn::Block> >::emplace_back<ndn::Block>(ndn::Block&&) (vector.tcc:101)
==4802==    by 0x4FC2DA0: push_back (stl_vector.h:932)
==4802==    by 0x4FC2DA0: ndn::Block::parse() const (block.cpp:347)
==4802==    by 0x50095EF: ndn::lp::Packet::wireEncode() const (packet.cpp:76)
==4802==    by 0x51C2C0: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket::test_method() (packet.t.cpp:323)
==4802==    by 0x51C784: ndn::lp::tests::LpPacket::DecodeBareNetworkLayerPacket_invoker() (packet.t.cpp:307)
==4802==    by 0x477192: invoke<void (*)()> (callback.hpp:56)
==4802==    by 0x477192: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==4802==    by 0x5BA5CD0: ??? (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B858B5: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5B860D2: boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==4802==    by 0x5BA5E01: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)

[several other similar errors follow]
Actions #8

Updated by Junxiao Shi almost 9 years ago

  • Status changed from Code review to Closed
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF