Merge pull request #247 from boriel/bugfix/letsubstr_memleak

Fix error with LETSUBSTR

Author: boriel

api/check.py

@@ -411,6 +411,12 @@ def is_block_accessed(block):
     return False
 
 
+def is_temporary_value(node) -> bool:
+    """ Returns if the AST node value is a variable or a temporary copy in the heap.
+    """
+    return node.token not in ('STRING', 'VAR') and node.t[0] not in ('_', '#')
+
+
 def common_type(a, b):
     """ Returns a type which is common for both a and b types.
     Returns None if no common types allowed.

arch/zx48k/translator.py

@@ -314,12 +314,12 @@ def visit_LETARRAY(self, node):
 
     def visit_LETSUBSTR(self, node):
         yield node.children[3]
-        self.ic_param(TYPE.string, node.children[3].t)
 
-        if node.children[3].token != 'STRING' and (node.children[3].token != 'VAR' or
-                                                   node.children[3].mangled[0] != '_'):
-            self.ic_param(TYPE.ubyte, 1)  # If the argument is not a variable, it must be freed
+        if check.is_temporary_value(node.children[3]):
+            self.ic_param(TYPE.string, node.children[3].t)
+            self.ic_param(TYPE.ubyte, 1)
         else:
+            self.ic_param(gl.PTR_TYPE, node.children[3].t)
             self.ic_param(TYPE.ubyte, 0)
 
         yield node.children[1]
@@ -338,6 +338,7 @@ def visit_LETARRAYSUBSTR(self, node):
         yield expr
         self.ic_param(TYPE.string, expr.t)
 
+        # TODO: this produces a memory leak
         if expr.token != 'STRING' and (expr.token != 'VAR' or expr.mangled[0] != '_'):
             self.ic_param(TYPE.ubyte, 1)  # If the argument is not a variable, it must be freed
         else:

library-asm/letsubstr.asm

@@ -16,7 +16,6 @@ __LETSUBSTR:
 	LOCAL __CONT1
 	LOCAL __CONT2
 	LOCAL __FREE_STR
-	LOCAL __FREE_STR0
 
 	exx
 	pop hl ; Return address
@@ -32,10 +31,11 @@ __LETSUBSTR:
 	exx
 	push hl ; push ret addr back
 	exx
+	push de ; B$ addr to be freed upon return (if A != 0)
 
 	ld a, h
 	or l
-	jp z, __FREE_STR0 ; Return if null
+	jp z, __FREE_STR ; Return if null
 	
 	ld c, (hl)
 	inc hl
@@ -46,21 +46,21 @@ __LETSUBSTR:
 	exx
 	ex de, hl
 	or a
-	sbc hl, bc ; HL = Length of string requester by user
+	sbc hl, bc ; HL = Length of string requested by user
 	inc hl	   ; len (a$(p0 TO p1)) = p1 - p0 + 1
 	ex de, hl  ; Saves it in DE
 
 	pop hl	   ; HL = String length
 	exx
-	jp c, __FREE_STR0	   ; Return if greather
-	exx		   ; Return if p0 > p1
+	jp c, __FREE_STR	   ; Return if p0 > p1
+	exx
 
 	or a
 	sbc hl, bc ; P0 >= String length?
 	exx
 
-	jp z, __FREE_STR0	   ; Return if equal
-	jp c, __FREE_STR0	   ; Return if greather
+	jp z, __FREE_STR	   ; Return if equal
+	jp c, __FREE_STR	   ; Return if greater
 
 	exx
 	add hl, bc ; Add it back
@@ -72,7 +72,7 @@ __LETSUBSTR:
 	ld d, h
 	ld e, l	   ; Truncate length of substring to fit within the strlen
 	
-__CONT0:	   ; At this point DE = Length of subtring to copy
+__CONT0:	   ; At this point DE = Length of substring to copy
 			   ; BC = start of char to copy
 	push de
 
@@ -143,15 +143,10 @@ __CONT1:
 	pop de
 	ldir	; Copy b$ into a$(x to y)
 
-	exx
-	ex de, hl
-
-__FREE_STR0:
-	ex de, hl
-
 __FREE_STR:
+    pop hl
 	ex af, af'
-	or a		; If not 0, free 
+	or a		; If not 0, free
 	jp nz, __MEM_FREE
 	ret