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

panic on CREATE TABLE L(x)L #44

Closed
MarinPostma opened this issue Mar 13, 2024 · 4 comments · Fixed by #46
Closed

panic on CREATE TABLE L(x)L #44

MarinPostma opened this issue Mar 13, 2024 · 4 comments · Fixed by #46

Comments

@MarinPostma
Copy link
Contributor

MarinPostma commented Mar 13, 2024

The following input causes a panic: CREATE TABLE L(x)L

Not sure what to do about this one, it hits an unreachable statement in parse.rs:

    fn yy222(self) -> Name {
        if let YYMINORTYPE::yy222(v) = self.minor {
            v
        } else {
            unreachable!()
        }
    }

I have many more inputs that trigger that as well, but they all are relatively similar to that one.

@MarinPostma
Copy link
Contributor Author

alright I found it, in parse.y, this bit:

table_option(A) ::= nm(X). {
  if "strict".eq_ignore_ascii_case(&X.0) {
    A = TableOptions::STRICT;
  }else{
    A = TableOptions::NONE;
    let msg = format!("unknown table option: {}", &X);
    self.ctx.sqlite3_error_msg(&msg);
  }
}

generates:

//line 161 "src/parser/parse.y"
{
  if "strict".eq_ignore_ascii_case(&self.yy_move(0).yy222().0) {
    yylhsminor = YYMINORTYPE::yy201( TableOptions::STRICT);
  }else{
    yylhsminor = YYMINORTYPE::yy201( TableOptions::NONE);
    let msg = format!("unknown table option: {}", &self.yy_move(0).yy222());
    self.ctx.sqlite3_error_msg(&msg);
  }
}

but the call to yy222 consumes self. on the subsequent call to create the error message, the content of the name was already taken from self.minor, and it blows up.

changing the rule to:

table_option(A) ::= nm(X). {
  let name = X;
  if "strict".eq_ignore_ascii_case(&name.0) {
    A = TableOptions::STRICT;
  }else{
    A = TableOptions::NONE;
    let msg = format!("unknown table option: {name}");
    self.ctx.sqlite3_error_msg(&msg);
  }
}

it doesn't panic anymore, but doesn't return an error either. Still investigating.

@gwenn
Copy link
Owner

gwenn commented Mar 14, 2024

Indeed, in one action, we are supposed to consume entry once.

@MarinPostma
Copy link
Contributor Author

I fixed it by immediately returning a ParseError after self.ctx.sqlite3_error_msg(&msg);, now it returns an error as expected. LMK if you are interested in these fixes!

@gwenn
Copy link
Owner

gwenn commented Mar 16, 2024

I've reapplied your fix here: a7cd0c1
And I am going to fix all usages of sqlite3_error_msg.
Thanks for all your feedback.

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

Successfully merging a pull request may close this issue.

2 participants