Closed fsmosca closed 7 years ago
I think the cause for crashing is clear unlike that for multiple paths. asmFish expects a single path as was in the original syzygy implementation. When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths. So the engine stalls. In Marco's rewrite of the original syzygy code, there is a piece that allows multiple syzygy paths, separated by a semi-colon (on Windows) or colon (in Linux):
public:
// Look for and open the file among the Paths directories where the .rtbw
// and .rtbz files can be found. Multiple directories are separated by ";"
// on Windows and by ":" on Unix-based operating systems.
//
// Example:
// C:\tb\wdl345;C:\tb\wdl6;D:\tb\dtz345;D:\tb\dtz6
static std::string Paths;
TBFile(const std::string& f) {
#ifndef _WIN32
const char SepChar = ':';
#else
const char SepChar = ';';
#endif
std::stringstream ss(Paths);
std::string path;
while (std::getline(ss, path, SepChar)) {
fname = path + "/" + f;
std::ifstream::open(fname);
if (is_open())
return;
}
When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths.
When I tried a path with ';' it works fine, it found the TB's.
id name asmFishW_2017-03-30_popcnt
[...]
setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1
info string found 145 tablebases
However adding additional ';' at the end,
setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1;
and the engine would crash.
Just out of curiosity. Is it possible the TB's it found are just from the 1st path (C:\chess\syzygy) and not both?
From: fsmosca notifications@github.com Sent: Friday, March 31, 2017 9:02:22 AM To: lantonov/asmFish Cc: Subscribed Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42)
When the engine finds ';' in the path, it doesn't know what to do with it as this symbol is forbidden in paths.
When I tried a path with ';' it works fine, it found the TB's.
id name asmFishW_2017-03-30_popcnt
[...]
setoption name syzygypath value C:\chess\syzygy;C:\chess\syzygy1
info string found 145 tablebases
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/lantonov/asmFish/issues/42#issuecomment-290705843, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ATYgIyQ1xlzUbfMrfaDAJHmHKVBBPc8Kks5rrPlegaJpZM4MvHD0.
In my example, the second path is just a dummy, only the first path has the TB's.
That may be but it'd be good to confirm that when we use a path like you have which is also what I use except I have three paths in that string separated by a ";" then I wonder if all 3 paths are actually being used. I mean it's possible that we think TB's are working but perhaps only one of those paths is and not the rest.
I realize this is a separate issue (if there is one) than the ; crash bug but it made me think and wonder if all paths in the Syzygy Path string are working as expected.
From: fsmosca notifications@github.com Sent: Friday, March 31, 2017 9:32:29 AM To: lantonov/asmFish Cc: hero2017; Comment Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42)
In my example, the second path is just a dummy, only the first path has the TB's.
— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/lantonov/asmFish/issues/42#issuecomment-290712735, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ATYgIwqjubhn81a-dSwif_SSVQLKQ6YXks5rrQBtgaJpZM4MvHD0.
I think this is not difficult to verify if the path is being considered or not.
I will look at this in a day. Obviously, the code that needs to be checked out is around https://github.com/lantonov/asmFish/blob/master/asmFish/guts/TablebaseCore.asm#L1938
SEP_CHAR is semi-colon in Windows and colon in Linux according to this piece of asmFish code:
match ='W', VERSION_OS {
SEP_CHAR equ ';'
}
match ='L', VERSION_OS {
SEP_CHAR equ ':'
}
Obviously, I have been wrong to think that asmFish doesn't have code for multiple folders. TablebaseCore.asm is the only code that is not created anew but from disassembly. I'm sure that @tthsqe12 will reach to the bottom of it, however.
Confirming the above:
setoption name syzygypath value C:\Winboard\Syzygy
info string found 145 tablebases
setoption name syzygypath value C:\Winboard\Syzygy\
info string found 145 tablebases
setoption name syzygypath value "C:\Winboard\Syzygy"
info string found 0 tablebases
setoption name syzygypath value C:\;C:\Winboard\Syzygy
info string found 145 tablebases
setoption name syzygypath value C:\Winboard\Syzygy;
The last command crashes the engine. However, tablebases in multiple paths are recognized, e.g. with C:\;C:\Winboard\Syzygy the tablebases are in the second path.
I am surprised by this.
setoption name syzygypath value "C:\Winboard\Syzygy" info string found 0 tablebases
So am I
Some more (right slash). The last crashes
setoption name syzygypath value C:/Winboard/Syzygy
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy/
info string found 145 tablebases
setoption name syzygypath value "C:/Winboard/Syzygy/"
info string found 0 tablebases
setoption name syzygypath value "C:/Winboard/Syzygy;"
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:/
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C::
info string found 145 tablebases
setoption name syzygypath value C:/Winboard/Syzygy/:
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy:
info string found 0 tablebases
setoption name syzygypath value C:/Winboard/Syzygy;C:;
alright, the issue is with https://github.com/syzygy1/Cfish/blob/master/src/tbcore.c#L306 if you give it something like path1;path2; the code is going to start searching for a path after the last SEP_CHAR. In this case, there is not one and so the code will start reading past the buffer,
Verdict: both asmFish and Cfish suffer from a buffer overflow with setoption name syzygypath value path1;path2; Cfish uses the std malloc which is almost guaranteed to have non-zero bytes past the allocated space. In this case L306 stops at this point. However, asmFish has a malloc that will almost certainly have zero bytes past the allocated space. In this case, L306 loops until it falls off that page, at which point it will almost certainly seg fault.
a command such as setoption name syzygypath value path1;path2; is not 'legal'. The parsing in tbcore is very simple and expects one SEP_CHAR between the paths.
I could try to fool-proof the parsing of syzygypath. At least it could print the number of tbs found in each path instead of just the total number of found tbs.
Let me know what you all think.
It's not a big issue. If one wants to crash the engine, he can do it in a million ways. If fool-proofing the parsing doesn't require a lot of work on your part, then do it. Otherwise, leave it so.
Probably easiest solution and it makes sense if I understood you. Simply not allowing us to give it a syzygy path that ends in ";" should do the trick although I never enter the path with a ; and so it was never a problem for me. Like lantonov said, if it's not a lot of work sure otherwise I wouldn't bother.
The total number of TB's for each path in the string is a nice touch too...it'd be even better if we knew whether the total # it gives us for each path is correct but that may require too much effort. But this way we'd know if one of the TB files is perhaps corrupted or not.
I have a patch with the following behavior. Let me know if this seems acceptable.
setoption name syzygypath value E:\syzygy5; E:\syzygy6;
info string found 2 of 510 tablebases in E:\syzygy5
info string found 1 of 510 tablebases in E:\syzygy6
info string found 0 of 510 tablebases in
info string found 3 tablebases
setoption name syzygypath value E:\syzygy5; E:\syzygy6
info string found 2 of 510 tablebases in E:\syzygy5
info string found 1 of 510 tablebases in E:\syzygy6
info string found 3 tablebases
setoption name syzygypath value ;;E:\syzygy5; E:\syzygy6; ; ;
info string found 0 of 510 tablebases in
info string found 0 of 510 tablebases in
info string found 2 of 510 tablebases in E:\syzygy5
info string found 1 of 510 tablebases in E:\syzygy6
info string found 0 of 510 tablebases in
info string found 0 of 510 tablebases in
info string found 0 of 510 tablebases in
info string found 3 tablebases
The patch does what is intended though I can't understand what is counted. From the messages it is understood that there is a maximum of 510 TB, in E:\syzygy5 we have 2/510 TB and in E:\syzygy6 it's only 1/510 TB. Is this the real situation?
I have a patch with the following behavior. Let me know if this seems acceptable.
setoption name syzygypath value E:\syzygy5; E:\syzygy6; info string found 2 of 510 tablebases in E:\syzygy5 info string found 1 of 510 tablebases in E:\syzygy6 info string found 0 of 510 tablebases in info string found 3 tablebases ...
info string found a total of 3 tablebases
That looks fine.
Even the following should also be fine.
info string found 2 tablebases in E:\syzygy5
I only have 5-men so in that case
info string found 145 of 510 tablebases in E:\syzygy5
In the future for 7-men, that 510 (if that is for 6-men) would change.
My 5 men syzygy have 290 files, not 145 and I'm quite sure 290 is correct assuming that each TB found means one file. And my 6-men have 730 files. Total of all 5 and 6-men are 1020 files. If that's not how it works then I don't know what is correct.
And outputting 2 of 510 TB's seems to suggest that there are 508 missing in E:\syzygy5. I believe if we're talking about 2 PATHS in the "value" then yes that is correct but I don't think printing 2 of 510 is correct info based on the wording.
@hero2017
According to TB author 1 tablebase = 2 files So for a complete 5-men there are 145 tablebases and 145x2 = 290 files.
Perhaps it is better to just mention the number of tablebases, i.e.
setoption name syzygypath value E:\syzygy5; E:\syzygy6
info string found 145 tablebases in e:\syzygy5
info string found 365 tablebases in e:\syzygy6
info string found a total of 510 tablebases
I support the above suggestion. It's simpler and less confusing.
Thanks for explaining that fsmosca. It's all crystal clear now.
I too support the above suggestion. It's perfect.
From: fsmosca notifications@github.com Sent: Monday, April 3, 2017 12:25:09 AM To: lantonov/asmFish Cc: hero2017; Mention Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42)
@hero2017https://github.com/hero2017
According to TB author 1 tablebase = 2 files So for a complete 5-men there are 145 tablebases and 145x2 = 290 files.
Perhaps it is better to just mention the number of tablebases, i.e.
setoption name syzygypath value E:\syzygy5; E:\syzygy6 info string found 145 tablebases in e:\syzygy5 info string found 365 tablebases in e:\syzygy6 info string found a total of 510 tablebases
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/lantonov/asmFish/issues/42#issuecomment-291046005, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ATYgIwVW-4FVKYF-HYGLdhw0qpj3CmQrks5rsHSlgaJpZM4MvHD0.
this the patch that i am going to commit to my fork leading spaces are trimmed off (see second example) spaces at the end or before the SEP_CHAR are not trimmed (see fourth example) extra SEP_CHAR's cause at worst empty path strings. This is compatible with previous parsing of legal commands and slightly more forgiving. The surgeon general recommends the first or second commands
flat assembler version 1.71.54 (16384 kilobytes memory)
4 passes, 0.1 seconds, 113630 bytes.
bash-4.2$ ./pfish
pedantFishL_2017-04-03_popcnt
setoption name syzygypath value /home/me/syzygy345:/home/me/syzygy6
info string found 5 tablebases in "/home/me/syzygy345"
info string found 2 tablebases in "/home/me/syzygy6"
info string found 7 of 510 tablebases
setoption name syzygypath value /home/me/syzygy345: /home/me/syzygy6
info string found 5 tablebases in "/home/me/syzygy345"
info string found 2 tablebases in "/home/me/syzygy6"
info string found 7 of 510 tablebases
setoption name syzygypath value /home/me/syzygy345: /home/me/syzygy6:
info string found 5 tablebases in "/home/me/syzygy345"
info string found 2 tablebases in "/home/me/syzygy6"
info string found 0 tablebases in ""
info string found 7 of 510 tablebases
setoption name syzygypath value /home/me/syzygy345 : /home/me/syzygy6
info string found 0 tablebases in "/home/me/syzygy345 "
info string found 2 tablebases in "/home/me/syzygy6"
info string found 2 of 510 tablebases
setoption name syzygypath value :/home/me/syzygy345: /home/me/syzygy6: ::
info string found 0 tablebases in ""
info string found 5 tablebases in "/home/me/syzygy345"
info string found 2 tablebases in "/home/me/syzygy6"
info string found 0 tablebases in ""
info string found 0 tablebases in ""
info string found 0 tablebases in ""
info string found 7 of 510 tablebases
Great, more added value to AF! Executable with this patch: 2017-04-03 clean up parsing of syzygypath bug fix of Issue #42, bench 6861050
Looks like there is another issue, don't know if this is related to the attempted fix. In console send sy path, then send again sy path with value empty in angle brackets and the engine would crash.
asmFishW_2017-04-03_popcnt
uci
id name asmFishW_2017-04-03_popcnt
id author TypingALot
option name Hash type spin default 16 min 1 max 65536
option name LargePages type check default false
option name Threads type spin default 1 min 1 max 256
option name NodeAffinity type string default all
option name Priority type combo default none var none var normal var low var idl
e
option name TTFile type string default <empty>
option name TTSave type button
option name TTLoad type button
option name Clear Hash type button
option name Ponder type check default false
option name UCI_Chess960 type check default false
option name MultiPV type spin default 1 min 1 max 224
option name Contempt type spin default 0 min -100 max 100
option name MoveOverhead type spin default 30 min 0 max 5000
option name MinThinkTime type spin default 20 min 0 max 5000
option name SlowMover type spin default 89 min 10 max 1000
option name SyzygyProbeDepth type spin default 1 min 1 max 100
option name SyzygyProbeLimit type spin default 6 min 0 max 6
option name Syzygy50MoveRule type check default true
option name SyzygyPath type string default <empty>
uciok
setoption name syzygypath value c:\myfiles\chess\syzygy
info string found 145 tablebases in "c:\myfiles\chess\syzygy"
info string found 145 of 510 tablebases
setoption name syzygypath value <empty>
here engine would crash.
You load the TB and then decide to unload them. This is somewhat irrational behaviour which asmFish does not approve and crashes in indignation. I will check this when I get to my 64 bit.
It is a normal behavior when trying to observe how the engine would analyze a position with or without the use of TB, so the user may send the empty path again after setting the non-empty path.
I am closing this issue now.
It is up to you to create a new issue for this new crashing behaviour.
Yeah I noticed that too when it happened to me a few days ago but I just set the path to something like Z:\ where I have no TB's so that it doesn't use it. Although I wish SF would add a uci option to simply check/uncheck the use of TB's so we wouldn't have to do this manually every time.
From: fsmosca notifications@github.com Sent: Tuesday, April 4, 2017 1:13:51 AM To: lantonov/asmFish Cc: hero2017; Mention Subject: Re: [lantonov/asmFish] Adding semi-colon after syzygy path would crash asmFishW_2017-03-30_popcnt.exe (#42)
Looks like there is another issue, don't know if this is related to the attempted fix. In console send sy path, then send again sy path with value empty in angle brackets and the engine would crash.
asmFishW_2017-04-03_popcnt
uci
id name asmFishW_2017-04-03_popcnt
id author TypingALot
option name Hash type spin default 16 min 1 max 65536
option name LargePages type check default false
option name Threads type spin default 1 min 1 max 256
option name NodeAffinity type string default all
option name Priority type combo default none var none var normal var low var idl
e
option name TTFile type string default
setoption name syzygypath value c:\myfiles\chess\syzygy
info string found 145 tablebases in "c:\myfiles\chess\syzygy"
info string found 145 of 510 tablebases
setoption name syzygypath value
here engine would crash.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/lantonov/asmFish/issues/42#issuecomment-291395387, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ATYgIwtxyiuq1TH34PDzFsKrggXi_lWSks5rsdGPgaJpZM4MvHD0.
asmFishW_2017-04-03_bmi2
setoption name syzygypath value c:\Winboard\syzygy
info string found 145 tablebases in "c:\Winboard\syzygy"
info string found 145 of 510 tablebases
setoption name syzygypath value c:\Winboard\
info string found 0 tablebases in "c:\Winboard\"
info string found 0 of 510 tablebases
setoption name syzygypath value c:\
info string found 0 tablebases in "c:\"
info string found 0 of 510 tablebases
setoption name syzygypath value empty
info string found 0 tablebases in "empty"
info string found 0 of 510 tablebases
setoption name syzygypath value <
info string found 0 tablebases in "<"
info string found 0 of 510 tablebases
setoption name syzygypath value <>
info string found 0 tablebases in "<>"
info string found 0 of 510 tablebases
setoption name syzygypath value < >
info string found 0 tablebases in "< >"
info string found 0 of 510 tablebases
setoption name syzygypath value <empty>
Crashes only at the last command. I am reluctant to open a new issue mainly because we have to spare Mohammed Li's (tthsqe12) time. He is one of the few people in the world who can write a big serious assembly program from scratch and that's why his time is more valuable than ours. For the last fix he wrote a lot of code (132 lines added and 89 lines deleted) which I suppose took him at least 2-3 hours and possibly more because he is thoroughly testing each new code. This time can be spend for more significant aims like keeping up with SF, finishing armFish, expanding in new directions like mate-finder, networks, etc.
Example path that would crash the engine.
c:\chess\syzygy;J:\Syzygy;G:\Syzygy;
This was mentioned by Dann in CCC and I tried it on asmFishW_2017-03-30_popcnt.exe.