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. — Mehdi > On Nov 16, 2016, at 12:12 PM, Mehdi Amini via llvm-dev > <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 >>> >> >> -- >> 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
