Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#11906 closed Bugs (duplicate)

BOOST_TEST() violates expectations about expression lifetime

Reported by: bspencer@… Owned by: Raffi Enficiaud
Milestone: To Be Determined Component: test
Version: Boost 1.59.0 Severity: Problem
Keywords: Cc:

Description

The new BOOST_TEST() macro is a step in a great direction. However, because of how the macro expands to multiple expressions, it leads to simple checks performing undefined behaviour.

For example, consider the following complete test program:

#define BOOST_TEST_MAIN
#include <boost/test/included/unit_test.hpp>

struct Object
{
  Object()
    : m_value("A value for this string to have")
  {}

  const std::string &get() const
  { return m_value; }

private:
  std::string m_value;
};

BOOST_AUTO_TEST_CASE(test)
{
  BOOST_TEST(Object().get() == "check");
}

This program fails because only a "const std::string &" is captured for the left-hand side of the test, and then, in a separate expression (in the expanded BOOST_TEST() macro), that reference is used to evaluate the expression. This fails because the temporary Object has already been dereferenced.

Compare to BOOST_CHECK() with the same content (or BOOST_CHECK_EQUAL() with a comma instead of "==").

This kind of subtlety makes it difficult to write safe checks with BOOST_TEST().

Example valgrind output from the above program:

==24714== Invalid read of size 8
==24714==    at 0x4EF3C2E: std::string::compare(char const*) const (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20)
==24714==    by 0x414160: operator==<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2540)
==24714==    by 0x414160: eval (assertion.hpp:162)
==24714==    by 0x414160: value (assertion.hpp:348)
==24714==    by 0x414160: evaluate (assertion.hpp:357)
==24714==    by 0x414160: test::test_method() (foo.cc:19)
==24714==    by 0x414529: test_invoker() (foo.cc:17)
==24714==    by 0x4294BA: operator() (function_template.hpp:771)
==24714==    by 0x4294BA: operator() (execution_monitor.ipp:1304)
==24714==    by 0x4294BA: boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) (function_template.hpp:138)
==24714==    by 0x40ABEC: operator() (function_template.hpp:771)
==24714==    by 0x40ABEC: do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int()> > (execution_monitor.ipp:281)
==24714==    by 0x40ABEC: boost::execution_monitor::catch_signals(boost::function<int ()> const&) (execution_monitor.ipp:870)
==24714==    by 0x40AC98: boost::execution_monitor::execute(boost::function<int ()> const&) (execution_monitor.ipp:1207)
==24714==    by 0x40B376: boost::execution_monitor::vexecute(boost::function<void ()> const&) (execution_monitor.ipp:1313)
==24714==    by 0x410F31: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned int) (unit_test_monitor.ipp:46)
==24714==    by 0x43A332: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int) (framework.ipp:685)
==24714==    by 0x43A466: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int) (framework.ipp:636)
==24714==    by 0x41C4A1: boost::unit_test::framework::run(unsigned long, bool) (framework.ipp:1220)
==24714==    by 0x41C8AE: boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) (unit_test_main.ipp:228)
==24714==  Address 0x5a11a50 is 0 bytes inside a block of size 56 free'd
==24714==    at 0x4C2A360: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24714==    by 0x41433B: _M_dispose (lazy_ostream.hpp:35)
==24714==    by 0x41433B: ~basic_string (basic_string.h:547)
==24714==    by 0x41433B: ~Object (foo.cc:4)
==24714==    by 0x41433B: test::test_method() (foo.cc:19)
==24714==    by 0x414529: test_invoker() (foo.cc:17)
==24714==    by 0x4294BA: operator() (function_template.hpp:771)
==24714==    by 0x4294BA: operator() (execution_monitor.ipp:1304)
==24714==    by 0x4294BA: boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) (function_template.hpp:138)
==24714==    by 0x40ABEC: operator() (function_template.hpp:771)
==24714==    by 0x40ABEC: do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int()> > (execution_monitor.ipp:281)
==24714==    by 0x40ABEC: boost::execution_monitor::catch_signals(boost::function<int ()> const&) (execution_monitor.ipp:870)
==24714==    by 0x40AC98: boost::execution_monitor::execute(boost::function<int ()> const&) (execution_monitor.ipp:1207)
==24714==    by 0x40B376: boost::execution_monitor::vexecute(boost::function<void ()> const&) (execution_monitor.ipp:1313)
==24714==    by 0x410F31: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned int) (unit_test_monitor.ipp:46)
==24714==    by 0x43A332: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int) (framework.ipp:685)
==24714==    by 0x43A466: boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned int) (framework.ipp:636)
==24714==    by 0x41C4A1: boost::unit_test::framework::run(unsigned long, bool) (framework.ipp:1220)
==24714==    by 0x41C8AE: boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) (unit_test_main.ipp:228)

Change History (5)

comment:1 by Raffi Enficiaud, 7 years ago

Owner: changed from Gennadiy Rozental to Raffi Enficiaud
Status: newassigned

comment:2 by Raffi Enficiaud, 7 years ago

Would you please give a try to the branch fix/handling-rvalue-erasure? Thanks

comment:3 by bspencer@…, 7 years ago

I grabbed https://github.com/boostorg/test/commit/4124e3b754b2f524365fd420f6bad516b4445e99.patch and applied it to boost-1.59.0 and all seems well.

FWIW, the changes also makes sense to me ;)

Thanks for the fast patch!

comment:4 by Raffi Enficiaud, 7 years ago

Resolution: duplicate
Status: assignedclosed

Duplicates #11887, fixed in branch fix/handling-rvalue-erasure.

comment:5 by Raffi Enficiaud, 7 years ago

Thanks for the quick feedback!

Note: See TracTickets for help on using tickets.