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 Nov 2, 2021
1 parent 0802afb commit fdf57d0
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 15 deletions.
2 changes: 1 addition & 1 deletion contributors.txt
Original file line number Diff line number Diff line change
Expand Up @@ -318,4 +318,4 @@ YYYY/MM/DD, github id, Full name, email
2021/09/23, skalt, Steven Kalt, [email protected]
2021/10/10, tools4origins, Erwan Guyomarc'h, [email protected]
2021/10/19, jcking, Justin King, [email protected]
2021/10/31, skef, Skef Iterum, [email protected]
2021/10/31, skef, Skef Iterum, [email protected]
22 changes: 16 additions & 6 deletions runtime/Cpp/runtime/src/atn/LexerATNSimulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,13 @@ size_t LexerATNSimulator::match(CharStream *input, size_t mode) {
_startIndex = input->index();
_prevAccept.reset();
const dfa::DFA &dfa = _decisionToDFA[mode];
if (dfa.s0 == nullptr) {
_stateLock.readLock();
dfa::DFAState* s0 = dfa.s0;
_stateLock.readUnlock();
if (s0 == nullptr) {
return matchATN(input);
} else {
return execATN(input, dfa.s0);
return execATN(input, s0);
}
}

Expand Down Expand Up @@ -111,10 +114,7 @@ size_t LexerATNSimulator::matchATN(CharStream *input) {
bool suppressEdge = s0_closure->hasSemanticContext;
s0_closure->hasSemanticContext = false;

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

size_t predict = execATN(input, next);

Expand Down Expand Up @@ -536,6 +536,10 @@ void LexerATNSimulator::addDFAEdge(dfa::DFAState *p, size_t t, dfa::DFAState *q)
}

dfa::DFAState *LexerATNSimulator::addDFAState(ATNConfigSet *configs) {
return addDFAState(configs, true);
}

dfa::DFAState *LexerATNSimulator::addDFAState(ATNConfigSet *configs, bool suppressEdge) {
/* the lexer evaluates predicates on-the-fly; by this point configs
* should not contain any configurations with unevaluated predicates.
*/
Expand Down Expand Up @@ -563,6 +567,9 @@ dfa::DFAState *LexerATNSimulator::addDFAState(ATNConfigSet *configs) {
auto iterator = dfa.states.find(proposed);
if (iterator != dfa.states.end()) {
delete proposed;
if (!suppressEdge) {
_decisionToDFA[_mode].s0 = *iterator;
}
_stateLock.writeUnlock();
return *iterator;
}
Expand All @@ -572,6 +579,9 @@ dfa::DFAState *LexerATNSimulator::addDFAState(ATNConfigSet *configs) {
proposed->configs->setReadonly(true);

dfa.states.insert(proposed);
if (!suppressEdge) {
_decisionToDFA[_mode].s0 = proposed;
}
_stateLock.writeUnlock();

return proposed;
Expand Down
2 changes: 2 additions & 0 deletions runtime/Cpp/runtime/src/atn/LexerATNSimulator.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ namespace atn {
/// </summary>
virtual dfa::DFAState *addDFAState(ATNConfigSet *configs);

virtual dfa::DFAState *addDFASTate(ATNConfigSet *configs, bool suppressEdge);

public:
dfa::DFA& getDFA(size_t mode);

Expand Down
14 changes: 10 additions & 4 deletions runtime/Cpp/runtime/src/atn/ParserATNSimulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,30 +94,35 @@ size_t ParserATNSimulator::adaptivePredict(TokenStream *input, size_t decision,
});

dfa::DFAState *s0;
_stateLock.readLock();
if (dfa.isPrecedenceDfa()) {
// the start state for a precedence DFA depends on the current
// parser precedence, and is provided by a DFA method.
_edgeLock.readLock();
s0 = dfa.getPrecedenceStartState(parser->getPrecedence());
_edgeLock.readUnlock();
} else {
// the start state for a "regular" DFA is just s0
s0 = dfa.s0;
}
_stateLock.readUnlock();

if (s0 == nullptr) {
bool fullCtx = false;
std::unique_ptr<ATNConfigSet> s0_closure = computeStartState(dynamic_cast<ATNState *>(dfa.atnStartState),
&ParserRuleContext::EMPTY, fullCtx);

_stateLock.writeLock();
dfa::DFAState* ds0 = dfa.s0;
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,8 +132,9 @@ 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.
if (ds0 != s0) {
delete ds0; // Delete existing s0 DFA state, if there's any.
ds0 = nullptr;
dfa.s0 = s0;
}
if (s0 != newState) {
Expand Down
8 changes: 4 additions & 4 deletions runtime/Cpp/runtime/src/dfa/DFA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,19 @@ 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()));
DFAState* s0 = new DFAState(std::unique_ptr<atn::ATNConfigSet>(new atn::ATNConfigSet()));
s0->isAcceptState = false;
s0->requiresFullContext = false;
}
}
}

DFA::DFA(DFA &&other) : atnStartState(other.atnStartState), decision(other.decision) {
DFA::DFA(DFA &&other) : atnStartState(other.atnStartState), s0(other.s0), decision(other.decision) {
// Source states are implicitly cleared by the move.
states = std::move(other.states);

other.atnStartState = nullptr;
other.decision = 0;
s0 = other.s0;
other.s0 = nullptr;
_precedenceDfa = other._precedenceDfa;
other._precedenceDfa = false;
Expand All @@ -52,8 +51,9 @@ DFA::~DFA() {
delete state;
}

if (!s0InList)
if (!s0InList) {
delete s0;
}
}

bool DFA::isPrecedenceDfa() const {
Expand Down

0 comments on commit fdf57d0

Please sign in to comment.