Bug in history_mgr::ouput caused crash (simple fix)

[Justin Hoffman]

Hey Dmitriy, thanks for the great tool and articles! I was having a crash and figured you would like to know about it.

In history_mgr::output a reference is declared to a history_entry in the exec_history_ vector:

history_entry const& ent = exec_history_[i];

Later the output_ function is called on ent:

ent.output_(stream, exec_history_[i].ev_);

The problem is that deep into the callstack of this function, history_mgr::exec_log was being called. history_mgr::exec_log calls push_back on exec_history_ and in the case where the vector needs to allocate more memory, invalidates "ent". So it would crash on the next line:

 

stream << ", in " << ent.info_ << std::endl;

To fix it I removed "ent" and just used exec_history_[i].

[Dmitry Vyukov]

Hi Justin Hoffman,

There is something seriously broken if output of an event triggers addition of a new event. Output of an event is a strictly internal thing, it should not affect user history in any way.

I can not reproduce it with darwin/g++ and my internal tests.

What platform and test are you using?

Thanks.

[Herb Little]

Hi,

I just stumbled onto this issue too. I am using MSVC2012, but here is my call stack showing exec_log() being called back within print_exec_history(). The output stream is re-sizing itself, which calls delete, which eventually calls exec_log().

I fixed this issue by changing line 182 of history.cpp from

    history_entry const& ent = exec_history_[i];

to

    history_entry const ent = exec_history_[i];

> Test.exe!rl::history_mgr::exec_log<rl::memory_free_event>(int th, const rl::debug_info & info, const rl::memory_free_event & ev, bool output_history) Line 111 C++

  Test.exe!rl::context::exec_log<rl::memory_free_event>(const rl::debug_info & info, const rl::memory_free_event & ev) Line 63 C++

  Test.exe!rl::context_impl<_test,rl::context_bound_scheduler<3> >::free(void * p) Line 356 C++

  Test.exe!operator delete(void * p) Line 1275 C++

  Test.exe!std::allocator<char>::deallocate(char * _Ptr, unsigned int __formal) Line 586 C++

  Test.exe!std::basic_stringbuf<char,std::char_traits<char>,std::allocator<char> >::overflow(int _Meta) Line 183 C++

  msvcp110d.dll!std::basic_streambuf<char,std::char_traits<char> >::xsputn(const char * _Ptr, __int64 _Count) Line 406 C++

  msvcp110d.dll!std::basic_streambuf<char,std::char_traits<char> >::sputn(const char * _Ptr, __int64 _Count) Line 203 C++

  Test.exe!std::operator<<<std::char_traits<char> >(std::basic_ostream<char,std::char_traits<char> > & _Ostr, const char * _Val) Line 807 C++

  Test.exe!rl::operator<<(std::basic_ostream<char,std::char_traits<char> > & ss, const rl::debug_info & info) Line 85 C++

  Test.exe!rl::history_mgr::output(unsigned int i) Line 185 C++

  Test.exe!rl::history_mgr::print_exec_history(bool output_history) Line 135 C++

  Test.exe!rl::context_impl<_test,rl::context_bound_scheduler<3> >::output_history() Line 742 C++

  Test.exe!rl::context_impl<_test,rl::context_bound_scheduler<3> >::simulate2(bool second) Line 631 C++

  Test.exe!rl::context_impl<_test,rl::context_bound_scheduler<3> >::simulate(std::basic_ostream<char,std::char_traits<char> > & ss, std::basic_istream<char,std::char_traits<char> > & sss, bool second) Line 603 C++

  Test.exe!rl::run_test<_test,rl::context_bound_scheduler<3> >(rl::test_params & params, std::basic_ostream<char,std::char_traits<char> > & oss, bool second) Line 904 C++

  Test.exe!rl::simulate<_test>(rl::test_params & params) Line 993 C++

  Test.exe!main() Line 423 C++

  Test.exe!__tmainCRTStartup() Line 536 C

  Test.exe!mainCRTStartup() Line 377 C

  kernel32.dll!761c33aa() Unknown

  [Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]

  ntdll.dll!77479f72() Unknown

  ntdll.dll!77479f45() Unknown

[Dmitry Vyukov]

Hi Herb,

I've pushed the change that should fix the issue:

https://code.google.com/p/relacy/source/detail?r=974f5c22847350de28739fba0bbc072153be6e4e

Now that Relacy lives on googlecode, you need to pull the tip version from https://code.google.com/p/relacy/source/checkout

Sorry that the fix took so long.

[Joe Best-Rotheray]

I still have this problem with all the current Relacy code I can find, either on github or by downloading from 1024cores, was this fix ever actually submitted?

[Dmitry Vyukov]

Hi Joe,

The fix for that issue is submitted to github in 2013:
https://github.com/dvyukov/relacy/commit/0988bb774afba7c31570be7b7287becc06e330ce

The proposed fix:
- history_entry const& ent = exec_history_[i];
+ history_entry const ent = exec_history_[i];
only hides the real issue -- which is calling malloc from within relacy guts.

I guess you've hit another such issue. Can you figure out where we
call malloc after the "history_entry const& ent = exec_history_[i]"
line? It should be fixed along the lines of:
https://github.com/dvyukov/relacy/commit/0988bb774afba7c31570be7b7287becc06e330ce

Thanks.

> --
> You received this message because you are subscribed to the Google Groups
> "Relacy Race Detector" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to relacy+un...@googlegroups.com.
> To post to this group, send email to rel...@googlegroups.com.
> Visit this group at https://groups.google.com/group/relacy.
> For more options, visit https://groups.google.com/d/optout.



--
Dmitry Vyukov

All about lockfree/waitfree algorithms, multicore, scalability,
parallel computing and related topics:
http://www.1024cores.net

[Joe Best-Rotheray]

Ah I see, it's fixed in Github but not in the download from 1024cores. I thought it wasn't fixed in Github either as I was looking at the history_entry const& ent = exec_history_[i] line! Never mind!