Skip to content

Commit

Permalink
fix: sidestep gcc optimization bug with -Os
Browse files Browse the repository at this point in the history
  • Loading branch information
@amaanq
amaanq committed May 14, 2024
1 parent d5aea05 commit 27afeb0
Showing 1 changed file with 1 addition and 2 deletions.
3 changes: 1 addition & 2 deletions common/scanner.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,7 @@ static bool scan(Scanner *scanner, TSLexer *lexer, const bool *valid_symbols) {
array_delete(&word);

lexer->mark_end(lexer);
String last_word = array_pop(&scanner->heredocs).word;
array_delete(&last_word);
array_delete(&array_pop(&scanner->heredocs).word);
return true;
}

Expand Down

2 comments on commit 27afeb0

@KorigamiK
Copy link

Choose a reason for hiding this comment

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

can you explain what this fixes?

@amaanq
Copy link
Member Author

@amaanq amaanq commented on 27afeb0 May 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It fixes what I believe to be a bug in GCC. Even though you'd think this should not change anything, it does actually spit out different assembly for whatever reason:

Bad:
image

Good:
image

You can see in the bad one the value from array_pop isn't stored in a temporary to be later freed (array_delete), it's calling free on an earlier stack variable thus freeing an invalid pointer (garbage data)

Note: This only happens in gcc, and only with -Os, clang is totally fine and -O0-3 in gcc as well

Please sign in to comment.