[PATCH v2] stdlib: Remove attr_write from mbstows if dst is NULL [BZ: 29265]

[Noah Goldstein]

On Wed, Jun 22, 2022 at 1:56 PM Mark Wielaard <mark@klomp.org> wrote:
>
> Hi,
>
> (Sorry for the duplicate, got the mailinglist name wrong)
>
> On Wed, Jun 22, 2022 at 03:42:43PM +0000, Siddhesh Poyarekar wrote:
> > On 22/06/2022 20:54, Noah Goldstein via Libc-alpha wrote:
> > > mbstows is defined if dst is NULL and is defined to special cased if
> > > dst is NULL so the fortify objsize check if incorrect in that case.
> > >
> > > Tested on x86-64 linux.
> > > ---
> > >   stdlib/Makefile      |  3 +++
> > >   stdlib/bits/stdlib.h | 16 +++++++++++-----
> > >   stdlib/testmb.c      |  7 +++++++
> > >   3 files changed, 21 insertions(+), 5 deletions(-)
> >
> > LGTM.
> >
> > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
> This broke make check for me on Fedora 35 with gcc (GCC) 11.3.1
> 20220421 (Red Hat 11.3.1-2)
>
> gcc -o /srv/glibc/glibc-obj/stdlib/testmb -pie  -Wl,-O1 -nostdlib -nostartfiles    -Wl,-z,relro  /srv/glibc/glibc-obj/csu/Scrt1.o /srv/glibc/glibc-obj/csu/crti.o `gcc  --print-file-name=crtbeginS.o` /srv/glibc/glibc-obj/stdlib/testmb.o /srv/glibc/glibc-obj/support/libsupport_nonshared.a  -Wl,-dynamic-linker=/lib64/ld-linux-x86-64.so.2 -Wl,-rpath-link=/srv/glibc/glibc-obj:/srv/glibc/glibc-obj/math:/srv/glibc/glibc-obj/elf:/srv/glibc/glibc-obj/dlfcn:/srv/glibc/glibc-obj/nss:/srv/glibc/glibc-obj/nis:/srv/glibc/glibc-obj/rt:/srv/glibc/glibc-obj/resolv:/srv/glibc/glibc-obj/mathvec:/srv/glibc/glibc-obj/support:/srv/glibc/glibc-obj/crypt:/srv/glibc/glibc-obj/nptl -lgcc -Wl,--as-needed -lgcc_s  -Wl,--no-as-needed /srv/glibc/glibc-obj/libc.so.6 /srv/glibc/glibc-obj/libc_nonshared.a -Wl,--as-needed /srv/glibc/glibc-obj/elf/ld.so -Wl,--no-as-needed -lgcc -Wl,--as-needed -lgcc_s  -Wl,--no-as-needed `gcc  --print-file-name=crtendS.o` /srv/glibc/glibc-obj/csu/crtn.o
> /usr/bin/ld: /srv/glibc/glibc-obj/stdlib/testmb.o: in function `mbstowcs':
> /srv/glibc/glibc/stdlib/../stdlib/bits/stdlib.h:121: undefined reference to `mbstowcs_chk'
> collect2: error: ld returned 1 exit status
> make[2]: *** [../Rules:238: /srv/glibc/glibc-obj/stdlib/testmb] Error 1
>
> It looks like that mbstowcs_chk should be __mbstowcs_chk
>
> diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
> index d9c2d822a5..a0c4996614 100644
> --- a/stdlib/bits/stdlib.h
> +++ b/stdlib/bits/stdlib.h
> @@ -99,7 +99,7 @@ extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
>  extern size_t __REDIRECT_NTH (__mbstowcs_chk_nulldst,
>                               (wchar_t *__restrict __dst,
>                                const char *__restrict __src,
> -                              size_t __len), mbstowcs_chk)
> +                              size_t __len), __mbstowcs_chk)
>      __attr_access ((__read_only__, 2));
>  extern size_t __REDIRECT_NTH (__mbstowcs_alias,
>                               (wchar_t *__restrict __dst,
>
> Which makes it compile, but then the stdlib/testmb testcase fails.

I think it should be just mbstowcs, we don't pass dstlen

Does:

```
diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
index d9c2d822a5..ece553df74 100644
--- a/stdlib/bits/stdlib.h
+++ b/stdlib/bits/stdlib.h
@@ -96,10 +96,10 @@ extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
        const char *__restrict __src,
        size_t __len, size_t __dstlen) __THROW
     __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
-extern size_t __REDIRECT_NTH (__mbstowcs_chk_nulldst,
+extern size_t __REDIRECT_NTH (__mbstowcs_nulldst,
        (wchar_t *__restrict __dst,
         const char *__restrict __src,
-        size_t __len), mbstowcs_chk)
+        size_t __len), mbstowcs)
     __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__mbstowcs_alias,
        (wchar_t *__restrict __dst,
@@ -118,7 +118,7 @@ __NTH (mbstowcs (wchar_t *__restrict __dst, const
char *__restrict __src,
  size_t __len))
 {
   if (__builtin_constant_p (__dst == NULL) && __dst == NULL)
-    return __mbstowcs_chk_nulldst (__dst, __src, __len);
+    return __mbstowcs_nulldst (__dst, __src, __len);
   else
     return __glibc_fortify_n (mbstowcs, __len, sizeof (wchar_t),
        __glibc_objsize (__dst), __dst, __src, __len);
```

Work?
>
> Cheers,
>
> Mark
>