Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory error with macros with arguments #434

Closed
tueda opened this issue Mar 6, 2023 · 2 comments
Closed

Memory error with macros with arguments #434

tueda opened this issue Mar 6, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@tueda
Copy link
Collaborator

tueda commented Mar 6, 2023

This is found during the discussion of #433. The following code leads to a Valgrind error with the current master branch:

#do i=1,5
  #define x`i'
#enddo
#define var(a) "(b)"
#show
#message `var(1)'
.end
FORM 4.3.0 (Dec 23 2022, v4.3.0-10-g6cc038c) 64-bits  Run: Mon Mar  6 13:36:12 2023
    #do i=1,5
      #define x`i'
    #enddo
    #define var(a) "(b)"
    #show
#The preprocessor variables:
0: VERSION_ = "4"
1: SUBVERSION_ = "3"
2: DATE_ = "Mon Mar  6 13:36:12 2023"
3: random_ = "________"
4: optimminvar_ = "0"
5: optimmaxvar_ = "0"
6: OLDNUMEXTRASYMBOLS_ = "0"
7: optimvalue_ = "0"
8: optimscheme_ = "0"
9: tolower_ = "0"
10: toupper_ = "0"
11: SYSTEMERROR_ = "0"
12: PID_ = "30607"
13: PARALLELTASK_ = "0"
14: NPARALLELTASKS_ = "1"
15: NAME_ = "test.frm"
16: NTHREADS_ = "1"
17: CMODULE_ = "1"
18: x1 = "1"
19: x2 = "1"
20: x3 = "1"
21: x4 = "1"
22: x5 = "1"
23: var = "(b)"
==30607== Invalid read of size 4
==30607==    at 0x4BABE5: GetChar (pre.c:370)
==30607==    by 0x4BAD6F: LoadInstruction (pre.c:1204)
==30607==    by 0x4BB3EA: PreProInstruction (pre.c:1125)
==30607==    by 0x4BC0C4: PreProcessor (pre.c:936)
==30607==    by 0x4F723D: main (startup.c:1647)
==30607==  Address 0x59367f0 is 944 bytes inside a block of size 960 free'd
==30607==    at 0x4E07C2B: free (in /home/linuxbrew/.linuxbrew/Cellar/valgrind/3.19.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30607==    by 0x50DB06: M_free (tools.c:2449)
==30607==    by 0x50E7BD: FromList (tools.c:2791)
==30607==    by 0x4B59C2: PutPreVar (pre.c:668)
==30607==    by 0x4BAC0F: GetChar (pre.c:375)
==30607==    by 0x4BAD6F: LoadInstruction (pre.c:1204)
==30607==    by 0x4BB3EA: PreProInstruction (pre.c:1125)
==30607==    by 0x4BC0C4: PreProcessor (pre.c:936)
==30607==    by 0x4F723D: main (startup.c:1647)
==30607==  Block was alloc'd at
==30607==    at 0x4E05154: malloc (in /home/linuxbrew/.linuxbrew/Cellar/valgrind/3.19.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==30607==    by 0x50DA97: Malloc1 (tools.c:2322)
==30607==    by 0x50E770: FromList (tools.c:2786)
==30607==    by 0x4B59C2: PutPreVar (pre.c:668)
==30607==    by 0x4F5945: StartVariables (startup.c:1160)
==30607==    by 0x4F712D: main (startup.c:1590)
==30607==
    #message `var(1)'
~~~(b)
    .end
  0.06 sec out of 0.07 sec
==30607==
==30607== HEAP SUMMARY:
==30607==     in use at exit: 2,310,956,848 bytes in 125 blocks
==30607==   total heap usage: 167 allocs, 42 frees, 2,311,074,730 bytes allocated
==30607==
==30607== LEAK SUMMARY:
==30607==    definitely lost: 0 bytes in 0 blocks
==30607==    indirectly lost: 0 bytes in 0 blocks
==30607==      possibly lost: 0 bytes in 0 blocks
==30607==    still reachable: 2,310,956,848 bytes in 125 blocks
==30607==         suppressed: 0 bytes in 0 blocks
==30607== Rerun with --leak-check=full to see details of leaked memory
==30607==
==30607== For lists of detected and suppressed errors, rerun with: -s
==30607== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

The point is that the var macro has the index 23, which is $12 \times 2^n - 1$ with $n \in \mathbb{Z}_{\ge0}$. The magic number 12 comes from how the FromList function extends the list (the initial capacity is 12; the comment says 10 but that is wrong).

@tueda tueda added the bug Something isn't working label Mar 6, 2023
@tueda
Copy link
Collaborator Author

tueda commented Mar 6, 2023

The following patch makes this problem manifest and gives a segfault.

diff --git a/sources/tools.c b/sources/tools.c
index 3279d07..f392291 100644
--- a/sources/tools.c
+++ b/sources/tools.c
@@ -2788,7 +2788,15 @@ VOID *FromList(LIST *L)
                        i = ( L->num * L->size ) / sizeof(int);
                        old = (int *)L->lijst; newL = (int *)newlist;
                        while ( --i >= 0 ) *newL++ = *old++;
-                       if ( L->lijst ) M_free(L->lijst,"L->lijst FromList");
+                       if ( L->lijst ) {
+                               // Before freeing the memory block, we mess up its content.
+                               // This must not be a problem.
+                               char *p = (char *)L->lijst;
+                               for (int i = 0; i < L->num * L->size; i++) {
+                                       *p++ = 'x'; // non-zero
+                               }
+                               M_free(L->lijst,"L->lijst FromList");
+                       }
                }
                L->lijst = newlist;
        }

@tueda
Copy link
Collaborator Author

tueda commented Mar 7, 2023

When PutPreVar at line 372 or 375 extends the list of the preprocessor variables, the pointer p to a preprocessor variable in the current list will be invalid.

form/sources/pre.c

Lines 370 to 381 in 6cc038c

for ( j = 0; j < p->nargs; j++ ) {
if ( ( nargs == p->nargs-1 ) && ( *t == '?' ) ) {
PutPreVar(t,0,0,0);
}
else {
PutPreVar(t,s,0,0);
while ( *s ) s++;
s++;
}
while ( *t ) t++;
t++;
}

@tueda tueda closed this as completed in 741861a Mar 7, 2023
tueda added a commit that referenced this issue Apr 20, 2023
The pointer to the currently processed preprocessor variable is
invalidated when the list of preprocessor variables is extended.

Repatched 741861a (#441)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant