~skeeto/public-inbox

1

Re: An improved chkstk function on Windows

Details
Message ID
<20240209135830.r5hwnpyds7p3ihog@nullprogram.com>
DKIM signature
missing
Download raw message
There are 5 defects, quite impressive for 15-ish lines of code. For the 
record, and for anyone following along, here is libgcc/memmove.c:

void *
memmove (void *dest, const void *src, size_t len)
{
  char *d = dest;
  const char *s = src;
  if (d < s)
    while (len--)
      *d++ = *s++;
  else
    {
      char *lasts = s + (len-1);
      char *lastd = d + (len-1);
      while (len--)
        *lastd-- = *lasts--;
    }
  return dest;
}

> You’re referring to UB when `lastd` and `lasts` are decremented one past 
> their start?

That's the final two, yes. Nice spotting. This can be solved by keeping 
the pointers one beyond their "actual" address. That is, lasts and lastd 
begin one-past-the-end, and the loop is "*--lastd = *--lasts". When the 
loop exits, lasts == s and lastd == d.

The first defect is "d < s". Arguments to memmove may point to unrelated 
objects, in which case comparison is undefined. You might think the C 
standard does this for non-uniform memory (i.e. 8086 segmentation), but 
GCC actively uses it as a optimization hint regardless, and you can get a 
broken program if you break that rule.

Standard C has no portable solution for this. The closest you can get is 
casting the pointers to uintptr_t before comparison. That has a built-in 
assumption about host memory layout, but as far as I know it's true on all 
GCC targets.

The next two defects also come as a pair. Consider what happens when len 
is zero, which is permitted. Expression "len-1" wraps around to SIZE_MAX, 
then is added to s and d, overflowing each pointer, which is undefined. My 
suggested fix to the defects you spotted would fix this as well.

Because memmove forbids null pointer arguments — a grave misdesign in the 
C standard — null pointer checks are not required, avoiding those issues. 
If null pointers were allowed, but adding zero to a null pointer remained 
undefined — another grave error in the standard — then you'd need to check 
for null before running the "reverse" copy, lest computing last{s,d} would 
be undefined.

The memmove (named bcopy) in GCC's libiberty appears to be an independent 
implementation, but shares all five of these defects.

Re: An improved chkstk function on Windows

Matthew Fernandez <matthew.fernandez@gmail.com>
Details
Message ID
<4dc259e6-e11a-f380-1469-2bf32e6903a2@gmail.com>
In-Reply-To
<20240209135830.r5hwnpyds7p3ihog@nullprogram.com> (view parent)
DKIM signature
pass
Download raw message
Aha, thanks for the explanation. I had all the information but was not 
eagle eyed enough to spot these myself.
Reply to thread Export thread (mbox)