[PATCH] D16390: Introduce allocsize with arbitrary expressions

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 18:14:45 PST 2016


george.burgess.iv created this revision.
george.burgess.iv added a subscriber: llvm-commits.
Herald added a subscriber: joker.eph.

**Note: I'm not thrilled with the compromises this patch makes; it's more here to show an approach we can take to get allocsize to support arbitrary expressions in LLVM. If we decide that the changes made are acceptable, I'll tidy this up and request a more in-depth review.**

This patch introduces a more general version of the `allocsize` attribute proposed in D14933. It would allow code like the following to be lowered to LLVM:

```
// Allocates an ObjectHeader, with N bytes of space for an Object after it. Returns a pointer
// right after the header.
Object *object_malloc(int N) __attribute__((alloc_size_ex(N + sizeof(ObjectHeader), sizeof(ObjectHeader)));

Object *checked_malloc(int N) {
	Object *O = object_malloc(N);
	if (__builtin_sizeof(O, 0) < N)
		__builtin_trap();
	return O;
}
```

The resulting LLVM IR would look something like (simplified; clang would probably throw in allocas, etc.):

```
declare i8* @object_malloc(i32 %N) allocsize({i32, i32}(i32)* @__object_malloc_alloc_size_fn, [0])

; Assuming sizeof(ObjectHeader) == 8
define private {i32, i32} @__object_malloc_alloc_size_fn(i32 %N) unnamed_addr {
	%1 = add i32 %N, 8
	%2 = insertelement {i32, i32} {i32 0, i32 8}, i32 %N, 0
	ret {i32, i32} %2
}

define i8* @checked_malloc(i32 %N) {
	%1 = call i8* @object_malloc(i32 %N)
	%2 = call i32 @llvm.objectsize.i32.p0i8(i8* %1)
	%3 = icmp lt %2, %N
	br i1 %3, label %Trap, label %Return
Trap:
	call void @llvm.trap()
	unreachable
Return:
	ret i8* %1
}

@llvm.compiler.used = appending global [1 x {i32, i32}(i32)*] [
	@__object_malloc_alloc_size_fn
]
```

This design buys us moderately low-overhead support for `allocsize` with arbitrary expressions, and doesn’t unreasonably impact code size. (read: Ideally, `@__object_malloc_alloc_size_fn` should be discarded by the linker, so the only issue we have is that allocation functions tagged with different `allocsize` functions can’t be deduped. In practice, this probably isn’t an issue, because if two functions return different looking results, they probably don’t have identical bodies.)

The implementation of it, however, is not ideal. It requires the following:
- Attributes must become mutable, which makes relative ordering unstable across RAUWs, and breaks uniquing a bit.
- Attributes aren’t `User`s, so an external mechanism is needed to handle the lifetime of e.g. `@__object_malloc_alloc_size_fn` from above. `@llvm.compiler.used` is the current suggestion.
  - N.B. `AssertingVH` can't be used with ^, because `Function`s are deleted before `Attribute`s. So we wait to `assert(false)` on deletion until the user actually tries to use the Attribute. This is a great place for subtle bugs to creep in unnoticed. This can be fixed by having a `IsBeingDeleted` member in `LLVMContextImpl` that's only true when `LLVMContextImpl` is being destructed, but... ew.
- Type safety isn't ideal; `Attribute::PseudoCall` has a `Constant *` instead of a `Function *` for `Fn` because BitcodeReader hands us a non-`Function` placeholder. Fixing it probably involves duplicating a lot of methods.
- Custom trivial call folding had to be added, because we never *actually* call e.g. `@__object_malloc_alloc_size_fn` in the code. In some cases, it's correct to emit this call; in others, it’s incorrect. Namely, when the `allocsize` function has side-effects/can trap/...
- And more!

Any suggestions for how to better solve these problems are highly appreciated. The best overall solution seems to be handling most/all of this in Clang alone. We lose the (probably substantial) benefits of inlining/general optimizations, which makes `alloc_size_ex` less helpful, but doing so would almost certainly be a less error-prone approach.

http://reviews.llvm.org/D16390

Files:
  docs/HowToUseAttributes.rst
  docs/LangRef.rst
  include/llvm/Analysis/ConstantFolding.h
  include/llvm/Bitcode/LLVMBitCodes.h
  include/llvm/IR/Attributes.h
  include/llvm/IR/Attributes.td
  lib/Analysis/ConstantFolding.cpp
  lib/Analysis/MemoryBuiltins.cpp
  lib/AsmParser/LLLexer.cpp
  lib/AsmParser/LLParser.cpp
  lib/AsmParser/LLParser.h
  lib/AsmParser/LLToken.h
  lib/Bitcode/Reader/BitcodeReader.cpp
  lib/Bitcode/Writer/BitcodeWriter.cpp
  lib/IR/AttributeImpl.h
  lib/IR/Attributes.cpp
  lib/IR/LLVMContextImpl.h
  lib/IR/Verifier.cpp
  test/Bitcode/allocsize.ll
  test/Transforms/InstCombine/allocsize.ll
  test/Verifier/alloc-size-failedparse.ll
  test/Verifier/allocsize.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16390.45478.patch
Type: text/x-patch
Size: 80808 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160121/630f2a58/attachment.bin>


More information about the llvm-commits mailing list