[PATCH] gold: Avoid sharing Plugin_list::iterator

Fri Nov 6 02:35:56 GMT 2020

On Thu, Nov 05, 2020 at 05:46:46PM -0800, H.J. Lu wrote:
> On Thu, Nov 5, 2020 at 4:42 PM Alan Modra <amodra@gmail.com> wrote:
> > HJ, I don't need to be told why sharing variables among threads can go
> > wrong.  I understand that quite well.  I was asking for the specifics
> > of what was going wrong.  Now if you haven't analysed the problem to
> > that level, that's fine, just say so.
> 
> I didn't know what went wrong.

OK, I just tried clang --sanitize=thread and see the following.  It
looks like Plugin_manager::claim_file and
Plugin_manager::all_symbols_read might be running simultaneously.
This was with commit 712b8a0f68f binutils source.

That in itself is interesting, and says to me that
Plugin_manager::all_symbols_read is running too soon.

==================
WARNING: ThreadSanitizer: data race (pid=10780)
  Write of size 1 at 0x000001811040 by main thread:
    #0 gold::Target_selector_nacl<(anonymous namespace)::Target_selector_x86_64<64>, (anonymous namespace)::Target_x86_64_nacl<64> >::do_recognize(gold::Input_file*, long, int, int, int) /home/alan/src/binutils-gdb/gold/nacl.h:116:20 (ld-new+0x4e715c)
    #1 gold::Target_selector::recognize(gold::Input_file*, long, int, int, int) /home/alan/src/binutils-gdb/gold/target-select.h:82:18 (ld-new+0x89a917)
    #2 gold::select_target(gold::Input_file*, long, int, int, bool, int, int) /home/alan/src/binutils-gdb/gold/target-select.cc:112:21 (ld-new+0x89a3d1)
    #3 gold::Object* (anonymous namespace)::make_elf_sized_object<64, false>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, gold::Input_file*, long, elfcpp::Ehdr<64, false> const&, bool*) /home/alan/src/binutils-gdb/gold/object.cc:3380:20 (ld-new+0x78e829)
    #4 gold::make_elf_object(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, gold::Input_file*, long, unsigned char const*, long, bool*) /home/alan/src/binutils-gdb/gold/object.cc:3504:11 (ld-new+0x78e1d5)
    #5 gold::Read_symbols::do_read_symbols(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:336:17 (ld-new+0x826710)
    #6 gold::Read_symbols::run(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:167:14 (ld-new+0x8264b2)
    #7 gold::Workqueue::find_and_run_task(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:319:10 (ld-new+0x89b6cb)
    #8 gold::Workqueue::process(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:495:16 (ld-new+0x89bc19)
    #9 main /home/alan/src/binutils-gdb/gold/main.cc:252:13 (ld-new+0x4b7ca6)

  Previous write of size 1 at 0x000001811040 by thread T1:
    #0 gold::Target_selector_nacl<(anonymous namespace)::Target_selector_x86_64<64>, (anonymous namespace)::Target_x86_64_nacl<64> >::do_recognize(gold::Input_file*, long, int, int, int) /home/alan/src/binutils-gdb/gold/nacl.h:116:20 (ld-new+0x4e715c)
    #1 gold::Target_selector::recognize(gold::Input_file*, long, int, int, int) /home/alan/src/binutils-gdb/gold/target-select.h:82:18 (ld-new+0x89a917)
    #2 gold::select_target(gold::Input_file*, long, int, int, bool, int, int) /home/alan/src/binutils-gdb/gold/target-select.cc:112:21 (ld-new+0x89a3d1)
    #3 gold::Object* (anonymous namespace)::make_elf_sized_object<64, false>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, gold::Input_file*, long, elfcpp::Ehdr<64, false> const&, bool*) /home/alan/src/binutils-gdb/gold/object.cc:3380:20 (ld-new+0x78e829)
    #4 gold::make_elf_object(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, gold::Input_file*, long, unsigned char const*, long, bool*) /home/alan/src/binutils-gdb/gold/object.cc:3504:11 (ld-new+0x78e1d5)
    #5 gold::Read_symbols::do_read_symbols(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:336:17 (ld-new+0x826710)
    #6 gold::Read_symbols::run(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:167:14 (ld-new+0x8264b2)
    #7 gold::Workqueue::find_and_run_task(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:319:10 (ld-new+0x89b6cb)
    #8 gold::Workqueue::process(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:495:16 (ld-new+0x89bc19)
    #9 gold::Workqueue_threader_threadpool::process(int) /home/alan/src/binutils-gdb/gold/workqueue-internal.h:92:28 (ld-new+0x89cb93)
    #10 gold::Workqueue_thread::thread_body(void*) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:117:21 (ld-new+0x89c82f)

  Location is global '(anonymous namespace)::target_selector_x86_64' of size 152 at 0x000001810fd8 (ld-new+0x000001811040)

  Thread T1 (tid=10782, running) created by main thread at:
    #0 pthread_create <null> (ld-new+0x42837b)
    #1 gold::Workqueue_thread::Workqueue_thread(gold::Workqueue_threader_threadpool*, int) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:86:9 (ld-new+0x89c759)
    #2 gold::Workqueue_threader_threadpool::set_thread_count(int) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:168:8 (ld-new+0x89ca53)
    #3 gold::Workqueue::set_thread_count(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:507:20 (ld-new+0x89bc86)
    #4 gold::queue_initial_tasks(gold::General_options const&, gold::Dirsearch&, gold::Command_line const&, gold::Workqueue*, gold::Input_objects*, gold::Symbol_table*, gold::Layout*, gold::Mapfile*) /home/alan/src/binutils-gdb/gold/gold.cc:196:14 (ld-new+0x72dd82)
    #5 main /home/alan/src/binutils-gdb/gold/main.cc:247:3 (ld-new+0x4b7c93)

SUMMARY: ThreadSanitizer: data race /home/alan/src/binutils-gdb/gold/nacl.h:116:20 in gold::Target_selector_nacl<(anonymous namespace)::Target_selector_x86_64<64>, (anonymous namespace)::Target_x86_64_nacl<64> >::do_recognize(gold::Input_file*, long, int, int, int)
==================
==================
WARNING: ThreadSanitizer: data race (pid=10780)
  Write of size 8 at 0x7b4800000018 by main thread (mutexes: write M66):
    #0 gold::Plugin_manager::claim_file(gold::Input_file*, long, long, gold::Object*) /home/alan/src/binutils-gdb/gold/plugin.cc:758:23 (ld-new+0x81e7c0)
    #1 gold::Read_symbols::do_read_symbols(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:343:57 (ld-new+0x8267b0)
    #2 gold::Read_symbols::run(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:167:14 (ld-new+0x8264b2)
    #3 gold::Workqueue::find_and_run_task(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:319:10 (ld-new+0x89b6cb)
    #4 gold::Workqueue::process(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:495:16 (ld-new+0x89bc19)
    #5 main /home/alan/src/binutils-gdb/gold/main.cc:252:13 (ld-new+0x4b7ca6)

  Previous write of size 8 at 0x7b4800000018 by thread T1:
    #0 std::__1::__list_iterator<gold::Plugin*, void*>::operator++() /usr/include/c++/v1/list:369:16 (ld-new+0x81ef5d)
    #1 gold::Plugin_manager::all_symbols_read(gold::Workqueue*, gold::Task*, gold::Input_objects*, gold::Symbol_table*, gold::Dirsearch*, gold::Mapfile*, gold::Task_token**) /home/alan/src/binutils-gdb/gold/plugin.cc:855:8 (ld-new+0x81ef5d)
    #2 gold::Plugin_hook::run(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/plugin.cc:1770:29 (ld-new+0x820f6c)
    #3 gold::Workqueue::find_and_run_task(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:319:10 (ld-new+0x89b6cb)
    #4 gold::Workqueue::process(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:495:16 (ld-new+0x89bc19)
    #5 gold::Workqueue_threader_threadpool::process(int) /home/alan/src/binutils-gdb/gold/workqueue-internal.h:92:28 (ld-new+0x89cb93)
    #6 gold::Workqueue_thread::thread_body(void*) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:117:21 (ld-new+0x89c82f)

  Location is heap block of size 368 at 0x7b4800000000 allocated by main thread:
    #0 operator new(unsigned long) <null> (ld-new+0x4b528b)
    #1 gold::General_options::add_plugin(char const*) /home/alan/src/binutils-gdb/gold/options.cc:1075:22 (ld-new+0x7b994a)
    #2 gold::General_options::parse_plugin(char const*, char const*, gold::Command_line*) /home/alan/src/binutils-gdb/gold/options.cc:455:9 (ld-new+0x7b98ee)
    #3 gold::options::Struct_special::parse_to_value(char const*, char const*, gold::Command_line*, gold::General_options*) /home/alan/src/binutils-gdb/gold/options.h:249:5 (ld-new+0x4c2a8f)
    #4 gold::Command_line::process_one_option(int, char const**, int, bool*) /home/alan/src/binutils-gdb/gold/options.cc:1571:23 (ld-new+0x7c055b)
    #5 gold::Command_line::process(int, char const**) /home/alan/src/binutils-gdb/gold/options.cc:1613:6 (ld-new+0x7c0c91)
    #6 main /home/alan/src/binutils-gdb/gold/main.cc:165:16 (ld-new+0x4b790d)

  Mutex M66 (0x7b0c0003e468) created at:
    #0 pthread_mutex_init <null> (ld-new+0x4298fd)
    #1 gold::Lock_impl_threads::Lock_impl_threads() /home/alan/src/binutils-gdb/gold/gold-threads.cc:110:9 (ld-new+0x73100c)
    #2 gold::Lock::Lock() /home/alan/src/binutils-gdb/gold/gold-threads.cc:153:25 (ld-new+0x731287)
    #3 gold::Initialize_lock::do_run_once(void*) /home/alan/src/binutils-gdb/gold/gold-threads.cc:447:24 (ld-new+0x731a56)
    #4 gold::Once::internal_run(void*) /home/alan/src/binutils-gdb/gold/gold-threads.cc:421:9 (ld-new+0x731947)
    #5 gold::c_run_once() /home/alan/src/binutils-gdb/gold/gold-threads.cc:328:17 (ld-new+0x7319b4)
    #6 pthread_once <null> (ld-new+0x42b485)
    #7 gold::Once::run_once(void*) /home/alan/src/binutils-gdb/gold/gold-threads.cc:401:9 (ld-new+0x73183a)
    #8 gold::Initialize_lock::initialize() /home/alan/src/binutils-gdb/gold/gold-threads.cc:437:13 (ld-new+0x731a06)
    #9 gold::Plugin_manager::claim_file(gold::Input_file*, long, long, gold::Object*) /home/alan/src/binutils-gdb/gold/plugin.cc:742:50 (ld-new+0x81e623)
    #10 gold::Read_symbols::do_read_symbols(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:343:57 (ld-new+0x8267b0)
    #11 gold::Read_symbols::run(gold::Workqueue*) /home/alan/src/binutils-gdb/gold/readsyms.cc:167:14 (ld-new+0x8264b2)
    #12 gold::Workqueue::find_and_run_task(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:319:10 (ld-new+0x89b6cb)
    #13 gold::Workqueue::process(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:495:16 (ld-new+0x89bc19)
    #14 main /home/alan/src/binutils-gdb/gold/main.cc:252:13 (ld-new+0x4b7ca6)

  Thread T1 (tid=10782, running) created by main thread at:
    #0 pthread_create <null> (ld-new+0x42837b)
    #1 gold::Workqueue_thread::Workqueue_thread(gold::Workqueue_threader_threadpool*, int) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:86:9 (ld-new+0x89c759)
    #2 gold::Workqueue_threader_threadpool::set_thread_count(int) /home/alan/src/binutils-gdb/gold/workqueue-threads.cc:168:8 (ld-new+0x89ca53)
    #3 gold::Workqueue::set_thread_count(int) /home/alan/src/binutils-gdb/gold/workqueue.cc:507:20 (ld-new+0x89bc86)
    #4 gold::queue_initial_tasks(gold::General_options const&, gold::Dirsearch&, gold::Command_line const&, gold::Workqueue*, gold::Input_objects*, gold::Symbol_table*, gold::Layout*, gold::Mapfile*) /home/alan/src/binutils-gdb/gold/gold.cc:196:14 (ld-new+0x72dd82)
    #5 main /home/alan/src/binutils-gdb/gold/main.cc:247:3 (ld-new+0x4b7c93)

SUMMARY: ThreadSanitizer: data race /home/alan/src/binutils-gdb/gold/plugin.cc:758:23 in gold::Plugin_manager::claim_file(gold::Input_file*, long, long, gold::Object*)
==================
ThreadSanitizer: reported 2 warnings
collect2: error: ld returned 66 exit status

-- 
Alan Modra
Australia Development Lab, IBM