Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cpp Target:we found memory-leak in LexerATNSimulator::computeTargetState #3364

Closed
tangxing806 opened this issue Nov 25, 2021 · 4 comments
Closed

Comments

@tangxing806
Copy link

  • target:Cpp
  • antlr tags : 4.9.2
  • we found memory-leak in LexerATNSimulator::computeTargetState
dfa::DFAState *LexerATNSimulator::computeTargetState(CharStream *input, dfa::DFAState *s, size_t t) {
  OrderedATNConfigSet *reach = new OrderedATNConfigSet(); /* mem-check: deleted on error or managed by new DFA state. */

  // if we don't find an existing DFA state
  // Fill reach starting from closure, following t transitions
  getReachableConfigSet(input, s->configs.get(), reach, t);

  if (reach->isEmpty()) { // we got nowhere on t from s
    if (!reach->hasSemanticContext) {
      // we got nowhere on t, don't throw out this knowledge; it'd
      // cause a failover from DFA later.
      delete reach;
      addDFAEdge(s, t, ERROR.get());
    }

    // stop when we can't match any more char
    return ERROR.get();
  }

  // Add an edge from s to target DFA found/created for reach
  return addDFAEdge(s, t, reach);
}

after my modify:only add delete reach

dfa::DFAState *LexerATNSimulator::computeTargetState(CharStream *input, dfa::DFAState *s, size_t t) {
  OrderedATNConfigSet *reach = new OrderedATNConfigSet(); /* mem-check: deleted on error or managed by new DFA state. */

  // if we don't find an existing DFA state
  // Fill reach starting from closure, following t transitions
  getReachableConfigSet(input, s->configs.get(), reach, t);

  if (reach->isEmpty()) { // we got nowhere on t from s
    if (!reach->hasSemanticContext) {
      // we got nowhere on t, don't throw out this knowledge; it'd
      // cause a failover from DFA later.
      delete reach;
      addDFAEdge(s, t, ERROR.get());
    } else {
      delete reach;                                       // <<--------------------------- we modify here 
    }

    // stop when we can't match any more char
    return ERROR.get();
  }

  // Add an edge from s to target DFA found/created for reach
  return addDFAEdge(s, t, reach);
}

now, memory-leak disappear.
Due to the big grammar size and company policy, we can't provide the test grammar.

Sorry for that.

Is the above modifications reasonable and the best?

@jcking
Copy link
Collaborator

jcking commented Nov 29, 2021

The delete can be moved after the enclosing if statement instead of adding an additional else. I proposed a patch #3374.

@jcking
Copy link
Collaborator

jcking commented Dec 10, 2021

This should be resolved. @mike-lischke

@tangxing806
Copy link
Author

thanks!

@mike-lischke
Copy link
Member

Yes, so I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants