Remove fixed size tid array #1
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Donald,
du bist ein Held! Wer hat schon mehr als 64 cores, wer braucht mehr als 640k RAM?
Nb: nachdem ich mir noch die defines angeschaut hatte, und nix merkwürdiges gesehn hatte, hab ich für mich das thema abgehakt.....
Thomas
…On October 7, 2017 4:08:55 PM GMT+02:00, Donald Buczek ***@***.***> wrote:
This fixed size array, which overflows when more than 64 threads
are requested, is not used. Remove it.
This might fix the problem with running this program with to many
threads.
You can view, comment on, or merge this pull request online at:
#1
-- Commit Summary --
* main.cpp: Remove fixed size tid array
-- File Changes --
M main.cpp (9)
-- Patch Links --
https://github.molgen.mpg.de/molgen/bsmap/pull/1.patch
https://github.molgen.mpg.de/molgen/bsmap/pull/1.diff
--
Thomas Kreitler - Information Retrieval
kreitler@molgen.mpg.de
49/30/8413 1702
|
Member
Author
|
Where can I pick up my prize? |
Donald Buczek (donald)
added a commit
that referenced
this pull request
Oct 8, 2017
There is some a global variable read_time which is updated by
all threads without proper synchronization.
WARNING: ThreadSanitizer: data race (pid=15896)
Read of size 4 at 0x00000066d53c by thread T7:
#0 t_PairAlign(void*) /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:118 (bsmap+0x00000042c4f2)
Previous write of size 4 at 0x00000066d53c by thread T6:
[failed to restore the stack]
Location is global 'read_time' of size 4 at 0x00000066d53c (bsmap+0x00000066d53c)
Thread T7 (tid=15960, running) created by main thread at:
#0 pthread_create /scratch/local/bee-root/gcc/gcc-5.3.0-0/source/libsanitizer/tsan/tsan_interceptors.cc:895 (libtsan.so.0+0x000000026c44)
#1 Do_PairAlign() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:128 (bsmap+0x00000042c613)
#2 RunProcess() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:499 (bsmap+0x0000004306b0)
#3 main /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:611 (bsmap+0x0000004320ae)
Thread T6 (tid=15959, finished) created by main thread at:
#0 pthread_create /scratch/local/bee-root/gcc/gcc-5.3.0-0/source/libsanitizer/tsan/tsan_interceptors.cc:895 (libtsan.so.0+0x000000026c44)
#1 Do_PairAlign() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:128 (bsmap+0x00000042c613)
#2 RunProcess() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:499 (bsmap+0x0000004306b0)
#3 main /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:611 (bsmap+0x0000004320ae)
SUMMARY: ThreadSanitizer: data race /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:118 t_PairAlign(void*)
==================
This is not a real issue, because the value is not currently
used. But because we are holding the mutex_fout mutex at this point
in code anyway, swap the order and work in the global first, then
release the mutex. This puts read_time under the protection of
mutex_fout.
Collaborator
|
Very nice. Tested on fluffybutt with 98 threads. Seems to work. I’d mention the address sanitizer in the commit message. |
This fixed size array, which overflows when more than 64 threads
are requested, is not used. Remove it.
Bug identified by address sanitizer:
#0 0x449813 in Do_PairAlign() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:129
#1 0x4527e4 in RunProcess() /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:498
#2 0x40613c in main /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:609
#3 0x7f0dff71803f in __libc_start_main ../csu/libc-start.c:308
#4 0x40a0c9 in _start (/scratch/cluster02/buczek/bsmap/bsmap.git/bsmap+0x40a0c9)
0x00000069b9e0 is located 32 bytes to the left of global variable 'read_time' defined in 'main.cpp:45:19' (0x69ba00) of size 4
0x00000069b9e0 is located 0 bytes to the right of global variable 'tid' defined in 'main.cpp:46:9' (0x69b960) of size 128
SUMMARY: AddressSanitizer: global-buffer-overflow /scratch/cluster/buczek/bsmap/bsmap.git/main.cpp:129 in Do_PairAlign()
Shadow bytes around the buggy address:
0x0000800cb6e0: 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00
0x0000800cb6f0: 00 f9 f9 f9 f9 f9 f9 f9 00 00 00 00 00 00 00 00
0x0000800cb700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0000800cb710: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0000800cb720: 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 00 00 00 00
=>0x0000800cb730: 00 00 00 00 00 00 00 00 00 00 00 00[f9]f9 f9 f9
0x0000800cb740: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0000800cb750: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0000800cb760: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0000800cb770: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
0x0000800cb780: 04 f9 f9 f9 f9 f9 f9 f9 04 f9 f9 f9 f9 f9 f9 f9
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==6441==ABORTING
Donald Buczek (donald)
force-pushed
the
remove-tid
branch
from
ff870a6 to
9075ce8
Compare
|
🏆 |
Contributor
|
Also checked a bit, bsmap gives reproducable output with 12 and 120 threads. NB. |
Sign in
to join this conversation on GitHub.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixed size array, which overflows when more than 64 threads
are requested, is not used. Remove it.
This might fix the problem with running this program with to many threads.