libxml2 xmlNodePtr's type changes when I obser

Question

I spent some time yesterday debugging a lovely Heisenbug where an xmlNodePtr's type would change on me. This example shows the error:

#include <iostream>

#include <vector>
#include <string>
#include <memory>    // std::unique_ptr

#include <cstdint>

#include <libxml/tree.h>
#include <libxml/parser.h>

struct SomeDataType {
    std::vector<std::vector<std::string>> data;

    explicit SomeDataType(uint32_t rows_, uint32_t columns_)
        : data(rows_)
    {
        for (uint32_t row = 0; row < rows_; ++row) {
            data[row].resize(columns_);
        }
    }
};

static std::vector<xmlNodePtr> GetChildren(xmlNodePtr node)
{
    std::vector<xmlNodePtr> children;

    xmlNodePtr child = node->children;
    while (child) {
        if (child->type == XML_ELEMENT_NODE) {
            children.push_back(child);
        }
        child = child->next;
    }

    return children;
}

int main() {
    std::unique_ptr<xmlDoc, void(*)(xmlDoc*)> document = { xmlParseEntity("libxml2-fail.xml"), xmlFreeDoc };

    SomeDataType{ 3, 2 };

    xmlNodePtr root = xmlDocGetRootElement(document.get());

    for (const xmlNodePtr &child : GetChildren(root)) {
        const xmlNodePtr &entry = GetChildren(child)[0]; // Problem here...
        std::cout << "Expected " << XML_ELEMENT_NODE << " but was " << entry->type << std::endl;
        std::cout << entry->name << std::endl;
    }
}

Compiled with:

g++ -g -std=c++14 -Wall -Wextra -pedantic -I/usr/include/libxml2 libxml2-fail.cpp -lxml2 -o fail.out

The xml file:

<?xml version="1.0" encoding="utf-8"?>
<data>
  <tag>
    <subtag>1</subtag>
  </tag>
</data>

Running gives me the following output:

Expected 1 but was 17

Stepping through with gdb, everything is fine until we reach the line const xmlNodePtr & = .... Instead of having type XML_ELEMENT_NODE, it has type XML_ENTITY_DECL. However, if I run the following commands, the reference xmlNodePtr morphs into the type I expect:

48          const xmlNodePtr &entry = GetChildren(child)[0];
(gdb) n
49          std::cout << "Expected " << XML_ELEMENT_NODE << " but was " << entry->type << std::endl;
(gdb) p *entry
$1 = {_private = 0x0, type = XML_ENTITY_DECL, name = 0x0, children = 0xb7e67d7c <std::string::_Rep::_S_empty_rep_storage+12>, last = 0x0, parent = 0x69, next = 0x0, prev = 0x9, doc = 0x0, ns = 0x805edb8, content = 0x805edb8 "", properties = 0x0, nsDef = 0x0, psvi = 0x0, line = 60648, extra = 2053}
(gdb) p *child
$2 = {_private = 0x0, type = XML_ELEMENT_NODE, name = 0x805ee98 "tag", children = 0x805eea8, last = 0x805ef98, parent = 0x805edb8, next = 0x805efe8, prev = 0x805ee08, doc = 0x805ece8, ns = 0x0, content = 0x0, properties = 0x0, nsDef = 0x0, psvi = 0x0, line = 3, extra = 0}
(gdb) p GetChildren(child)
$3 = std::vector of length 1, capacity 1 = {0x805eef8}
(gdb) p *entry
$4 = {_private = 0x0, type = XML_ELEMENT_NODE, name = 0x805ef38 "subtag", children = 0x805ef48, last = 0x805ef48, parent = 0x805ee58, next = 0x805ef98, prev = 0x805eea8, doc = 0x805ece8, ns = 0x0, content = 0x0, properties = 0x0, nsDef = 0x0, psvi = 0x0, line = 4, extra = 0}
(gdb) 

I don't have the problem when I instead loop over the one element like so:

for (const xmlNodePtr &entry : GetChildren(child)) {
    ...
}

I also don't have the problem when I don't make the xmlNodePtr a const reference like so:

xmlNodePtr entry = GetChildren(child)[0];

However, according to this stackoverflow question, it shouldn't be a problem.

The SomeDataType struct is strangely necessary; otherwise I get a segfault because entry becomes a null pointer.

What is this bug coming from?

Answer 1

When you do this:

const xmlNodePtr &entry = GetChildren(child)[0]; // Problem here...

You're effectively binding a reference to a temporary in a way that is not lifetime extended. operator[] returns a reference, so you're not binding a reference to a temporary - you're binding a reference to a reference. But that returned reference from operator[] refers to an element in the underlying temporary vector returned by GetChildren() which goes out of scope at the end of the line, leaving yourself a dangling reference.

---

However, when you instead tried:

for (const xmlNodePtr &entry : GetChildren(child)) {

that is syntactic sugar for:

{
    auto&& __range = GetChildren(child); // bind temporary to reference
                                         // lifetime IS extended
    auto b = begin(__range);
    auto e = end(__range);
    for (; b != e; ++b) {
        const xmlNodePtr& entry = *b;
        // ...
    }
}

here, *b isn't a temporary or any part of a temporary - it's a reference into a container whose lifetime lasts as long as __range does, which is through the entire body of the loop. No dangling reference.

---

Similarly,

xmlNodePtr entry = GetChildren(child)[0];

is just copying, no reference issues whatsoever.