Commit f9b6b37c authored by Colin Hogben's avatar Colin Hogben Committed by Damien George

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.
parent e5f06559
...@@ -1010,9 +1010,10 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { ...@@ -1010,9 +1010,10 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) {
// truncate final chunk and link into chain of chunks // truncate final chunk and link into chain of chunks
if (parser.cur_chunk != NULL) { if (parser.cur_chunk != NULL) {
(void)m_renew(byte, parser.cur_chunk, (void)m_renew_maybe(byte, parser.cur_chunk,
sizeof(mp_parse_chunk_t) + parser.cur_chunk->alloc, sizeof(mp_parse_chunk_t) + parser.cur_chunk->alloc,
sizeof(mp_parse_chunk_t) + parser.cur_chunk->union_.used); sizeof(mp_parse_chunk_t) + parser.cur_chunk->union_.used,
false);
parser.cur_chunk->alloc = parser.cur_chunk->union_.used; parser.cur_chunk->alloc = parser.cur_chunk->union_.used;
parser.cur_chunk->union_.next = parser.tree.chunk; parser.cur_chunk->union_.next = parser.tree.chunk;
parser.tree.chunk = parser.cur_chunk; parser.tree.chunk = parser.cur_chunk;
......
...@@ -199,7 +199,7 @@ qstr qstr_from_strn(const char *str, size_t len) { ...@@ -199,7 +199,7 @@ qstr qstr_from_strn(const char *str, size_t len) {
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); 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);
if (new_p == NULL) { if (new_p == NULL) {
// could not grow existing memory; shrink it to fit previous // could not grow existing memory; shrink it to fit previous
(void)m_renew(byte, MP_STATE_VM(qstr_last_chunk), MP_STATE_VM(qstr_last_alloc), MP_STATE_VM(qstr_last_used)); (void)m_renew_maybe(byte, MP_STATE_VM(qstr_last_chunk), MP_STATE_VM(qstr_last_alloc), MP_STATE_VM(qstr_last_used), false);
MP_STATE_VM(qstr_last_chunk) = NULL; MP_STATE_VM(qstr_last_chunk) = NULL;
} else { } else {
// could grow existing memory // could grow existing memory
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment