Fathom, munmap issue

[Tearth]

Hi, I know a lot of people are using Fathom here - last time I'm trying to incorporate this library into my engine and found an issue that I can't explain. The thing happens on Linux (Windows works ok) when I'm calling tb_init two times (so this is basically a scenario when SyzygyPath is changed two times through UCI) - in the second call, Fathom tries to free tables initialized earlier:

Code: Select all

// if pathString is set, we need to clean up first.
if (pathString) {
  free(pathString);
  free(paths);

  for (int i = 0; i < tbNumPiece; i++)
    free_tb_entry((struct BaseEntry *)&pieceEntry[i]);
  for (int i = 0; i < tbNumPawn; i++)
    free_tb_entry((struct BaseEntry *)&pawnEntry[i]);

  LOCK_DESTROY(tbMutex);

  pathString = NULL;
  numWdl = numDtm = numDtz = 0;
}

The issue is, every time during this process, I'm getting multiple (like 4 or 7, it's also not constant) "munmap: No error information" messages in the output. The data pointer and size look okayish (so no nulls or some strange values, but can't say it for sure since sadly I almost completely don't understand how the whole algorithm is working).

Code: Select all

static void unmap_file(void *data, map_t size)
{
  if (!data) return;
  if (!munmap(data, size)) {
    perror("munmap");
  }
}

Besides that, everything still works well, so in theory, I can just silence this error and forget about everything (especially considering that SyzygyPath is changed twice rather rarely), but at the same time I'm kinda worried that it's the symptom of some other bug that will crash my engine one day. If anyone experienced anything similar, I would be glad for a little help or explanation.

[syzygy]

Code: Select all

// if pathString is set, we need to clean up first.
if (pathString) {
  free(pathString);
  free(paths);

  for (int i = 0; i < tbNumPiece; i++)
    free_tb_entry((struct BaseEntry *)&pieceEntry[i]);
  for (int i = 0; i < tbNumPawn; i++)
    free_tb_entry((struct BaseEntry *)&pawnEntry[i]);

  LOCK_DESTROY(tbMutex);

  pathString = NULL;
  numWdl = numDtm = numDtz = 0;
}

The problem is that the Fathom code does not set tbNumPiece and tbNumPawn to 0 if a few lines later tb_init() exists because path is zero or "<empty>". tbNumPiece and tbNumPawn are reset only later in the code.

The CFish tb_init() code has this:

Code: Select all

  // if pathString is set, we need to clean up first.
  if (pathString) {
    free(pathString);
    free(paths);

    TB_release();

    LOCK_DESTROY(tbMutex);

    pathString = NULL;
  }

  numWdl = numDtm = numDtz = 0;
  tbNumPiece = tbNumPawn = 0;
  TB_MaxCardinality = TB_MaxCardinalityDTM = 0;

  // if path is an empty string or equals "<empty>", we are done.
  const char *p = path;
  if (strlen(p) == 0 || !strcmp(p, "<empty>")) return;

This prevents the error messages.

[Tearth]

Thanks, while what you wrote wasn't exactly the reason, I found that this condition is just written inverted (CFish doesn't check it at all):

Code: Select all

if (!munmap(data, size)) {
    perror("munmap");
}

https://github.com/jdart1/Fathom/blob/m ... obe.c#L372

Function munmap returns 0 on success and -1 on error, and indeed I've tested that it's always 0 so everything is perfectly fine. That's also why sometimes I was getting nonsensical errno value, which was obviously not set during munmap calls. I will create a pull request, so hopefully it will be fixed for future users.

Source:
https://linux.die.net/man/2/munmap

On success, munmap() returns 0, on failure -1, and errno is set (probably to EINVAL).

[Tearth]

Pull request with the fix has been accepted and merged, so the problem is now resolved for everyone.