Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 4 Jun 2022 14:16:36 -0400
From: Rich Felker <dalias@...c.org>
To: Pascal Cuoq <cuoq@...st-in-soft.com>
Cc: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: Re: [PATCH] fix undefined behavior from large shifts

On Sat, Jun 04, 2022 at 06:04:15PM +0000, Pascal Cuoq wrote:
> 
> > On 4 Jun 2022, at 19:26, NRK <nrk@...root.org> wrote:
> > 
> > +    uint32_t nw = (uint32_t)n[0]<<24 | (uint32_t)n[1]<<16 | n[2]<<8;
> > +    uint32_t hw = (uint32_t)h[0]<<24 | (uint32_t)h[1]<<16 | h[2]<<8;
> 
> If it were a goal to support 16-bit ints in musl, then your patch
> would still have UB by shifting a 1 into the sign bit with
> {h,n}[2]<<8, which in C is a form of signed arithmetic overflow (the
> C++ standard makes a special case for this situation but the C
> standard doesn't).
> 
> However, I do not think it is a goal to support narrower that 32-bit
> ints in musl, and the original code is free of UB in these
> conditions.

Indeed, musl code assumes int is at least 32-bit since it assumes the
class of ABIs it supports.

It's arguable that the source files that are "pure library" code that
don't have anything to do with being part of a unified implementation
code base could/should be written with even fewer assumptions, but
there's a lot of subtle pain in environments where default promotions
don't do what you expect, and I don't think it's a good use of time to
try to maintain that in a good state. If you're really targeting some
tiny 8bit microcontroller or whatever, you don't want the code that's
in musl; you want either even more naive implementations of these
functions for minimal code size, or hand written asm.

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.