Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization of malloc/free Produces Undefined Behavior #121219

Closed
jonathan-gruber-jg opened this issue Dec 27, 2024 · 1 comment
Closed

Optimization of malloc/free Produces Undefined Behavior #121219

jonathan-gruber-jg opened this issue Dec 27, 2024 · 1 comment

Comments

@jonathan-gruber-jg
Copy link

Optimization of malloc and free calls can produce undefined behavior.

Consider this minimal test case:

#include <stddef.h>
#include <stdint.h>
#include <stdlib.h>

#define TOO_LARGE_SIZE ((size_t)PTRDIFF_MAX + 1) /* Request size greater than PTRDIFF_MAX. */

ptrdiff_t test(void) {
	char *const p = malloc(TOO_LARGE_SIZE);

	if (!p) {
		return 0;
	}

	char *const q = p + TOO_LARGE_SIZE;

	const ptrdiff_t ret = q - p;

	free(p);

	return ret;
}

Critically, glibc's malloc unconditionally fails for requests larger than PTRDIFF_MAX, so, with glibc, the above function test would always return 0. However, on all nonzero optimization levels, Clang assumes that the call to malloc will unconditionally succeed, effectively deleting the if-statement. As a result, the function invokes undefined behavior by computing a pointer subtraction whose result overflows (since TOO_LARGE_SIZE > PTRDIFF_MAX). If the if-statement were not deleted, the body of the if-statement would always be executed, thereby avoiding invoking undefined behavior.

On all nonzero optimization levels, Clang generates (x86_64, Intel syntax) assembly similar to the following:

test:
	movabs	rax, -9223372036854775808
	ret

Host system type: Arch Linux, x86_64 (with glibc, of course).
Clang version: official Arch Linux package clang 18.1.8-5.

@hstk30-hw
Copy link
Contributor

Instruction *InstCombinerImpl::visitAllocSite(Instruction &MI) {
assert(isa<AllocaInst>(MI) || isRemovableAlloc(&cast<CallBase>(MI), &TLI));
// If we have a malloc call which is only used in any amount of comparisons to
// null and free calls, delete the calls and replace the comparisons with true
// or false as appropriate.
// This is based on the principle that we can substitute our own allocation
// function (which will never return null) rather than knowledge of the
// specific function being called. In some sense this can change the permitted
// outputs of a program (when we convert a malloc to an alloca, the fact that
// the allocation is now on the stack is potentially visible, for example),
// but we believe in a permissible manner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants