unique_ptr lifetime bug in Builder.cpp

[Mandar12]

In Builder.cpp

  auto builder2 = HtmlElement::build("ul")
    ->add_child_2("li", "hello")->add_child_2("li", "world");
  cout << builder2 << endl;

builder2 doesn't store unique_ptr returned by HtmlElement::build, but stores HtmlBuilder* returned by add_child2. unique_ptr is destroyed before call to cout and builder2 contains pointer to freed memory (address sanitizer error - heap use after free).

Solution: Increase unique_ptr lifetime by storing in local variable.

  auto builder = HtmlElement::build("ul");
  auto builder2 = builder->add_child_2("li", "hello")->add_child_2("li", "world");
  cout << builder2->str() << endl;

[dzvancuks]

  auto builder2 = HtmlElement::build("ul")
    ->add_child_2("li", "hello")->add_child_2("li", "world");
  cout << builder2 << endl;

Please correct me if I'm wrong. To me it seems that we create builder in unique_ptr, add children and return HtmlElement by using "operator HtmlElement ()". After that builder is destroyed. On cout we are operating with HtmlElement. Thus everything should be safe.

[zzzz-qq]

I'm curious about this issue, too. To figure it out, I simplify the Builder.cpp as follow

#include <memory>

struct HtmlBuilder
{
    HtmlBuilder* add_child();
};

struct HtmlElement
{
    static std::unique_ptr<HtmlBuilder> build();
};

void demo()
{
    HtmlElement::build()
        ->add_child()
        ->add_child()
        ->add_child();
}

And check the assembly code (see Godbolt) of void demo()

demo():
        pushq   %rbp
        movq    %rsp, %rbp
        pushq   %rbx
        subq    $24, %rsp
        leaq    -24(%rbp), %rax
        movq    %rax, %rdi
        call    HtmlElement::build()
        leaq    -24(%rbp), %rax
        movq    %rax, %rdi
        call    std::unique_ptr<HtmlBuilder, std::default_delete<HtmlBuilder> >::operator->() const
        movq    %rax, %rdi
        call    HtmlBuilder::add_child()
        movq    %rax, %rdi
        call    HtmlBuilder::add_child()
        movq    %rax, %rdi
        call    HtmlBuilder::add_child()
        leaq    -24(%rbp), %rax
        movq    %rax, %rdi
        call    std::unique_ptr<HtmlBuilder, std::default_delete<HtmlBuilder> >::~unique_ptr() [complete object destructor]

According to assembly code, destructor of unqiue_ptr was not called until all three HtmlBuilder::add_child() returned, so there is no lifetime issue, this issue may be closed.