From f9b6b37cf65c4f65c4ad461d439fbf624c0f10c1 Mon Sep 17 00:00:00 2001
From: Colin Hogben <colin@infinnovation.co.uk>
Date: Mon, 31 Oct 2016 14:05:56 +0000
Subject: [PATCH] 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.
---
py/parse.c | 5 +++--
py/qstr.c | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/py/parse.c b/py/parse.c
index 5920828fe..397d46d9f 100644
--- a/py/parse.c
+++ b/py/parse.c
@@ -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
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->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->union_.next = parser.tree.chunk;
parser.tree.chunk = parser.cur_chunk;
diff --git a/py/qstr.c b/py/qstr.c
index 28df06ca3..5aa161064 100644
--- a/py/qstr.c
+++ b/py/qstr.c
@@ -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);
if (new_p == NULL) {
// 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;
} else {
// could grow existing memory
--
GitLab