gwenn / lemon-rs

LALR(1) parser generator for Rust based on Lemon + SQL parser
The Unlicense
48 stars 10 forks source link

panic on `CREATE TABLE L(x)L` #44

Closed MarinPostma closed 7 months ago

MarinPostma commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

MarinPostma commented 7 months ago

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 commented 7 months ago

I've reapplied your fix here: https://github.com/gwenn/lemon-rs/pull/46/commits/a7cd0c1dbb2da8730dda5e9f01917dc17776b1fd And I am going to fix all usages of sqlite3_error_msg. Thanks for all your feedback.