[PATCH] D33705: [CGVTables] Finalize SP before attempting to clone it

[David Blaikie via cfe-commits]

If SPs are now all finalized early - I'm assuming there's some other code
that finalizes SPs that's no longer needed?

Also - this seems like a layering/separation issue - does other code call
into the DI as casually/explicitly as this code being added to CGVTables?
(Or should the callback go through some more generic entry point in
CGDebugInfo to delegate/handle that?) & is there nothing in CGDebugInfo
that is already called back in a timely fashion to handle this? I'm
guessing what's needed is a "IR has finished being generated for this
function, opt/codegen is about to start"? That seems like a good generic
callback & probably shouldn't be coming from something called CGVTables?
(presuming there's a more general place to put that)

On Tue, May 30, 2017 at 5:30 PM Keno Fischer via Phabricator <
reviews at reviews.llvm.org> wrote:

> loladiro created this revision.
>
> LLVM commit https://reviews.llvm.org/D33655 was reverted, because it
> exposed an assertion failure,
> in `GenerateVarArgsThunk`. That function attempts to clone a function
> before clang is entirely done generating the debug info for the current
> compliation unit. That is not a valid use of the API, because it expects
> that there are no temporary MD nodes in the nodes that it needs to clone.
> However, until now, this did not cause any problems (though could have
> caused assertions in the backend due to mismatched debug info metadata).
>
> Attempt to rectify this by finalizing the SP before attempting to clone it.
> The requisite LLVM API is being introduced in
> https://reviews.llvm.org/D33704.
>
>
> https://reviews.llvm.org/D33705
>
> Files:
>   lib/CodeGen/CGDebugInfo.h
>   lib/CodeGen/CGVTables.cpp
>
>
> Index: lib/CodeGen/CGVTables.cpp
> ===================================================================
> --- lib/CodeGen/CGVTables.cpp
> +++ lib/CodeGen/CGVTables.cpp
> @@ -152,6 +152,12 @@
>    llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
>    llvm::Function *BaseFn = cast<llvm::Function>(Callee);
>
> +  // We may not clone a subprogram that contains temporary metadata, so
> finalize
> +  // it now - we will not be modifying it.
> +  if (CGDebugInfo *DI = CGM.getModuleDebugInfo())
> +    if (llvm::DISubprogram *SP = BaseFn->getSubprogram())
> +      DI->finalizeSP(SP);
> +
>    // Clone to thunk.
>    llvm::ValueToValueMapTy VMap;
>    llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);
> Index: lib/CodeGen/CGDebugInfo.h
> ===================================================================
> --- lib/CodeGen/CGDebugInfo.h
> +++ lib/CodeGen/CGDebugInfo.h
> @@ -308,6 +308,7 @@
>    ~CGDebugInfo();
>
>    void finalize();
> +  void finalizeSP(llvm::DISubprogram *SP) { DBuilder.finalizeSP(SP); }
>
>    /// Module debugging: Support for building PCMs.
>    /// @{
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170531/1bfb153c/attachment.html>