[Nasm-bugs] [Bug 3392892] New: [Patch] Major memory leak when using many %rep loops with nontrivial content

[noreply-nasm at dev.nasm.us]

https://bugzilla.nasm.us/show_bug.cgi?id=3392892

            Bug ID: 3392892
           Summary: [Patch] Major memory leak when using many %rep loops
                    with nontrivial content
           Product: NASM
           Version: 2.17 (development)
          Hardware: All
                OS: All
            Status: OPEN
          Severity: blocker
          Priority: Medium
         Component: Assembler
          Assignee: nobody at nasm.us
          Reporter: pushbx at ulukai.org
                CC: chang.seok.bae at intel.com, gorcunov at gmail.com,
                    hpa at zytor.com, nasm-bugs at nasm.us
     Obtained from: Built from git using configure

During development of a new check feature for my 86-DOS debugger [1] I
encountered reliably reproducable OOM killings of NASM. The feature in question
was to use an mmacro for the call instruction that would expand to dozens of
lines, including three %rep loops, to check every call instruction for
potential cross-section calls that were not marked properly. I don't know how
many call instructions there are in the debugger sources but it must be
hundreds of them.

I found that the assembler appears to have some leaks, which I suspected were
causing my problems. First I changed the #define TOKEN_BLOCKSIZE 4096 [3] to 0
in order to make the preprocessor call nasm_free when deleting tokens. Next I
used valgrind --leak-check=full nasm to look around and find leaks. It pointed,
among others, to the nasm_new call in the preprocessor handling of starting a
%rep loop [4].

Eventually, I ended up checking how the global variable named "defining" is
used, and how it is assigned a pointer to an allocated structure and how that
structure is freed. After some false starts I found the culprit: Once they are
no longer used, the structures pointed to by "defining" are not freed properly.
In particular, %rep structures are affected and seem to be leaked entirely.

In my tests, the %rep loop bodies have to contain some nontrivial amount of
text to cause the OOM killing, though I suspect the exact contents don't
actually matter. The space used up by this content probably does matter.

This is the fix for the big memory leak and another, smaller leak. The big leak
already had a FIXME note [5] linking to another bug reported earlier [6].
Unlike expected the %rep leak is significant and causes the OOM killings I
observed. In my fix I took care to only free the structure if it has no name,
that is it belongs to a %rep rather than an mmacro being expanded. Diff:

$ git diff
diff --git a/asm/preproc.c b/asm/preproc.c
index ac42131e..f131d655 100644
--- a/asm/preproc.c
+++ b/asm/preproc.c
@@ -7640,10 +7640,12 @@ static Token *pp_tokline(void)
 #endif
                     {
                         nasm_free(m->params);
+                        nasm_free(m->iname);
                         free_tlist(m->iline);
                         nasm_free(m->paramlen);
                         fm->in_progress = 0;
                        m->params = NULL;
+                       m->iname = NULL;
                        m->iline = NULL;
                        m->paramlen = NULL;
                     }
@@ -7673,8 +7675,8 @@ static Token *pp_tokline(void)
                  *
                  * https://bugzilla.nasm.us/show_bug.cgi?id=3392414
                  */
-#if 0
-                else
+#if 1
+                if (!m->name)
                     free_mmacro(m);
 #endif
             }


Contents of the session log [2]:

test/20230720$ cat testchk2.asm
        %imacro call 1.nolist
%push
 %assign %$size 2
 %assign %$isreg 0
 %assign %$exit 0
 %rep 2
  %ifn %$exit
   %if %$size == 2
    %define %$regnames "ax  cx  dx  bx  sp  bp  si  di  "
   %elif %$size == 4
    %define %$regnames "eax ecx edx ebx esp ebp esi edi "
   %endif
   %assign %$index 0
   %rep 8
    %ifn %$exit
     %substr %$reg %$regnames %$index * 4 + 1, 4
     %deftok %$reg %$reg
     %ifnempty %$reg
      %ifidni %$reg, %1
       %assign %$isreg %$size
       %assign %$exit 1
       %exitrep
      %endif
     %endif
    %endif
    %assign %$index %$index + 1
   %endrep
   %if %$exit
    %exitrep
   %endif
   %assign %$size %$size * 2
  %endif
 %endrep

 %assign %$ismulti 0
 %assign %$ismem 0
 %defstr %$string %1
 %strlen %$length %$string
 %assign %$ii 0
 %rep %$length
  %substr %$point %$string %$ii + 1, 1
  %if %$point == 32 || %$point == 9
   %assign %$ismulti 1
  %endif
  %ifidn %$point, '['
   %assign %$ismem 1
  %endif
  %assign %$ii %$ii + 1
 %endrep
%pop
        %endmacro


%ifndef _OUTER
 %assign _OUTER 100
%endif
%ifndef _INNER
 %assign _INNER 50
%endif

%rep _OUTER
call cx
call near [0]
call .
 %rep _INNER
call label
call labelfoo
call labelbar
call labelbaz
 %endrep
%endrep
test/20230720$ orignasm -v
NASM version 2.17rc0 compiled on Jul 20 2023
test/20230720$ orignasm testchk2.asm -D_INNER=1
test/20230720$ orignasm testchk2.asm
Killed
test/20230720$ nasm -v
NASM version 2.17rc0 compiled on Jul 20 2023
test/20230720$ nasm testchk2.asm -D_INNER=1
test/20230720$ nasm testchk2.asm
test/20230720$

[4435708.583745] OOM killed process 1845493 (orignasm) total-vm:2435924kB,
anon-rss:2432680kB, file-rss:80kB


[1]: https://hg.pushbx.org/ecm/ldebug/rev/7d94a90c607b
[2]: https://pushbx.org/ecm/test/20230720/testchk2.txt
[3]:
https://github.com/netwide-assembler/nasm/blob/a916e4127b2eaa3bf40bddf3de9b0ceefc0d98a4/asm/preproc.c#L1763
[4]:
https://github.com/netwide-assembler/nasm/blob/a916e4127b2eaa3bf40bddf3de9b0ceefc0d98a4/asm/preproc.c#L4743
[5]:
https://github.com/netwide-assembler/nasm/blob/a916e4127b2eaa3bf40bddf3de9b0ceefc0d98a4/asm/preproc.c#L7670
[6]: https://bugzilla.nasm.us/show_bug.cgi?id=3392414

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are watching all bug changes.