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

Unsoundness in MemoryBuffer::create_from_memory_range #516

Open
fuzzypixelz opened this issue Jul 18, 2024 · 1 comment
Open

Unsoundness in MemoryBuffer::create_from_memory_range #516

fuzzypixelz opened this issue Jul 18, 2024 · 1 comment

Comments

@fuzzypixelz
Copy link

Describe the Bug

MemoryBuffer::create_from_memory_range calls LLVMCreateMemoryBufferWithMemoryRange with RequiresNullTerminator set to false, while an LLVM memory buffer should always be null-terminated:

https://github.com/llvm/llvm-project/blob/adacb5010f5ca6e923b3cf2d8ea47cbaab96099d/llvm/include/llvm/Support/MemoryBuffer.h#L41-L51

Please note that setting RequiresNullTerminator to true won't solve the issue; LLVM will simply throw an error. Instead, the function signature needs to be changed to require null-terminated buffers. There can be no zero-copy implementation of MemoryBuffer::create_from_memory_range for arbitrary &[u8].

The RequiresNullTerminator argument is truly baffling to me. It seems to mean "if true, check if the buffer is null-terminated" and not "I don't need this memory buffer to be null terminated" because it is always assumed to be so (perhaps you want to patch it after construction):

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/MemoryBuffer.cpp#L49-L57

To Reproduce

Create a memory buffer with MemoryBuffer::create_from_memory_range and attempt to use it in Context::create_module_from_ir for instance. The parser will behave oddly as it tries to read past the buffer.

Expected Behavior

MemoryBuffer::create_from_memory_range should always pass a null-terminated buffer to LLVMCreateMemoryBufferWithMemoryRange and set RequiresNullTerminator to true.

As this requires a copy in the general case, it should take a type that represents a buffer b of length N where b[N - 1] == b'\0' and set the buffer length to N-1 in LLVMCreateMemoryBufferWithMemoryRange.

Otherwise the documentation should be updated to ask the user to do the the necessary manipulation themselves before calling the function.

LLVM Version (please complete the following information):

  • LLVM Version: 17.0.6
  • Inkwell Branch Used: master

Desktop (please complete the following information):

  • OS: macOS 14.2.1

Additional Context

N/A

@TheDan64
Copy link
Owner

Dupe of #185 I think

@TheDan64 TheDan64 added this to the 0.6.0 milestone Aug 4, 2024
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

2 participants