[lldb-dev] [llvm-dev] [RFC] Runtime checks for ABI breaking build of LLVM
Chris Bieneman via lldb-dev
lldb-dev at lists.llvm.org
Sat Nov 19 23:25:34 PST 2016
Mehdi,
I think your second approach is the better option. Going with the first option means we would need to remove references to llvm-config.h in ADT, which I don't think is a simple task.
-Chris
> On Nov 18, 2016, at 4:23 PM, Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
>
>> On Nov 18, 2016, at 3:45 PM, Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
>>
>> Hi,
>>
>> I had to revert it, because it breaks building LLDB on MacOS.
>>
>> It turns out it would break any client that is including llvm-config.h but not linking to libLLVMSupport.
>> So either:
>>
>> - we shouldn’t allow to include llvm-config.h without linking to LLVM, in which case I need to look a bit closer as of why lldb includes this header in a context where they don’t link LLVM.
>> - we should allow to include llvm-config.h without linking to LLVM (at least libSupport). In which case we need a new solution for this feature: it can be to use another header dedicated to this flag, that would be included only from headers that contain the ABI break.
>
> The second approach is implemented here: https://reviews.llvm.org/D26876 <https://reviews.llvm.org/D26876>
>
> I extracted the LLVM_ENABLE_ABI_BREAKING_CHECKS to its own header to have a fine granularity on which client needs to include the check.
>
> —
> Mehdi
>
>
>
>
>>
>>
>>> On Nov 16, 2016, at 12:12 PM, Mehdi Amini via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>>>
>>>>
>>>> On Nov 16, 2016, at 12:05 PM, Jonathan Roelofs <jonathan at codesourcery.com <mailto: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 <mailto:llvm-dev at lists.llvm.org>
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>>>>>
>>>>
>>>> --
>>>> Jon Roelofs
>>>> jonathan at codesourcery.com <mailto:jonathan at codesourcery.com>
>>>> CodeSourcery / Mentor Embedded
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20161119/0f765a15/attachment-0001.html>
More information about the lldb-dev
mailing list