frankpfenning / C0

C0 Language
4 stars 0 forks source link

Runtimes don't support valid NULL-returning calloc implementaitons #60

Open robsimmons opened 10 years ago

robsimmons commented 10 years ago

Alex Cappiello and I worked this out thinking about the VM. It is implementation-defined whether the following code in the bare and c0rt runtime will cause a c0_abort_mem if we are allocating a struct with no fields, because calloc is allowed to return NULL in this case.

c0_pointer c0_alloc(size_t elt_size) { int* p = calloc(1, elt_size); if (p == NULL) c0_abort_mem("allocation failed"); return (void *)p; }

In the safe C0 runtimes (bare and c0rt), a valid implementation of calloc might cause a c0_abort_mem on a struct with zero fields. Even if this was fixed in the obvious way, in order to maintain the desirable property that different allocations result in unequal pointers, size-zero struct allocations must allocate non-zero memory. I think the simplest solution would be to disallow structs with no fields - pedantic C99 appears to do this as well, I was surprised that they were allowed in C0.

Because c0_arrays are allocated as one contiguous block, this is not a problem for arrays of length zero in the safe C0 runtimes, though under the unsafe runtime it is allowed that alloc_array(int, 0) == alloc_array(int, 0) if the calloc implementation returns NULL.

P.S. I'd argue this is also a problem with the xalloc C libraries used for 122 - it shouldn't be a show-stopping error for a malloc or calloc of size 0 to return NULL.

frankpfenning commented 10 years ago

I think this is handled correctly in the C0 standard runtime (at least). From cc0/c0rt.c:

c0_pointer c0_alloc(size_t size) { if (!size) size = 1;

void* p = GC_MALLOC(size); if (!p) c0_abort_mem("allocation failed"); bzero(p, size); return p; }

Probably should be fixed in the bare runtime, although we don't seem to use 'bar' in practice.

All of pointer-related reasoning in C0 assumes that allocation returns something new each time (if it is succeeds), and that it is non-NULL. Allocation of an empty struct can be used as a gensym.

I think the xalloc C libraries may be a different issue. Given the behavior of C, it might make sense to require, per contract, that the allocation is non-zero to avoid undefined behavior?

On Sat, Nov 30, 2013 at 1:39 PM, Robert J. Simmons <notifications@github.com

wrote:

Alex Cappiello and I worked this out thinking about the VM. It is implementation-defined whether the following code in the bare and c0rt runtime will cause a c0_abort_mem if we are allocating a struct with no fields, because calloc is allowed to return NULL in this case.

c0_pointer c0_alloc(size_t elt_size) { int* p = calloc(1, elt_size); if (p == NULL) c0_abort_mem("allocation failed"); return (void *)p; }

In the safe C0 runtimes (bare and c0rt), a valid implementation of calloc might cause a c0_abort_mem on a struct with zero fields. Even if this was fixed in the obvious way, in order to maintain the desirable property that different allocations result in unequal pointers, size-zero struct allocations must allocate non-zero memory. I think the simplest solution would be to disallow structs with no fields - pedantic C99 appears to do this as well, I was surprised that they were allowed in C0.

Because c0_arrays are allocated as one contiguous block, this is not a problem for arrays of length zero in the safe C0 runtimes, though under the unsafe runtime it is allowed that alloc_array(int, 0) == alloc_array(int, 0) if the calloc implementation returns NULL.

P.S. I'd argue this is also a problem with the xalloc C libraries used for 122 - it shouldn't be a show-stopping error for a malloc or calloc of size 0 to return NULL.

— Reply to this email directly or view it on GitHubhttps://github.com/frankpfenning/C0/issues/60 .

Frank Pfenning, Professor and Head Department of Computer Science Carnegie Mellon University Pittsburgh, PA 15213-3891

http://www.cs.cmu.edu/~fp +1 412 268-6343 GHC 7019

robsimmons commented 10 years ago

I'll make the analogous change to bare - bare is still linked with the VM and Coin.

The problem with changing xalloc as you describe is that it'll mean that the VM will necessarily be more complicated - we do have test cases where we allocate length-zero arrays, and the easy way of handling this will cause a separate zero-length allocation for the owned array.