This is an archive of the discontinued LLVM Phabricator instance.

[LLVM-C] Begin to Expose A More General Binary Interface
ClosedPublic

Authored by CodaFi on Apr 5 2019, 8:47 AM.

Details

Summary

Provides a new type, LLVMBinaryRef, and a binding to llvm::object::createBinary for more general interoperation with binary files than LLVMObjectFileRef. It also provides the proper non-consuming API for input buffers and populates an out parameter for error handling if necessary - two things the previous API did not do.

In a follow-up, I'll define section and symbol iterators and begin to build upon the existing test infrastructure.

This patch is a first step towards deprecating that API and replacing it with something more robust.

Diff Detail

Event Timeline

CodaFi created this revision.Apr 5 2019, 8:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 5 2019, 8:47 AM

Can you provide a bit more context for this change? It looks good, I just want to understand the plan here in more detail.

CodaFi added a comment.EditedApr 5 2019, 12:35 PM

The ObjectFile APIs seem to have rotted a bit. There's a bunch of pitfalls when working with the thing like the fact that it goes out of its way to move the input buffer despite the create APIs taking non-owning references to the buffer in C++, accessors that no longer do anything useful (LLVMGetRelocationValueString) and inconsistent error handling routines (some functions return NULL, LLVMGetSymbolName and LLVMGetSymbolName reports a fatal error). On top of that, it's not actually a general interface to ObjectFile - it can't handle Mach-O Universal binaries as inputs for one.

I want to re-approach these APIs from the perspective of llvm::object::Binary first, then specialize them for ObjectFiles, Archives, etc. The idea being to eventually grow a Binary.h once we can provide a more comprehensive API.

SGTM, just one question about ownership.

llvm/include/llvm-c/Object.h
74

Who owns the returned memory buffer?

CodaFi marked an inline comment as done.Apr 5 2019, 12:47 PM
CodaFi added inline comments.
llvm/include/llvm-c/Object.h
74

Good point, it's created from a MemoryBuffer Ref, so whoever owns the memory buffer that created this binary file owns the returned memory buffer. Deallocating the buffer returned from this function will also deallocate that buffer and cause all kinds of fun nosebats to take flight.

I'll update the docs.

CodaFi marked an inline comment as done.Apr 5 2019, 1:41 PM
CodaFi added inline comments.
llvm/include/llvm-c/Object.h
74

Ugh, no, that's wrong. It's a non-owning reference to the actually block of memory but it's still a pointer value that needs to be freed by the caller.

CodaFi updated this revision to Diff 193963.Apr 5 2019, 1:49 PM

Rename LLVMBinaryGetMemoryBuffer to LLVMBinaryCopyMemoryBuffer to better convey ownership semantics. Update the docs.

CodaFi marked an inline comment as done.Apr 5 2019, 1:50 PM
This revision is now accepted and ready to land.Apr 5 2019, 1:53 PM
This revision was automatically updated to reflect the committed changes.