[llvm-dev] UB in MemoryBufferMMapFile

[Justin Bogner via llvm-dev]

In MemoryBuffer::init, we have an assert that reads the memory at
`BufEnd`, which is one past the end of some memory region:

from lib/Support/MemoryBuffer.cpp:45:
> void MemoryBuffer::init(const char *BufStart, const char *BufEnd,
>                         bool RequiresNullTerminator) {
>   assert((!RequiresNullTerminator || BufEnd[0] == 0) &&
>          "Buffer is not null terminated!");
>   BufferStart = BufStart;
>   BufferEnd = BufEnd;
> }

However, this can be, and often is, one past an allocated region, since
in MemoryBufferMMapFile we mmap `Len` bytes and then call this with
`BufEnd = Start + Len`:

from lib/Support/MemoryBuffer.cpp:210:
>   MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
>                        uint64_t Offset, std::error_code &EC)
>       : MFR(FD, sys::fs::mapped_file_region::readonly,
>             getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
>     if (!EC) {
>       const char *Start = getStart(Len, Offset);
>       init(Start, Start + Len, RequiresNullTerminator);
>     }
>   }

This is UB, since we're reading one past an allocated region, and I have
an internal test case where I'm seeing it crash because that memory
happens to be unreadable.

There are three options here:

1. Remove the assert - it isn't possible to write correctly.
2. Remove the RequiresNullTerminator argument from MemoryBufferMMapFile
   and hard code it to false.
3. Allocate an extra byte here and zero it.

I'm not sure which is best. Thoughts?