[ruby-core:109138] [Ruby master Bug#18893] Don't redefine memcpy(3)

Issue #18893 has been updated by shyouhei (Shyouhei Urabe).

Assignee set to shyouhei (Shyouhei Urabe)
Status changed from Rejected to Open

alx (Alejandro Colomar) wrote in #note-2:
> Actually, ISO C specifies (indirectly) that `memcpy(dest, NULL, 0)` is Undefined Behavior.
> See: <https://stackoverflow.com/questions/5243012/is-it-guaranteed-to-be-safe-to-perform-memcpy0-0-0>
> 
> ISO C doesn't specify the `[[gnu::nonnull]]` attribute (at least not yet), but that attribute makes sure that you don't invoke UB, so I don't think it's wrong here.

Oh, didn't know this.  Thank you.

> However, I can see reasons for calling `memcpy(dest, NULL, 0)` (e.g., not having to check always before calling memcpy(3)),
> and glibc (and all libcs that I know of) implement `memcpy(dest, NULL, 0)` as a no-op (see below for the glibc implementation), so it should be a safe call, as far as libc is concerned,
> so maybe ISO C could add a guarantee that it is a safe call.
> 
> So, I see reasons to redefine memcpy(3) on your side, to remove the warning.  But I think you could do it in a better way, that doesn't break users:
> 
> a - You could use ruby_nonempty_memcpy() directly, and don't redefine memcpy(3).
> b - Or you could define ruby_nonempty_memcpy() as a macro, instead of a static inline, so that your memcpy(3) wrapper can be called everywhere.
> c - Or you could define ruby_nonempty_memcpy() as a C99 inline function, adding an extern prototype in a .c file, and exporting the function.
> d - Or use `[[gnu::always_inline]]`, to make sure it's inlined (but we would still have a problem when a pointer to the function is taken), so (c) is better.
> 
> I think (c) will be the simplest for everyone.
> 
> About using `#undef`, I'm not sure it's legal to do that.  It's likely to work, but I don't think users should have to do this anyway.
> 
> Thanks,
> 
> Alex

Now I'm persuaded.  It is us who is doing something nasty.  Let me handle this.


----------------------------------------
Bug #18893: Don't redefine memcpy(3)
https://bugs.ruby-lang.org/issues/18893#change-98279

* Author: alx (Alejandro Colomar)
* Status: Open
* Priority: Normal
* Assignee: shyouhei (Shyouhei Urabe)
* ruby -v: ruby 3.0.4p208 (2022-04-12 revision 3fa771dded) [x86_64-linux-gnu]
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
It is Undefined Behavior, by any standard ever issued.


See what I have in my system right now:


```
alx@asus5775:/usr/include$ grepc memcpy
./string.h:43:
extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
		     size_t __n) __THROW __nonnull ((1, 2));


./x86_64-linux-gnu/ruby-3.0.0/rb_mjit_min_header-3.0.4.h:1520:
extern void *memcpy (void *__restrict __dest, const void *__restrict __src,
       size_t __n) __attribute__ ((__nothrow__ , __leaf__)) __attribute__ ((__nonnull__ (1, 2)));


./x86_64-linux-gnu/ruby-3.0.0/rb_mjit_min_header-3.0.4.h:1670:
extern __inline __attribute__ ((__always_inline__)) __attribute__ ((__gnu_inline__)) __attribute__ ((__artificial__)) void *
__attribute__ ((__nothrow__ , __leaf__)) memcpy (void *__restrict __dest, const void *__restrict __src, size_t __len)
{
  return __builtin___memcpy_chk (__dest, __src, __len,
     __builtin_object_size (__dest, 0));
}


./ruby-3.0.0/ruby/internal/memory.h:278:
#define memcpy ruby_nonempty_memcpy


./x86_64-linux-gnu/ruby-3.0.0/rb_mjit_min_header-3.0.4.h:22679:
#define memcpy ruby_nonempty_memcpy


$ grepc ruby_nonempty_memcpy
./ruby-3.0.0/ruby/internal/memory.h:266:
static inline void *
ruby_nonempty_memcpy(void *dest, const void *src, size_t n)
{
    if (n) {
        return memcpy(dest, src, n);
    }
    else {
        return dest;
    }
}


./x86_64-linux-gnu/ruby-3.0.0/rb_mjit_min_header-3.0.4.h:5673:
__attribute__((__nonnull__ (1)))
__attribute__((__returns_nonnull__))
static inline void *
ruby_nonempty_memcpy(void *dest, const void *src, size_t n)
{
    if (n) {
        return memcpy(dest, src, n);
    }
    else {
        return dest;
    }
}
```

Some code that I maintain includes some ruby headers, which end up defining memcpy(3) to that thing.  Then, my code calls memcpy(3) from a function, which happens to be inline (yes, inline, not static inline, which is a horrible thing), and I get an error from the compiler, for using a static function within a non-static inline function.

So, I'd like you to please remove that definition from public headers, and refrain from redefining any ISO C functions.
If not, please define it to something not broken (and yes, static inline is broken, as it produces duplicated code when not inlined; you could use [[gnu::always_inline]] if you want to keep static, but don't).  Just C99 inline would be reasonable.  See also: <https://www.greenend.org.uk/rjk/tech/inline.html>

Thanks,

Alex



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>