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-...@lists.llvm.org> wrote: > > >> On Nov 18, 2016, at 3:45 PM, Mehdi Amini <mehdi.am...@apple.com >> <mailto:mehdi.am...@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-...@lists.llvm.org <mailto:llvm-...@lists.llvm.org>> wrote: >>> >>>> >>>> On Nov 16, 2016, at 12:05 PM, Jonathan Roelofs <jonat...@codesourcery.com >>>> <mailto:jonat...@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-...@lists.llvm.org <mailto:llvm-...@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 >>>> jonat...@codesourcery.com <mailto:jonat...@codesourcery.com> >>>> CodeSourcery / Mentor Embedded >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-...@lists.llvm.org <mailto:llvm-...@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-...@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
