Page MenuHomePhabricator

[LLVM-C] Move and Clean Up The Instrumentation Pass Bindings
Needs ReviewPublic

Authored by CodaFi on Feb 27 2019, 12:06 PM.

Details

Summary

Moves the bindings to ASAN, MSAN, TSAN, and DFSAN out of the Go bindings and into the C bindings.

Additionally, add bindings to Code Coverage Sanitizer.

The notable addition to the API is the options wrappers. Memory Sanitizer, and Code Coverage Sanitizer offer structures that encapsulate their configuration options. As these structures are subject to change in future releases of LLVM, the bindings have corresponding opaque wrapper structs that have explicit allocation/destruction routines. This way, we can support future added options by adding bindings that act as setters. We support future removed options by marking their corresponding parameter dead in the documentation/deprecating the setter wrapper.

Diff Detail

Event Timeline

CodaFi created this revision.Feb 27 2019, 12:06 PM
whitequark added inline comments.Mar 11 2019, 12:42 AM
llvm/include/llvm-c/Instrumentation.h
46

I think enormous parameter lists are a very error-prone way to do this kind of binding. What about making a C structure reflecting the currently recognized options, and passing the length of this structure to the LLVMCreate*Options functions? This is similar to what WinAPI is doing.

I.e.

struct SanitizerCoverageOptions {
  unsigned CoverageType;
  LLVMBool IndirectCalls;
  // ...
}

LLVMSanitizerCoverageOptions LLVMCreateSanitizerCoverageOptions(
    SanitizerCoverageOptions *Options,
    unsigned OptionsLength
);

So, we could deprecate options by making them dead (and hopefully there's some attribute we can use to cause a warning to be emitted on access), and add new ones by appending them towards the end of the structure. For C/C++ users of the C API as well as any autogenerated bindings, the upgrades are now completely transparent. For non-autogenerated bindings, it's still less work to add new options.

@echristo @deadalnix What do you think?

CodaFi added a comment.Apr 5 2019, 8:52 AM

Looks like ESan is dead https://reviews.llvm.org/rL355862. I'll update this patch.

CodaFi updated this revision to Diff 193894.Apr 5 2019, 8:56 AM
CodaFi edited the summary of this revision. (Show Details)

Remove support for Efficiency Sanitizer