Build broken on 32 bit systems

[clausecker]

The build of 1.7.0 on armv7 FreeBSD 13.0-RELEASE still fails as reported in bug #629. The issue reported there was never actually fixed. This concerns this line of code:

/wrkdirs/usr/ports/devel/stlink/work/stlink-1.7.0/src/common.c:2222:16: error: implicit conversion loses integer precision: 'off_t' (aka 'long long') to 'size_t' (aka 'unsigned int') [-Werror,-Wshorten-64-to-32]
  mf->len = st.st_size;
          ~ ~~~^~~~~~~
1 error generated.

The problem is that while files are allowed to be very large, there is only a 32 bit address space on 32 bit machines. So size_t (for sizes of objects) is a 32 bit type while off_t (for sizes of files) is a 64 bit type. The correct resolution of this issue is to check if st.st_size is larger than SIZE_MAX (the largest possible size_t). If it is, an error must be returned immediately as the file is too large to be mapped. If st.st_size <= SIZE_MAX, a cast to size_t will not change the value and is thus correct to perform.

Please consider applying such a patch for 1.7.1.

[Nightwalker-87]

Related to #985.

[Nightwalker-87]

Yes absolutely - this must clearly be addressed.

[Nightwalker-87]

@clausecker

Actually there is such an attempt in /src/common.c to catch this error, but it does not seem to work properly for all cases:

  if (sizeof(st.st_size) != sizeof(size_t)) {
    // on 32 bit systems, check if there is an overflow
    if (st.st_size > (off_t)INT32_MAX) {
      fprintf(stderr, "mmap() size_t overflow for file %s\n", path);
      goto on_error;
    }
  }

[clausecker]

Well the check seems correct, but you still have to put the cast in. That is, you have to write

mf->len = (size_t)st.st_size;

to communicate to the compiler that you acknowledge the reduction in range. Also, the correct macro to check against is of course SSIZE_MAX, not INT32_MAX.

[Nightwalker-87]

I see, however I must admit this is not part of any own implementation, I just got aware of this one being present.
Can you confirm the above fix to work in general (e.g. also on other unix-based systems)?
If so, please give me a short buzz and I'll directly push it to develop.

[clausecker]

Unfortunately I only have FreeBSD machines to test, but I'll go ahead and give it a try.

[Nightwalker-87]

@clausecker Have you had the chance for a test in the meanwhile?

[clausecker]

Sorry, had a very stressful week. Will test it today.

[clausecker]

I can confirm that with the cast added, the current develop branch builds fine on armv7 FreeBSD 13.0-RELEASE.

[Nightwalker-87]

Oh, that's good news. 👍
I've already prepared a commit that will close this issue per GitHub Automation.