Skip to content

Commit f9b6b37

Browse files
Colin Hogbendpgeorge
Colin Hogben
authored and
dpgeorge
committed
py: Fix wrong assumption that m_renew will not move if shrinking
In both parse.c and qstr.c, an internal chunking allocator tidies up by calling m_renew to shrink an allocated chunk to the size used, and assumes that the chunk will not move. However, when MICROPY_ENABLE_GC is false, m_renew calls the system realloc, which does not guarantee this behaviour. Environments where realloc may return a different pointer include: (1) mbed-os with MBED_HEAP_STATS_ENABLED (which adds a wrapper around malloc & friends; this is where I was hit by the bug); (2) valgrind on linux (how I diagnosed it). The fix is to call m_renew_maybe with allow_move=false.
1 parent e5f0655 commit f9b6b37

2 files changed

Lines changed: 4 additions & 3 deletions

File tree

‎py/parse.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,9 +1010,10 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) {
10101010

10111011
// truncate final chunk and link into chain of chunks
10121012
if (parser.cur_chunk != NULL) {
1013-
(void)m_renew(byte, parser.cur_chunk,
1013+
(void)m_renew_maybe(byte, parser.cur_chunk,
10141014
sizeof(mp_parse_chunk_t) + parser.cur_chunk->alloc,
1015-
sizeof(mp_parse_chunk_t) + parser.cur_chunk->union_.used);
1015+
sizeof(mp_parse_chunk_t) + parser.cur_chunk->union_.used,
1016+
false);
10161017
parser.cur_chunk->alloc = parser.cur_chunk->union_.used;
10171018
parser.cur_chunk->union_.next = parser.tree.chunk;
10181019
parser.tree.chunk = parser.cur_chunk;

‎py/qstr.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ qstr qstr_from_strn(const char *str, size_t len) {
199199
byte *new_p = m_renew_maybe(byte, MP_STATE_VM(qstr_last_chunk), MP_STATE_VM(qstr_last_alloc), MP_STATE_VM(qstr_last_alloc) + n_bytes, false);
200200
if (new_p == NULL) {
201201
// could not grow existing memory; shrink it to fit previous
202-
(void)m_renew(byte, MP_STATE_VM(qstr_last_chunk), MP_STATE_VM(qstr_last_alloc), MP_STATE_VM(qstr_last_used));
202+
(void)m_renew_maybe(byte, MP_STATE_VM(qstr_last_chunk), MP_STATE_VM(qstr_last_alloc), MP_STATE_VM(qstr_last_used), false);
203203
MP_STATE_VM(qstr_last_chunk) = NULL;
204204
} else {
205205
// could grow existing memory

0 commit comments

Comments
 (0)