Skip to content

Commit

Permalink
Fix TSAN warnings related to dfa::DFA::s0
Browse files Browse the repository at this point in the history
  • Loading branch information
jcking committed Oct 19, 2021
1 parent 23f93e0 commit def8cc7
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 26 deletions.
2 changes: 2 additions & 0 deletions contributors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -315,3 +315,5 @@ YYYY/MM/DD, github id, Full name, email
2021/08/08, ansiemens, Yi-Hong Lin, [email protected]
2021/09/08, jmcken8, Joel McKenzie, [email protected]
2021/10/10, tools4origins, Erwan Guyomarc'h, [email protected]
2021/10/19, jcking, Justin King, [email protected]

7 changes: 4 additions & 3 deletions runtime/Cpp/runtime/src/atn/LexerATNSimulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ size_t LexerATNSimulator::match(CharStream *input, size_t mode) {
_startIndex = input->index();
_prevAccept.reset();
const dfa::DFA &dfa = _decisionToDFA[mode];
if (dfa.s0 == nullptr) {
dfa::DFAState* s0 = dfa.getS0();
if (s0 == nullptr) {
return matchATN(input);
} else {
return execATN(input, dfa.s0);
return execATN(input, s0);
}
}

Expand Down Expand Up @@ -113,7 +114,7 @@ size_t LexerATNSimulator::matchATN(CharStream *input) {

dfa::DFAState *next = addDFAState(s0_closure.release());
if (!suppressEdge) {
_decisionToDFA[_mode].s0 = next;
_decisionToDFA[_mode].setS0(next);
}

size_t predict = execATN(input, next);
Expand Down
14 changes: 8 additions & 6 deletions runtime/Cpp/runtime/src/atn/ParserATNSimulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ size_t ParserATNSimulator::adaptivePredict(TokenStream *input, size_t decision,
s0 = dfa.getPrecedenceStartState(parser->getPrecedence());
} else {
// the start state for a "regular" DFA is just s0
s0 = dfa.s0;
s0 = dfa.getS0();
}

if (s0 == nullptr) {
Expand All @@ -109,15 +109,16 @@ size_t ParserATNSimulator::adaptivePredict(TokenStream *input, size_t decision,
&ParserRuleContext::EMPTY, fullCtx);

_stateLock.writeLock();
dfa::DFAState* ds0 = dfa.getS0();
if (dfa.isPrecedenceDfa()) {
/* If this is a precedence DFA, we use applyPrecedenceFilter
* to convert the computed start state to a precedence start
* state. We then use DFA.setPrecedenceStartState to set the
* appropriate start state for the precedence level rather
* than simply setting DFA.s0.
*/
dfa.s0->configs = std::move(s0_closure); // not used for prediction but useful to know start configs anyway
dfa::DFAState *newState = new dfa::DFAState(applyPrecedenceFilter(dfa.s0->configs.get())); /* mem-check: managed by the DFA or deleted below */
ds0->configs = std::move(s0_closure); // not used for prediction but useful to know start configs anyway
dfa::DFAState *newState = new dfa::DFAState(applyPrecedenceFilter(ds0->configs.get())); /* mem-check: managed by the DFA or deleted below */
s0 = addDFAState(dfa, newState);
dfa.setPrecedenceStartState(parser->getPrecedence(), s0, _edgeLock);
if (s0 != newState) {
Expand All @@ -127,9 +128,10 @@ size_t ParserATNSimulator::adaptivePredict(TokenStream *input, size_t decision,
dfa::DFAState *newState = new dfa::DFAState(std::move(s0_closure)); /* mem-check: managed by the DFA or deleted below */
s0 = addDFAState(dfa, newState);

if (dfa.s0 != s0) {
delete dfa.s0; // Delete existing s0 DFA state, if there's any.
dfa.s0 = s0;
if (ds0 != s0) {
delete ds0; // Delete existing s0 DFA state, if there's any.
ds0 = nullptr;
dfa.setS0(s0);
}
if (s0 != newState) {
delete newState; // If there was already a state with this config set we don't need the new one.
Expand Down
35 changes: 20 additions & 15 deletions runtime/Cpp/runtime/src/dfa/DFA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ DFA::DFA(atn::DecisionState *atnStartState, size_t decision)
if (is<atn::StarLoopEntryState *>(atnStartState)) {
if (static_cast<atn::StarLoopEntryState *>(atnStartState)->isPrecedenceDecision) {
_precedenceDfa = true;
s0 = new DFAState(std::unique_ptr<atn::ATNConfigSet>(new atn::ATNConfigSet()));
s0->isAcceptState = false;
s0->requiresFullContext = false;
DFAState* ds0 = new DFAState(std::unique_ptr<atn::ATNConfigSet>(new atn::ATNConfigSet()));
ds0->isAcceptState = false;
ds0->requiresFullContext = false;
setS0(ds0);
}
}
}
Expand All @@ -38,22 +39,25 @@ DFA::DFA(DFA &&other) : atnStartState(other.atnStartState), decision(other.decis

other.atnStartState = nullptr;
other.decision = 0;
s0 = other.s0;
other.s0 = nullptr;
setS0(other.getS0());
other.setS0(nullptr);
_precedenceDfa = other._precedenceDfa;
other._precedenceDfa = false;
}

DFA::~DFA() {
bool s0InList = (s0 == nullptr);
DFAState* ds0 = getS0();
bool s0InList = (ds0 == nullptr);
for (auto *state : states) {
if (state == s0)
if (state == ds0)
s0InList = true;
delete state;
}

if (!s0InList)
delete s0;
if (!s0InList) {
delete ds0;
setS0(nullptr);
}
}

bool DFA::isPrecedenceDfa() const {
Expand All @@ -63,8 +67,9 @@ bool DFA::isPrecedenceDfa() const {
DFAState* DFA::getPrecedenceStartState(int precedence) const {
assert(_precedenceDfa); // Only precedence DFAs may contain a precedence start state.

auto iterator = s0->edges.find(precedence);
if (iterator == s0->edges.end())
DFAState* ds0 = getS0();
auto iterator = ds0->edges.find(precedence);
if (iterator == ds0->edges.end())
return nullptr;

return iterator->second;
Expand All @@ -81,7 +86,7 @@ void DFA::setPrecedenceStartState(int precedence, DFAState *startState, SingleWr

{
lock.writeLock();
s0->edges[precedence] = startState;
getS0()->edges[precedence] = startState;
lock.writeUnlock();
}
}
Expand All @@ -99,7 +104,7 @@ std::vector<DFAState *> DFA::getStates() const {
}

std::string DFA::toString(const std::vector<std::string> &tokenNames) {
if (s0 == nullptr) {
if (getS0() == nullptr) {
return "";
}
DFASerializer serializer(this, tokenNames);
Expand All @@ -108,7 +113,7 @@ std::string DFA::toString(const std::vector<std::string> &tokenNames) {
}

std::string DFA::toString(const Vocabulary &vocabulary) const {
if (s0 == nullptr) {
if (getS0() == nullptr) {
return "";
}

Expand All @@ -117,7 +122,7 @@ std::string DFA::toString(const Vocabulary &vocabulary) const {
}

std::string DFA::toLexerString() {
if (s0 == nullptr) {
if (getS0() == nullptr) {
return "";
}
LexerDFASerializer serializer(this);
Expand Down
8 changes: 7 additions & 1 deletion runtime/Cpp/runtime/src/dfa/DFA.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#pragma once

#include <atomic>

#include "dfa/DFAState.h"

namespace antlrcpp {
Expand All @@ -22,7 +24,7 @@ namespace dfa {
/// From which ATN state did we create this DFA?
atn::DecisionState *atnStartState;
std::unordered_set<DFAState *, DFAState::Hasher, DFAState::Comparer> states; // States are owned by this class.
DFAState *s0;
std::atomic<DFAState*> s0;
size_t decision;

DFA(atn::DecisionState *atnStartState);
Expand Down Expand Up @@ -56,6 +58,10 @@ namespace dfa {
*/
DFAState* getPrecedenceStartState(int precedence) const;

DFAState* getS0() const { return s0.load(std::memory_order_acquire); }

void setS0(DFAState* s0) { this->s0.store(s0, std::memory_order_release); }

/**
* Set the start state for a specific precedence value.
*
Expand Down
2 changes: 1 addition & 1 deletion runtime/Cpp/runtime/src/dfa/DFASerializer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ DFASerializer::~DFASerializer() {
}

std::string DFASerializer::toString() const {
if (_dfa->s0 == nullptr) {
if (_dfa->getS0() == nullptr) {
return "";
}

Expand Down

0 comments on commit def8cc7

Please sign in to comment.