Search code examples
c++heap-memoryshared-ptraddress-sanitizer

Heap use after free when using shared pointers


I keep getting heap use after a free error when I try to pass a shared pointer to a class object.

Firstly, my tree builder has the root as a private field:

class ExpressionTree{
private:
std::shared_ptr<ASTNode> root;
public:
std::shared_ptr<ASTNode> getRoot();
void build(std::string expression);
}

ASTNode for reference:

class ASTNode {
public:
    virtual ~ASTNode() = default;
    virtual void accept(ASTVisitor& visitor, ElementMap elements) = 0;
};

The build function (using the shunting yard algorithm) stores nodes in a dequeue:

 std::deque<std::shared_ptr<ASTNode>> nodeStack;
std::deque<std::string> operatorStack;

For example, for a binary operator (a class deriving from ASTNode):

auto right = nodeStack.back();
nodeStack.pop_back();
auto left= nodeStack.back();
nodeStack.pop_back();

root = std::make_shared<BinaryOperator>(operatorStack.back(), left, right);
operatorStack.pop_back();
nodeStack.emplace_back(std::move(root));

After the shunting yard algorithm goes through all the tokens, I move the root:

root = std::move(nodeStack.back());

I have a vector of game "rules" (stored as strings) and I am trying to store them into "Rule" classes that I have. The issue is in the "When" rule. My interpreter is where I use the tree builder and construct the "When" object:

ExpressionTree expressionTree;
    for (auto rule: rules_from_json->getVector()){
        if(ruleName == "when"){
        std::vector<std::pair<std::shared_ptr<ASTNode>, RuleVector>> conditionExpressionRulePairs;
        
        ElementVector cases = rule->getMapElement("cases")->getVector();
        for(auto caseRulePair : cases){
        
        std::string conditionString = caseRulePair->getMapElement("condition")->getString();
        expressionTree.build(conditionString);
        std::shared_ptr<ASTNode> conditionExpressionRoot = expressionTree.getRoot();
        
        RuleVector caseRules;
        toRuleVec(game, caseRulePair->getMapElement("rules"), caseRules);
        conditionExpressionRulePairs.push_back({conditionExpressionRoot, caseRules});
        }
        
        ruleObject = std::make_shared<When>(conditionExpressionRulePairs);
        }
    } //freed here

The error shows that std::shared_ptr is freed at the last brace of the above snippet.

This is the when rule:

class When : public Rule {
    std::shared_ptr<ASTNode> conditionRoot;

    std::map<std::shared_ptr<ASTNode>, RuleVector> condition_rule_pairs;
    std::map<std::shared_ptr<ASTNode>, RuleVector>::iterator condition_rule_pair;
    
    RuleVector::iterator rule;
    bool match = false;

public: 
    When(std::map<std::shared_ptr<ASTNode>, RuleVector>& conditon_rule_pairs);
    bool executeImpl(ElementSptr element, ElementMap elementsMap) final;
    void resetImpl() final;
};

All the other rules are working and are able to pass a visitor to the root correctly. The only difference here is that I'm storing the roots for each case in a vector and passing that in:

//rules.cpp in the `execute` function of the `when` rule
for (; conditionExpression_rule_pair != conditionExpression_rule_pairs.end(); conditionExpression_rule_pair++) {
conditionRoot = conditionExpression_rule_pair->first; //**this line gives the error**
conditionRoot->accept(resolver, elementsMap);

The error:

AddressSanitizer: heap-use-after-free on address 0x00010a902690 at pc 0x000104f83ec4 bp 0x00016b488550 sp 0x00016b488548
#0 0x104f83ec0 in std::__1::shared_ptr<ASTNode>::shared_ptr(std::__1::shared_ptr<ASTNode> const&) memory:3125
#1 0x104f687a4 in std::__1::shared_ptr<ASTNode>::shared_ptr(std::__1::shared_ptr<ASTNode> const&) memory:3127
#2 0x104f716b8 in std::__1::shared_ptr<ASTNode>::operator=(std::__1::shared_ptr<ASTNode> const&) memory:3246
#3 0x104f6fe34 in When::executeImpl(std::__1::shared_ptr<ListElement>, std::__1::map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::shared_ptr<ListElement>, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, std::__1::shared_ptr<ListElement> > > >) rules.cpp:105

The thing is, I tried to pass a visitor to the root in the constructor and it works:

When::When(std::vector<std::pair<std::shared_ptr<ASTNode>&,RuleVector>>& conditionExpression_rule_pairs)

: conditionExpression_rule_pairs(conditionExpression_rule_pairs), 

conditionExpression_rule_pair(conditionExpression_rule_pairs.begin()),

rule(conditionExpression_rule_pair->second.begin()) {

TreePrinter t; //a visitor just to pretty print nodes
ElementMap elementsMap;
for(auto it : conditionExpression_rule_pairs){
it.first->accept(t, elementsMap);
std::cout << std::endl;

}
}

So somehow, the root exists in the when constructor but not in the when execute because it has been freed from the heap. I suspect it has something to do with the fact that the roots are all stored in a vector because for the other rules, I just pass in a singular root and the constructor assigns it to another root like for example, the foreach rule :

Foreach::Foreach(std::shared_ptr<ASTNode> listExpressionRoot, RuleVector _rules, std::string elementName)
: listExpressionRoot(listExpressionRoot), rules(_rules), elementName(elementName) {
}

In the foreach, I can access the root in the execute function and pass a visitor, and so on.

I tried everything but cannot seem to get rid of this error in the when rule. Please help.


Solution

  • I cannot believe this. I spent a week on this and made posts to two different forums.

    TURNS OUT:

    rule(conditionExpression_rule_pair->second.begin())
    

    in the initializer list of the When constructor, was using conditionExpression_rule_pair from the constructor argument:

    When::When(std::vector<std::pair<std::shared_ptr<ASTNode>&,RuleVector>>& conditionExpression_rule_pairs)
    

    instead of the actual field from the when rule. So obviously that vector got deleted when the constructor went out of scope and the iterator would be pointing to nothing.