[Bug string/29265] New: mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[Bug string/29265] New: mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[goldstein.w.n at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

            Bug ID: 29265
           Summary: mbstowcs with NULL dst throws an incorrect warning
                    (glibc == 2.35, gcc=12.0.1)
           Product: glibc
           Version: 2.35
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: string
          Assignee: unassigned at sourceware dot org
          Reporter: goldstein.w.n at gmail dot com
  Target Milestone: ---

For example building binutils HEAD get the following warning:

```
In file included from /usr/include/features.h:486,
                 from
/usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33,
                 from /usr/include/stdio.h:27,
                 from as.h:42,
                 from read.c:33:
In function ‘mbstowcs’,
    inlined from ‘read_symbol_name’ at read.c:1670:11:
/usr/include/x86_64-linux-gnu/bits/stdlib.h:115:10: error: argument 1 is null
but the corresponding size argument 3 value is [128, 9223372036854775807]
[-Werror=nonnull]
  115 |   return __glibc_fortify_n (mbstowcs, __len, sizeof (wchar_t),
      |          ^~~~~~~~~~~~~~~~~
In file included from /usr/include/stdlib.h:1027,
                 from as.h:44:
/usr/include/x86_64-linux-gnu/bits/stdlib.h: In function ‘read_symbol_name’:
/usr/include/x86_64-linux-gnu/bits/stdlib.h:95:15: note: in a call to function
‘__mbstowcs_chk’ declared with attribute ‘access (read_only, 2)’
   95 | extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,

```

Which is from:
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=gas/read.c;h=c6ce0345892284a48e5280ebb9cca111fab975f4;hb=HEAD#l1670
```
1670:    if (mbstowcs (NULL, name, len) == (size_t) -1)
```

`mbstowcs` supports a NULL destination:
https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs#:~:text=POSIX%20specifies%20a%20common%20extension%3A%20if%20dst%20is%20a%20null%20pointer%2C%20this%20function%20returns%20the%20number%20of%20wide%20characters%20that%20would%20be%20written%20to%20dst%2C%20if%20converted.%20Similar%20behavior%20is%20standard%20for%20std%3A%3Ambsrtowcs.

so that seems like a bug in the GLIBC attributes of `mbstowcs`.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

Siddhesh Poyarekar <siddhesh at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
                 CC|                            |siddhesh at sourceware dot org
   Last reconfirmed|                            |2022-06-21

--- Comment #1 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
Agreed, the __attr_access ((__write_only__, 1, 3)) is incorrect for mbstowcs,
the former requires the size argument to accurately reflect the object argument
(i.e. 0 for a NULL pointer) whereas the mbstowcs implementation is expected to
ignore the size argument if the first argument is NULL.  Would you like to
write a patch to fix this up?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[goldstein.w.n at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

--- Comment #2 from Noah Goldstein <goldstein.w.n at gmail dot com> ---
Sure(In reply to Siddhesh Poyarekar from comment #1)
> Agreed, the __attr_access ((__write_only__, 1, 3)) is incorrect for
> mbstowcs, the former requires the size argument to accurately reflect the
> object argument (i.e. 0 for a NULL pointer) whereas the mbstowcs
> implementation is expected to ignore the size argument if the first argument
> is NULL.  Would you like to write a patch to fix this up?


Sure.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[goldstein.w.n at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

--- Comment #3 from Noah Goldstein <goldstein.w.n at gmail dot com> ---
Created attachment 14157
  --> https://sourceware.org/bugzilla/attachment.cgi?id=14157&action=edit
Potential Patch

Not able to reproduce the issue in glibc test suite so not fully confident in
this change.

Siddhesh, any ideas for how to get this error to throw in testmb.c?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

--- Comment #4 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
I'm not very picky about having a test for this FWIW, this seems very sensitive
to optimizations and may not survive gcc updates.

As for the fix, just remove the  __attr_access ((__write_only__, 1, 3)), it's
incorrect in the context of mbstowcs definition.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[goldstein.w.n at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

--- Comment #5 from Noah Goldstein <goldstein.w.n at gmail dot com> ---
(In reply to Siddhesh Poyarekar from comment #4)
> I'm not very picky about having a test for this FWIW, this seems very
> sensitive to optimizations and may not survive gcc updates.
> 
> As for the fix, just remove the  __attr_access ((__write_only__, 1, 3)),
> it's incorrect in the context of mbstowcs definition.

You don't think its worth preserving if dst is non-null? In that case it is
meaningful no?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[siddhesh at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

--- Comment #6 from Siddhesh Poyarekar <siddhesh at sourceware dot org> ---
(In reply to Noah Goldstein from comment #5)
> (In reply to Siddhesh Poyarekar from comment #4)
> > I'm not very picky about having a test for this FWIW, this seems very
> > sensitive to optimizations and may not survive gcc updates.
> > 
> > As for the fix, just remove the  __attr_access ((__write_only__, 1, 3)),
> > it's incorrect in the context of mbstowcs definition.
> 
> You don't think its worth preserving if dst is non-null? In that case it is
> meaningful no?

It is meaningful, just that the check may trigger if the dest pointer has not
yet been simplified to a const.  Although, maybe I'm overthinking it.  Lets go
with what you have.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug string/29265] mbstowcs with NULL dst throws an incorrect warning (glibc == 2.35, gcc=12.0.1)

[vincent.vsmeets at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=29265

Vincent Smeets <vincent.vsmeets at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |vincent.vsmeets at gmail dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.