• issue with preadv/pwritev and gcc on armel/armhf

    From =?UTF-8?B?SsOpcsOpbXkgTGFs?=@21:1/5 to All on Mon Jan 27 15:30:01 2025
    Hi,

    as discussed in
    https://github.com/libuv/libuv/issues/4678

    and associated build failures https://buildd.debian.org/status/package.php?p=libuv1&suite=experimental

    It seems that gcc is doing something wrong with the offset parameter of
    preadv, pwritev.
    The same problem happens with optimization levels 0, 1, 2.

    The call is made at https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494

    To reproduce, just dpkg-buildpackage from
    dget -xu https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc

    I'd be surprised if it concerns only libuv !
    Please have a look, because the libuv maintainers already tried their best.

    Jérémy

    <div dir="ltr">Hi,<div><br></div><div>as discussed in</div><div><a href="https://github.com/libuv/libuv/issues/4678">https://github.com/libuv/libuv/issues/4678</a></div><div><br></div><div>and associated build failures </div><div><a href="https://
    buildd.debian.org/status/package.php?p=libuv1&amp;suite=experimental">https://buildd.debian.org/status/package.php?p=libuv1&amp;suite=experimental</a></div><div><br></div><div>It seems that gcc is doing something wrong with the offset parameter of preadv,
    pwritev.</div><div>The same problem happens with optimization levels 0, 1, 2.</div><div><br></div><div>The call is made at</div><div><a href="https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494">https://salsa.
    debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494</a></div><div><br></div><div>To reproduce, just dpkg-buildpackage from</div><div>dget -xu <a href="https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc">
    https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc</a></div><div><br></div><div>I&#39;d be surprised if it concerns only libuv !</div><div>Please have a look, because the libuv maintainers already tried their best.</div><div><br></
    <div>Jérémy</div></div>

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Arnd Bergmann@21:1/5 to All on Mon Jan 27 17:50:01 2025
    On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:
    Hi,

    as discussed in
    https://github.com/libuv/libuv/issues/4678

    and associated build failures https://buildd.debian.org/status/package.php?p=libuv1&suite=experimental

    It seems that gcc is doing something wrong with the offset parameter of preadv, pwritev.
    The same problem happens with optimization levels 0, 1, 2.

    The call is made at https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494

    To reproduce, just dpkg-buildpackage from
    dget -xu https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc

    I'd be surprised if it concerns only libuv !
    Please have a look, because the libuv maintainers already tried their best.

    My strong guess is that this is caused by the implied 64-bit
    off_t after the time64 conversion. On i386, the file is
    likely still built with a 32-bit off_t.

    In the source line

    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");

    the uv__preadv_or_pwritev() function looks up the preadv() or
    pwritev() symbols in glibc, but those expect a 32-bit ("long")
    off_t, unlike preadv64() and pwritev64().

    It appears that https://github.com/libuv/libuv/issues/4532
    attempted to address the issue, but completely misunderstood
    the problem, as this was a bug in libuv, not in gcc.

    Ideally the library should avoid using dlsym() here, since
    that makes it unportable. If it can't be avoided, it could
    try to do something like

    if ((sizeof(long)< sizeof(off_t))
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv64" : "pwritev64");
    else
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");

    This in turn makes it less portable to non-glibc
    implementations like musl.

    Arnd

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From =?UTF-8?B?SsOpcsOpbXkgTGFs?=@21:1/5 to All on Mon Jan 27 19:40:02 2025
    Le lun. 27 janv. 2025 à 17:41, Arnd Bergmann <[email protected]> a écrit :

    On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:
    Hi,

    as discussed in
    https://github.com/libuv/libuv/issues/4678

    and associated build failures https://buildd.debian.org/status/package.php?p=libuv1&suite=experimental

    It seems that gcc is doing something wrong with the offset parameter of preadv, pwritev.
    The same problem happens with optimization levels 0, 1, 2.

    The call is made at

    https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494

    To reproduce, just dpkg-buildpackage from
    dget -xu
    https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc

    I'd be surprised if it concerns only libuv !
    Please have a look, because the libuv maintainers already tried their
    best.

    My strong guess is that this is caused by the implied 64-bit
    off_t after the time64 conversion. On i386, the file is
    likely still built with a 32-bit off_t.

    In the source line

    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");

    the uv__preadv_or_pwritev() function looks up the preadv() or
    pwritev() symbols in glibc, but those expect a 32-bit ("long")
    off_t, unlike preadv64() and pwritev64().

    It appears that https://github.com/libuv/libuv/issues/4532
    attempted to address the issue, but completely misunderstood
    the problem, as this was a bug in libuv, not in gcc.

    Ideally the library should avoid using dlsym() here, since
    that makes it unportable. If it can't be avoided, it could
    try to do something like

    if ((sizeof(long)< sizeof(off_t))
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv64" : "pwritev64");
    else
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");


    Thank you, it works with that.
    I linked your reply in the upstream issue.

    This in turn makes it less portable to non-glibc
    implementations like musl.


    I suppose that unless upstream rewrites some parts, we will just add a patch
    in the Debian package for now.

    <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">Le lun. 27 janv. 2025 à 17:41, Arnd Bergmann &lt;<a href="mailto:[email protected]">[email protected]</a>&gt; a écrit :<br></div><
    blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:<br>
    &gt; Hi,<br>
    &gt;<br>
    &gt; as discussed in<br>
    &gt; <a href="https://github.com/libuv/libuv/issues/4678" rel="noreferrer" target="_blank">https://github.com/libuv/libuv/issues/4678</a><br>
    &gt;<br>
    &gt; and associated build failures <br>
    &gt; <a href="https://buildd.debian.org/status/package.php?p=libuv1&amp;suite=experimental" rel="noreferrer" target="_blank">https://buildd.debian.org/status/package.php?p=libuv1&amp;suite=experimental</a><br>
    &gt;<br>
    &gt; It seems that gcc is doing something wrong with the offset parameter of <br>
    &gt; preadv, pwritev.<br>
    &gt; The same problem happens with optimization levels 0, 1, 2.<br>
    &gt;<br>
    &gt; The call is made at<br>
    &gt; <a href="https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494" rel="noreferrer" target="_blank">https://salsa.debian.org/debian/libuv1/-/blob/debian/sid/src/unix/fs.c?ref_type=heads#L494</a><br>
    &gt;<br>
    &gt; To reproduce, just dpkg-buildpackage from<br>
    &gt; dget -xu <a href="https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc" rel="noreferrer" target="_blank">https://deb.debian.org/debian/pool/main/libu/libuv1/libuv1_1.49.2-1.dsc</a><br>
    &gt;<br>
    &gt; I&#39;d be surprised if it concerns only libuv !<br>
    &gt; Please have a look, because the libuv maintainers already tried their best.<br>

    My strong guess is that this is caused by the implied 64-bit<br>
    off_t after the time64 conversion. On i386, the file is<br>
    likely still built with a 32-bit off_t.<br>

    In the source line<br>

        p = dlsym(RTLD_DEFAULT, is_pread ? &quot;preadv&quot; : &quot;pwritev&quot;);<br>

    the uv__preadv_or_pwritev() function looks up the preadv() or<br>
    pwritev() symbols in glibc, but those expect a 32-bit (&quot;long&quot;)<br> off_t, unlike preadv64() and pwritev64().<br>

    It appears that  <a href="https://github.com/libuv/libuv/issues/4532" rel="noreferrer" target="_blank">https://github.com/libuv/libuv/issues/4532</a><br>
    attempted to address the issue, but completely misunderstood<br>
    the problem, as this was a bug in libuv, not in gcc.<br>

    Ideally the library should avoid using dlsym() here, since<br>
    that makes it unportable. If it can&#39;t be avoided, it could<br>
    try to do something like<br>

       if ((sizeof(long)&lt; sizeof(off_t))<br>
                p = dlsym(RTLD_DEFAULT, is_pread ? &quot;preadv64&quot; : &quot;pwritev64&quot;);<br>
       else<br>
                p = dlsym(RTLD_DEFAULT, is_pread ? &quot;preadv&quot; : &quot;pwritev&quot;);<br></blockquote><div><br></div><div>Thank you, it works with that.</div><div>I linked your reply in the upstream issue.</div><div><br></div><blockquote class="
    gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
    This in turn makes it less portable to non-glibc<br>
    implementations like musl.<br></blockquote><div><br></div><div>I suppose that unless upstream rewrites some parts, we will just add a patch</div><div>in the Debian package for now.<br></div><div><br></div></div></div>

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From =?UTF-8?B?SsOpcsOpbXkgTGFs?=@21:1/5 to All on Tue Jan 28 09:50:01 2025
    Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann <[email protected]> a écrit :

    On Mon, Jan 27, 2025, at 19:31, Jérémy Lal wrote:
    Le lun. 27 janv. 2025 à 17:41, Arnd Bergmann <[email protected]> a écrit :
    On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:

    if ((sizeof(long)< sizeof(off_t))
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv64" :
    "pwritev64");
    else
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");

    Thank you, it works with that.
    I linked your reply in the upstream issue.

    It appears that the proposed workaround at


    https://github.com/libuv/libuv/pull/4683/commits/45bf09e2567b6480962d932946608454fab31b87

    now just always tries to find the preadv64()/pwritev64() symbols,
    which is still nonportable because this doesn't work on targets
    that use a 32-bit off_t.

    If i386-debian was working before this change, it is now broken.

    The bit I don't understand is why libuv was ever getting built
    without largefile support. It probably makes sense to change that
    for all architectures regardless of time64 support, but this
    is likely an ABI break and requires rebuilding all libuv users
    in turn.


    It builds fine in a i386 chroot on barriere.d.o with https://github.com/libuv/libuv/pull/4683.patch

    <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann &lt;<a href="mailto:[email protected]">[email protected]</a>&gt; a écrit :<br></div><
    blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jan 27, 2025, at 19:31, Jérémy Lal wrote:<br>
    &gt; Le lun. 27 janv. 2025 à 17:41, Arnd Bergmann &lt;<a href="mailto:[email protected]" target="_blank">[email protected]</a>&gt; a écrit :<br>
    &gt;&gt; On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:<br>
    &gt;&gt; <br>
    &gt;&gt;    if ((sizeof(long)&lt; sizeof(off_t))<br>
    &gt;&gt;             p = dlsym(RTLD_DEFAULT, is_pread ? &quot;preadv64&quot; : &quot;pwritev64&quot;);<br>
    &gt;&gt;    else<br>
    &gt;&gt;             p = dlsym(RTLD_DEFAULT, is_pread ? &quot;preadv&quot; : &quot;pwritev&quot;);<br>
    &gt;<br>
    &gt; Thank you, it works with that.<br>
    &gt; I linked your reply in the upstream issue.<br>

    It appears that the proposed workaround at<br>

    <a href="https://github.com/libuv/libuv/pull/4683/commits/45bf09e2567b6480962d932946608454fab31b87" rel="noreferrer" target="_blank">https://github.com/libuv/libuv/pull/4683/commits/45bf09e2567b6480962d932946608454fab31b87</a><br>

    now just always tries to find the preadv64()/pwritev64() symbols,<br>
    which is still nonportable because this doesn&#39;t work on targets<br>
    that use a 32-bit off_t.<br>

    If i386-debian was working before this change, it is now broken.<br>

    The bit I don&#39;t understand is why libuv was ever getting built<br>
    without largefile support. It probably makes sense to change that<br>
    for all architectures regardless of time64 support, but this<br>
    is likely an ABI break and requires rebuilding all libuv users<br>
    in turn.</blockquote><div><br></div><div>It builds fine in a i386 chroot on barriere.d.o with</div><div><a href="https://github.com/libuv/libuv/pull/4683.patch">https://github.com/libuv/libuv/pull/4683.patch</a></div></div></div>

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Arnd Bergmann@21:1/5 to All on Tue Jan 28 09:40:01 2025
    On Mon, Jan 27, 2025, at 19:31, Jérémy Lal wrote:
    Le lun. 27 janv. 2025 à 17:41, Arnd Bergmann <[email protected]> a écrit :
    On Mon, Jan 27, 2025, at 15:19, Jérémy Lal wrote:

    if ((sizeof(long)< sizeof(off_t))
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv64" : "pwritev64");
    else
    p = dlsym(RTLD_DEFAULT, is_pread ? "preadv" : "pwritev");

    Thank you, it works with that.
    I linked your reply in the upstream issue.

    It appears that the proposed workaround at

    https://github.com/libuv/libuv/pull/4683/commits/45bf09e2567b6480962d932946608454fab31b87

    now just always tries to find the preadv64()/pwritev64() symbols,
    which is still nonportable because this doesn't work on targets
    that use a 32-bit off_t.

    If i386-debian was working before this change, it is now broken.

    The bit I don't understand is why libuv was ever getting built
    without largefile support. It probably makes sense to change that
    for all architectures regardless of time64 support, but this
    is likely an ABI break and requires rebuilding all libuv users
    in turn.

    Arnd

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Arnd Bergmann@21:1/5 to All on Tue Jan 28 10:30:01 2025
    On Tue, Jan 28, 2025, at 09:38, Jérémy Lal wrote:
    Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann <[email protected]> a écrit :

    The bit I don't understand is why libuv was ever getting built
    without largefile support. It probably makes sense to change that
    for all architectures regardless of time64 support, but this
    is likely an ABI break and requires rebuilding all libuv users
    in turn.

    It builds fine in a i386 chroot on barriere.d.o with https://github.com/libuv/libuv/pull/4683.patch

    But does it successfully run the test case? I can't
    see how the behavior on i386 is correct both before
    and after the patch: either the bug existed on all
    32-bit architectures because they enabled largefile
    support, or changing i386 over to use the largefile
    version of preadv/writev is wrong.

    Can you debug into this file on i386 to see which symbol
    it picks up in the end, and whether off_t is defined
    as a 32-bit or 64-bit type?

    It's possible that for some reason the testcase fails
    to report a bug on i386 despite the code being wrong
    either before or after your patch.

    Arnd

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From =?UTF-8?B?SsOpcsOpbXkgTGFs?=@21:1/5 to All on Tue Jan 28 10:50:01 2025
    Le mar. 28 janv. 2025 à 10:21, Arnd Bergmann <[email protected]> a écrit :

    On Tue, Jan 28, 2025, at 09:38, Jérémy Lal wrote:
    Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann <[email protected]> a écrit :

    The bit I don't understand is why libuv was ever getting built
    without largefile support. It probably makes sense to change that
    for all architectures regardless of time64 support, but this
    is likely an ABI break and requires rebuilding all libuv users
    in turn.

    It builds fine in a i386 chroot on barriere.d.o with https://github.com/libuv/libuv/pull/4683.patch

    But does it successfully run the test case?


    Yes

    I can't
    see how the behavior on i386 is correct both before
    and after the patch: either the bug existed on all
    32-bit architectures because they enabled largefile
    support, or changing i386 over to use the largefile
    version of preadv/writev is wrong.

    Can you debug into this file on i386 to see which symbol
    it picks up in the end, and whether off_t is defined
    as a 32-bit or 64-bit type?


    After patch, preadv64.
    In both cases,
    (gdb) whatis off
    type = off_t
    (gdb) whatis off_t
    type = __off64_t


    It's possible that for some reason the testcase fails
    to report a bug on i386 despite the code being wrong
    either before or after your patch.

    <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">Le mar. 28 janv. 2025 à 10:21, Arnd Bergmann &lt;<a href="mailto:[email protected]">[email protected]</a>&gt; a écrit :<br></div><
    blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Jan 28, 2025, at 09:38, Jérémy Lal wrote:<br>
    &gt; Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann &lt;<a href="mailto:[email protected]" target="_blank">[email protected]</a>&gt; a écrit :<br>
    &gt;&gt; <br>
    &gt;&gt; The bit I don&#39;t understand is why libuv was ever getting built<br> &gt;&gt; without largefile support. It probably makes sense to change that<br> &gt;&gt; for all architectures regardless of time64 support, but this<br> &gt;&gt; is likely an ABI break and requires rebuilding all libuv users<br> &gt;&gt; in turn.<br>
    &gt;<br>
    &gt; It builds fine in a i386 chroot on barriere.d.o with<br>
    &gt; <a href="https://github.com/libuv/libuv/pull/4683.patch" rel="noreferrer" target="_blank">https://github.com/libuv/libuv/pull/4683.patch</a><br>

    But does it successfully run the test case?</blockquote><div><br></div><div>Yes </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">I can&#39;t<br>
    see how the behavior on i386 is correct both before<br>
    and after the patch: either the bug existed on all<br>
    32-bit architectures because they enabled largefile<br>
    support, or changing i386 over to use the largefile<br>
    version of preadv/writev is wrong.<br>

    Can you debug into this file on i386 to see which symbol<br>
    it picks up in the end, and whether off_t is defined<br>
    as a 32-bit or 64-bit type?<br></blockquote><div><br></div><div>After patch, preadv64.</div><div>In both cases,</div><div>(gdb) whatis off<br>type = off_t<br>(gdb) whatis off_t<br>type = __off64_t</div><div> </div><blockquote class="gmail_quote" style="
    margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
    It&#39;s possible that for some reason the testcase fails<br>
    to report a bug on i386 despite the code being wrong<br>
    either before or after your patch.</blockquote><div><br></div><div> </div></div></div>

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Arnd Bergmann@21:1/5 to All on Tue Jan 28 11:20:01 2025
    On Tue, Jan 28, 2025, at 10:39, Jérémy Lal wrote:
    Le mar. 28 janv. 2025 à 10:21, Arnd Bergmann <[email protected]> a écrit :
    On Tue, Jan 28, 2025, at 09:38, Jérémy Lal wrote:
    Le mar. 28 janv. 2025 à 09:29, Arnd Bergmann <[email protected]> a écrit : >>
    Can you debug into this file on i386 to see which symbol
    it picks up in the end, and whether off_t is defined
    as a 32-bit or 64-bit type?

    After patch, preadv64.
    In both cases,
    (gdb) whatis off
    type = off_t
    (gdb) whatis off_t
    type = __off64_t

    It's possible that for some reason the testcase fails
    to report a bug on i386 despite the code being wrong
    either before or after your patch.

    Ok, good, so most likely your new version just works and
    it was broken on all 32-bit targets including i386 before
    your patch. That leaves the question why the testcase
    didn't catch that bug on i386.

    What I think is going on here is that i386 calling
    conventions for 64-bit function arguments happen to
    make it work correctly, as they are passed on the stack
    as five 32-bit words in preadv64()

    <fd> <iov> <iovcnt> <off_low> <off_high>

    and preadv() takes exactly the same four arguments
    but ignores the last one.

    On ARM EABI, the arguments are passed in six registers
    instead, with the 64-bit argument in an aligned pair
    of registers

    <fd> <iov> <iovcnt> <_PAD_> <off_low> <off_high>

    which in turn means that calling preadv() by accident
    with the preadv64 arguments puts uninitialized stack
    data into the 32-bit offset argument slow. The same thing
    happens on mips, powerpc, xtensa, arc, nios2, openrisc
    sh and riscv. On top of this, big-endian architectures
    also typically flip the low and high word for the
    64-bit argument, so you pass the wrong word on m68k,
    parisc, s390, sparc and openrisc despite those not
    inserting the padding argument.

    To catch these bugs better, I think a testcase could
    be added that passes and argument that has valid
    32-bit offset, but garbage in the high bits, e.g.
    0xffffffff00000008. On correctly working targets this
    should return errno=ENXIO, while on broken i386
    systems it would succeed reading from file offset 8.

    Arnd

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)