> On Nov 18, 2016, at 3:45 PM, Mehdi Amini <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 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>
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
