Re: llvm 13 memcpy optimiser breaks lang/node

On Tue, Jan 25, 2022 at 07:51:46AM +1000, Jonathan Matthew wrote:
> On Mon, Jan 24, 2022 at 10:03:19AM -0700, Theo de Raadt wrote:
> > Stuart Henderson <stu@spacehopper.org> wrote:
> >
> > > On 2022/01/24 22:17, Jonathan Matthew wrote:
> > > > The proposed update to lang/node makes this irrelevant, but I thought I'd send
> > > > it anyway since it may come up elsewhere too.
> > > >
> > > > I noticed that on one system, 'npm install less' would abort, logging
> > > > 'node: backwards memcpy', but on another it worked fine. Eventually I
> > > > figured out this was because the working one had packages built with llvm 11,
> > > > the other with llvm 13 packages, and llvm 13's memcpy optimiser was turning
> > > > a series of small memcpys in node's bundled zlib into one larger one, without
> > > > identifying that the src and dest of the larger memcpy could overlap.
> > > >
> > > > Compiling the bundled zlib with -fno-builtin-memcpy prevents it from doing
> > > > that, which fixes npm.
> > >
> > > If we can't trust the compiler's builtin memcpy to do the right thing then
> > > it probably needs disabling completely...
> >
> > Right. We cannot conclude this is only in one specific piece of software.
> >
> > Has anyone reached opened a bug report with clang?
>
> I'll extract the memcpy loop out and try to turn it into something I can
> include in a bug report.

While doing that I noticed the zlib code was applying the 'restrict' qualifier
to one of the pointers in one case where both the source and destination of
the copy were in the output buffer. The compiler can assume there's no
pointer aliasing in that case, so it used memcpy rather than memmove.
So the fault here is really with node, it just happens that llvm's optimiser
got more aggressive between 11 and 13.

chromium fixed this a while ago here: https://chromium.googlesource.com/chromium/src.git/+/e0f88a903fdcb6c772de1929834a73d1662d509a%5E%21/ but node hasn't
applied that change to their copy yet. I've opened an issue against node
suggesting that they should do so.