[PATCH] Value.h, linkage, and NDEBUG differences between llvm lib build-time and client usage

[Todd Fiala via llvm-commits]

Added missing [PATCH] to subject...

On Tue, Feb 2, 2016 at 3:38 PM, Todd Fiala <todd.fiala at gmail.com> wrote:

> Hello all!
>
> On LLDB, if I build LLVM and clang with Release + Asserts, and then build
> LLDB in Debug mode, I find the llvm::Value class presents itself in a way
> that creates a linkage issue.
>
> I fixed it with the diff at the end of the email.
>
> Essentially what happens is that during the LLVM/clang lib build step,
> NDEBUG is defined, so Value::assertModuleIsMaterialized() is defined inline
> (empty method).  This method is called via inline definitions elsewhere in
> the Value class that clients may call.
>
> Now, when a client of this library uses it (in this case LLDB), and that
> client is being built without NDEBUG, then the inline definitions in the
> header for the Value class are seeing the definition of
> Value::assertModuleIsMaterialized() that is claiming to be *not* inlined.
> However, when the LLVM lib was compiled, no such method was defined and
> therefore is not in the lib.  This leads to the symbol missing at link time
> because LLDB's usage of the header with the inline definitions expected to
> find a non-inlined version of the assertModuleIsMaterialized() method.
>
> The header is broken w/r/t whether NDEBUG is defined at the time the LLVM
> library is built vs. when it is used by a client.
>
> The patch below is a simplistic solution that says
> "assertModuleIsMaterialized() is always defined in the .cpp file".
> Therefore it makes it always a function call, and the call exists
> regardless of the NDEBUG state of the caller vs. the time the LLVM lib was
> built.  However, this also means all those assert calls in the
> header-implemented methods now turn into function calls instead of no-ops.
> If there's a better pattern for handling this in LLVM, feel free to suggest.
>
> Thanks!
>
> -Todd
>
> diff --git a/include/llvm/IR/Value.h b/include/llvm/IR/Value.h
> index 348ad97..c2997e9 100644
> --- a/include/llvm/IR/Value.h
> +++ b/include/llvm/IR/Value.h
> @@ -280,11 +280,7 @@ public:
>    // when using them since you might not get all uses.
>    // The methods that don't start with materialized_ assert that modules
> is
>    // fully materialized.
> -#ifdef NDEBUG
> -  void assertModuleIsMaterialized() const {}
> -#else
>    void assertModuleIsMaterialized() const;
> -#endif
>
>    bool use_empty() const {
>      assertModuleIsMaterialized();
> diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp
> index 250f451..955204a 100644
> --- a/lib/IR/Value.cpp
> +++ b/lib/IR/Value.cpp
> @@ -314,8 +314,8 @@ void Value::takeName(Value *V) {
>      ST->reinsertValue(this);
>  }
>
> -#ifndef NDEBUG
>  void Value::assertModuleIsMaterialized() const {
> +#ifndef NDEBUG
>    const GlobalValue *GV = dyn_cast<GlobalValue>(this);
>    if (!GV)
>      return;
> @@ -323,8 +323,10 @@ void Value::assertModuleIsMaterialized() const {
>    if (!M)
>      return;
>    assert(M->isMaterialized());
> +#endif
>  }
>
> +#ifndef NDEBUG
>  static bool contains(SmallPtrSetImpl<ConstantExpr *> &Cache, ConstantExpr
> *Expr,
>                       Constant *C) {
>    if (!Cache.insert(Expr).second)
>
>


-- 
-Todd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160202/d8b2338f/attachment.html>