Invalid optimization: clang makes invalid assumptions about malloc

[bhaible]

When optimizing, clang apparently assumes that a malloc invocation does not change errno. This assumption is wrong: on all platforms, malloc sets errno (usually to ENOMEM) when it returns NULL.

How to reproduce:

Save this as foo.c.

#include <stdlib.h> /* malloc */
#include <errno.h> /* errno */
#include <stdio.h> /* printf */
#include <stdint.h> /* SIZE_MAX */

int
main ()
{
  errno = 0;
  void *volatile p = malloc (SIZE_MAX / 10);
  int err = errno;
  printf ("errno=%d\n", err);
}

Then:

$ clang -O2 foo.c && ./a.out
errno=0
$ clang foo.c && ./a.out
errno=12

$ clang --version
clang version 19.1.0 (/home/runner/work/llvm-project/llvm-project/clang a4bf6cd7cfb1a1421ba92bca9d017b49936c55e4)
Target: x86_64-unknown-linux-gnu
Thread model: posix

[bhaible]

Likewise for calloc.

[antoniofrighetto]

IR before GVN: https://llvm.godbolt.org/z/qchWcEfYo. Seems like MemDep doesn't take into account that %call may be clobbered by the call to malloc, although I'm not completely sure it's MemDep role to establish that. Either way, I think GVN shouldn't simplify to values coming from global variables w/ thread-local storage, but we don't seem to treat errno as such. Maybe cc/ @nikic

[nikic]

@antoniofrighetto We mark malloc as memory(inaccessiblemem: readwrite), which is not correct wrt errno. I expect that not doing this would be a huge pessimization though, because it basically means that malloc can now clobber all escaped memory (like, say, a function argument) because we can't exclude that it may be a pointer to errno.

[antoniofrighetto]

@antoniofrighetto We mark malloc as memory(inaccessiblemem: readwrite), which is not correct wrt errno.

Oh, sure thing, thanks. I wonder if it could still make sense to somehow treat errno differently / have specialized logic here when searching for available values to materialize, though it sounds a bit of a workaround.

[bhaible]

I wonder if it could still make sense to somehow treat errno differently

I would hope, yes. The majority of pointers are not pointers to int and therefore cannot alias errno.

[nikic]

We recently adopted something like this by emitting int TBAA for FP libcalls. I don't think we'd want to do the same for malloc though, it's even more fishy in that case.

I'd be open to adding an errno location to MemoryEffects and have that interact with other TBAA. (cc @andykaylor as we had a related discussion at the conference.)

I would hope, yes. The majority of pointers are not pointers to int and therefore cannot alias errno.

Even if it's not a majority, I expect a significant fraction are, esp. in benchmarky code :) There's also the issue that TBAA is a C concept, so languages that don't have strict aliasing would pessimize much more. Though hopefully they already use a different allocator than malloc.