Skip to content

Correctly deallocate string vectors#221

Merged
dey4ss merged 3 commits into
masterfrom
dey4ss/fix_asan
Oct 17, 2022
Merged

Correctly deallocate string vectors#221
dey4ss merged 3 commits into
masterfrom
dey4ss/fix_asan

Conversation

@dey4ss
Copy link
Copy Markdown
Member

@dey4ss dey4ss commented Sep 23, 2022 •
edited
Loading

fixes #220

@dey4ss
Copy link
Copy Markdown
Member Author

dey4ss commented Oct 11, 2022

Update here: string vectors were not deallocated correctly. By applying the current status of this PR, I could not reproduce the error described in #220 anymore. With the current master, I reproduced the error.

@dey4ss dey4ss changed the title Correctly deallocate vectors of element pointers Correctly deallocate string vectors Oct 11, 2022
@dey4ss dey4ss requested review from Bouncner and mweisgut October 11, 2022 13:35
@Bouncner
Copy link
Copy Markdown
Collaborator

Bouncner commented Oct 14, 2022

So which files did you actually modify and which have been generated?

@dey4ss
Copy link
Copy Markdown
Member Author

dey4ss commented Oct 14, 2022 •
edited
Loading

So which files did you actually modify and which have been generated?

I only touched the few lines (destructor for <str_vec>, copied current destructor and replaced delete with free()) in bison_parser.y. The rest is generated. flex_lexer.[h | cpp] changes might be due to another flex version, and changes in bison_parser.cpp mostly consist of automatic updates in the comments (due to code insertion).

Bouncner
Bouncner approved these changes Oct 14, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Bouncner Bouncner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't judge this PR, but the (non automated) change make sense to me.

@dey4ss dey4ss merged commit c1e3de0 into master Oct 17, 2022
@dey4ss dey4ss deleted the dey4ss/fix_asan branch October 17, 2022 14:35
jwendell added a commit to jwendell/sql-parser that referenced this pull request Jan 27, 2026
@jwendell @claude
Merge upstream hyrise/sql-parser changes
34b1359 
This merge brings 107 commits from upstream hyrise/sql-parser while
preserving the MySQL-specific extensions from the Envoy fork:

Upstream features included:
- Window functions support (hyrise#233)
- Row locking grammar (hyrise#205)
- FOREIGN KEY constraints (hyrise#252)
- NULLS FIRST/LAST in ORDER BY (hyrise#257)
- BIGINT, SMALLINT, TIMESTAMP, BOOLEAN data types
- Date/interval literals
- Named columns for joins fix (hyrise#240)
- String vector deallocation fix (hyrise#221)
- GCC-13 compatibility (hyrise#245)
- ARM Mac support (hyrise#216)
- CSV import options for DELIMITER, NULL, QUOTE (hyrise#256)
- Various bug fixes and improvements

Fork features preserved:
- tablesAccessed() method for tracking table access operations
- MySQL-specific syntax: backticks, LOW_PRIORITY, IGNORE, QUICK
- MySQL statements: CREATE/DROP/ALTER DATABASE, SHOW DATABASES
- TEMPORARY tables, ALTER TABLE ADD COLUMN
- include/sqlparser/ directory structure

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Jonh Wendell <jwendell@redhat.com>
This was referenced Apr 27, 2026
Open
Open
Licht-T added a commit to Licht-T/sql-parser that referenced this pull request Apr 27, 2026
@Licht-T
Fix alloc-dealloc-mismatch in <str_vec> destructor; drop unused DELTA…
80cc4ae 
… token

The shared %destructor for <str_vec> <table_vec> ... called `delete` on
each element pointer, but <str_vec> elements are char* allocated by the
lexer via strdup() (unquoted IDENTIFIER) or hsql::substr() (quoted
IDENTIFIER) — both malloc-backed. Mixing free/delete is undefined
behavior. Under tcmalloc with -fsized-deallocation, sized operator
delete trusts the static type size (1 for char) and returns the chunk
to the wrong size-class freelist, eventually crashing on an unrelated
allocation (envoyproxy/envoy#36471).

This is the same bug fixed upstream in hyrise#221.

Split the destructor: <str_vec> uses free(); the rest hold pointers to
new-allocated objects and stay on delete.

Also drop the dead DELTA token. It is declared in sql_keywords.txt /
bison_parser.y / flex_lexer.l but referenced by no grammar rule, so any
SQL using `delta` as an identifier (e.g. pgbench's pgbench_history.delta)
fails to parse — which is what trips the destructor cleanup path that
triggers the UB above.

Regenerate bison_parser.{cpp,h} and flex_lexer.{cpp,h} accordingly.

Add regression tests:
  - DeltaIsAValidIdentifier: confirms `delta` parses as IDENTIFIER.
  - RepeatedFailedInsertParseDoesNotCorruptHeap: stresses the
    failed-parse cleanup; under ASAN catches the alloc-dealloc-mismatch
    immediately on regression.

Signed-off-by: Rito Takeuchi <licht-t@outlook.jp>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Bouncner Bouncner Bouncner approved these changes

@mweisgut mweisgut Awaiting requested review from mweisgut

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Alloc-dealloc-mismatch in yydestruct at bison_parser.y:172

2 participants

@dey4ss @Bouncner