[llvm-dev] [RFC] Runtime checks for ABI breaking build of LLVM

[Mehdi Amini via llvm-dev]

> On Nov 16, 2016, at 12:05 PM, Jonathan Roelofs <jonathan at codesourcery.com> wrote:
> 
> 
> 
> On 11/16/16 11:48 AM, Mehdi Amini via llvm-dev wrote:
>> Hi all,
>> 
>> An issue that come up from time to time and has cost hours for debug for
>> many of us and users of LLVM is that an assert build isn’t ABI
>> compatible with a release build.
>> 
>> The CMake flags that controls this behavior is LLVM_ABI_BREAKING_CHECKS (
>> 
>> *LLVM_ABI_BREAKING_CHECKS*:STRING
>>    Used to decide if LLVM should be built with ABI breaking checks or
>>    not. Allowed values
>>    are WITH_ASSERTS (default), FORCE_ON and FORCE_OFF.  WITH_ASSERTS turns
>>    on ABI breaking checks in an assertion enabled
>>    build.  FORCE_ON (FORCE_OFF) turns them on (off) irrespective of
>>    whether normal (NDEBUG-based) assertions are enabled or not. A
>>    version of LLVM built with ABI breaking checks is not ABI compatible
>>    with a version built without it.
>> 
>> 
>> I propose to add a runtime check to detect when we have incorrectly
>> setup build.
>> 
>> The idea is that every translation unit that includes such header will
>> get a weak definition of a symbol with an initializer calling the
>> runtime check. The symbol name would be different if the ABI break is
>> enabled or not.
> 
> Can it be made into a link-time check instead? I'm imagining something like:

I’d love to, but didn’t find a universal solution unfortunately :(

>  #if LLVM_ENABLE_ABI_BREAKING_CHECKS
>  extern int EnableABIBreakingChecks;
>  __attribute__((weak)) int *VerifyEnableABIBreakingChecks = &EnableABIBreakingChecks;
>  #else
>  extern int DisableABIBreakingChecks;
>  __attribute__((weak)) int *VerifyDisableABIBreakingChecks = &VDisableABIBreakingChecks;
>  #endif
> 
> in llvm-config.h.cmake, and:
> 
>  #if LLVM_ENABLE_ABI_BREAKING_CHECKS
>  int EnableABIBreakingChecks;
>  #else
>  int DisableABIBreakingChecks;
>  #endif
> 
> in Error.cpp.
> 
> Then it'll only link if Error.cpp's TU's setting of LLVM_ENABLE_ABI_BREAKING_CHECKS matches that of the TU that includes llvm-config.h

It seems that this work, I thought I tried exactly this but got lost on the whiteboard at some point!

Maybe because one drawback that I tried to avoid is that the export-list of a LLVM dylib would depend on the value of LLVM_ENABLE_ABI_BREAKING_CHECKS with this.

Thanks,

— 
Mehdi





> 
> 
>> 
>> The runtime check maintains two boolean to track if it there is in the
>> image at least a translation unit for each value of this flag. If both
>> flags are set the process is aborted.
>> 
>> The cost is *one* static initializer per DSO (or per image I believe).
>> 
>> A straw-man patch (likely not windows compatible because of weak) is:
>> 
>> diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake
>> b/llvm/include/llvm/Config/llvm-config.h.cmake
>> index 4121e865ea00..4274c942d3b6 100644
>> --- a/llvm/include/llvm/Config/llvm-config.h.cmake
>> +++ b/llvm/include/llvm/Config/llvm-config.h.cmake
>> @@ -80,4 +80,18 @@
>> /* LLVM version string */
>> #define LLVM_VERSION_STRING "${PACKAGE_VERSION}"
>> 
>> +
>> +#ifdef __cplusplus
>> +namespace llvm {
>> +bool setABIBreakingChecks(bool Enabled);
>> +__attribute__((weak))
>> +#if LLVM_ENABLE_ABI_BREAKING_CHECKS
>> +bool
>> +ABICheckEnabled = setABIBreakingChecks(true);
>> +#else
>> +bool ABICheckDisabled = setABIBreakingChecks(true);
> 
> Do you mean `false` here ^ ?
> 
>> +#endif
>> +}
>> +#endif
>> +
>> #endif
>> diff --git a/llvm/lib/Support/Error.cpp b/llvm/lib/Support/Error.cpp
>> index 7436a1fd38ee..151fcdcbfb27 100644
>> --- a/llvm/lib/Support/Error.cpp
>> +++ b/llvm/lib/Support/Error.cpp
>> @@ -112,3 +112,17 @@ void report_fatal_error(Error Err, bool GenCrashDiag) {
>> }
>> 
>> }
>> +
>> +
>> +bool llvm::setABIBreakingChecks(bool Enabled) {
>> +  static char abi_breaking_checks_enabled = 0;
>> +  static char abi_breaking_checks_disabled = 0;
>> +  if (Enabled)
>> +    abi_breaking_checks_enabled = 1;
>> +  else
>> +    abi_breaking_checks_disabled = 1;
>> +  if (abi_breaking_checks_enabled && abi_breaking_checks_disabled)
>> +    report_fatal_error("Error initializing LLVM: mixing translation
>> units built"
>> +                       "with and without LLVM_ABI_BREAKING_CHECKS");
>> +  return true;
>> +}
>> 
>> 
>> 
>> —
>> Mehdi
>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> 
> 
> -- 
> Jon Roelofs
> jonathan at codesourcery.com <mailto:jonathan at codesourcery.com>
> CodeSourcery / Mentor Embedded

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20161116/a2b67b67/attachment.html>