[llvm-dev] TargetRegistry and MC object ownership.

[Rafael Avila de Espindola via llvm-dev]

Lang Hames via llvm-dev <llvm-dev at lists.llvm.org> writes:

> Hi All,
>
> While trying to put together an MC-based assembler the other day, I again
> encountered MC's non-obvious memory ownership rules. The most egregious
> example of these is MCObjectStreamer's destructor:
>
> MCObjectStreamer::~MCObjectStreamer() {
>   delete &Assembler->getBackend();
>   delete &Assembler->getEmitter();
>   delete &Assembler->getWriter();
>   delete Assembler;
> }
>
> In the depths of a fever from a head-cold, I snapped. I've been hacking MC
> to convert these raw pointers (and worse: references!) to unique_ptrs
> (apologies to people whose backbends I've broken), but I hit a big blocker
> when I get to Target in "llvm/Support/TargetRegistry.h".
>
> Target vends MC objects by calling registered ctor functions. E.g.:
>
> MCAsmBackend *createMCAsmBackend(const MCRegisterInfo &MRI,
>                                  StringRef TheTriple, StringRef CPU,
>                                  const MCTargetOptions &Options) const {
>   if (!MCAsmBackendCtorFn)
>     return nullptr;
>   return MCAsmBackendCtorFn(*this, MRI, Triple(TheTriple), CPU, Options);
> }
>
> The callee owns the resulting object so ideally we would return a
> unique_ptr<MCAsmBackend>, but to do this we'd need access to the definition
> of MCAsmBackend which we can't get without violating layering. (We need the
> definition because unique_ptr<MCAsmBackend> can notionally call
> MCAsmBackend's destructor, though we know it won't here as the whole point
> is to hand ownership back to the caller).
>
> There are three approaches I can think of to improve things:
>
> (1) Keep the raw pointers, but document *very* clearly, recommending the
> caller stash the result in a unique_ptr immediately.
>
> (2) Have the ctor functions take a unique_ptr<T>* as their first argument
> and store the result there (thanks to Dave Blaikie for this suggestion).
> Passing the unique_ptrs by pointer means we don't need a definition for T.
> Usage looks like:
>
> std::unique_ptr<MCAsmBackend> MAB;
> Target.createMCAsmBackend(&MAB, TheTriple, CPU, Options);
>
>
> (3) Introduce a 'transient_unique_ptr<T>' type that never calls T's
> destructor, and whose purpose is to pass off ownership to a real unique_ptr:
>
> template <typename T>
> class transient_unique_ptr {
> public:
>   transient_unique_ptr(std::unique_ptr<T> InVal) : Val(InVal.release()) {}
>   transient_unique_ptr(transient_unique_ptr<T> &&Other) :
> Val(Other.Val) { Other.Val
> = nullptr; }
>   transient_unique_ptr&& operator=(transient_unique_ptr<T> &&Other) {
> std::swap(Val,
> Other.Val); return *this; }
>   ~transient_unique_ptr() { assert(!Val && "value should have been handed
> off"); }
>   std::unique_ptr<T> take() { auto Tmp = Val; Val = nullptr; return
> unique_ptr<T>(Val); }
> private:
>   T *Val = nullptr;
> };

(4) Move target registration out of Support so that it can include the
required definitions from MC.

Cheers,
Rafael