Re: [musl] patches for C23

[Rich Felker <dalias@libc.org>]

On Wed, May 03, 2023 at 09:13:40AM +0200, Jₑₙₛ Gustedt wrote:
> Rich,
> 
> on Tue, 2 May 2023 19:20:09 -0400 you (Rich Felker <dalias@libc.org>)
> wrote:
> 
> > On Tue, May 02, 2023 at 03:59:03PM +0200, Jₑₙₛ Gustedt wrote:
> > > on Tue, 2 May 2023 08:57:40 +0200 you (Jₑₙₛ Gustedt
> > > <jens.gustedt@inria.fr>) wrote:
> > >   
> > > > I'll also setup a git repository for those who would be willing to
> > > > test the whole. Just be aware that is really testing and review,
> > > > not yet ready for direct inclusion. So probably this will be
> > > > rebased several times.  
> > > 
> > > So this can now be found here
> > > 
> > >    https://icube-forge.unistra.fr/icps/musl/
> > > 
> > > with my additional branch called "c23". I also put on tags for what
> > > I think might be good groups to treat together. An overview should
> > > be accessible here
> > > 
> > >            https://icube-forge.unistra.fr/icps/musl/-/network/c23?ref_type=heads
> > > 
> > > Let me know if you have any problems in accessing this.
> > > 
> > > I will then post the patches on the ML later, probably need some
> > > time for that to do it right.  
> > 
> > One quick find, in
> > https://icube-forge.unistra.fr/icps/musl/-/commit/3a2b83bf32d7c94f1bf0b2b2fd6ba8b6bf980d99
> > 
> > -				np = strtoul(r+9, &z, 10);
> > +				np = strtoul(r+9, (char**)&z, 10);
> > 
> > is UB.
> 
> I think it the situation is more subtle than that. If this were
> application C code the implementation of `strtoul` would provoque UB
> under certain circumstances. And this UB would be happening in line 16
> of strtol.c, not at the calling side. Here at the calling side, we
> only have a pointer cast, which as such is well-defined because the
> two pointer types have same representation and alignment.

The UB on the application side is passing a pointer to an object of
the wrong type to a standard function. But internally within the
implementation, the actual UB happens inside the implementation of
strtol. In any case it's wrong/UB.

> Spinning that further, the code would then be UB as written before
> (with an unqualified `z`) because the call to `strtoul` then stores a
> `char const*` value into a `char*` object. By providing a declaration

No, it does not. It stores a value that originally had type const
char *, converted into a value of type char *, into an object of type
char *. You're confusing accessing an object with wrong lvalue type
with conversions.

> > Accessing a const char * as char *. I would prefer in general
> > we just #undef any of the const-stuff-tg macros in files that use
> > them, or just have src/include/string.h always do that. (Not really
> > needed since musl source is written in c99 not c23, but it would be
> > nice to have it also compile with c11 and c23 compilers, so I think
> > the #undef is useful.)
> 
> I am not sure that I understand how you think that should work, we
> have to provide these tg macros to our users, don't we?

Yes but we don't have to use them inside the implementation.
src/include/* are a layer on top of the public headers for
implementation-internal use.

> In any case, I prefer to mark such positions explicitly with something
> like `(strchr)(r, '\n')` as in line 222 of the code that you are
> refering to. All of this is marked as obsolescent in 7.26.5.1

I'd rather just fix it in one place (the implementation-internal
header) so we don't have to worry about it.

Rich