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 leak in byterun/startup.c #7958

Closed
vicuna opened this issue Nov 25, 2002 · 2 comments
Closed

Memory leak in byterun/startup.c #7958

vicuna opened this issue Nov 25, 2002 · 2 comments

Comments

@vicuna
Copy link

vicuna commented Nov 25, 2002

Original bug ID: 1482
Reporter: administrator
Status: acknowledged
Resolution: open
Priority: normal
Severity: feature
Category: runtime system and C interface
Tags: patch

Bug description

Full_Name: Blair Zajac
Version: 3.06
OS: Linux
Submission from: bdsl.66.12.233.174.gte.net (66.12.233.174)

In the ocaml-3.06 build directory after building everything, running

% valgrind --leak-check=yes boot/ocamlrun ocaml < /dev/null
...
        Objective Caml version 3.06

#
==9689==
==9689== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==9689== malloc/free: in use at exit: 1530198 bytes in 25 blocks.
==9689== malloc/free: 68 allocs, 43 frees, 1691808 bytes allocated.
==9689== For counts of detected errors, rerun with: -v
==9689== searching for pointers to 25 not-freed blocks.
==9689== checked 5741940 bytes.
==9689==
==9689== definitely lost: 52 bytes in 1 blocks.
==9689== possibly lost:   0 bytes in 0 blocks.
==9689== still reachable: 1530146 bytes in 24 blocks.
==9689==
==9689== 52 bytes in 1 blocks are definitely lost in loss record 1 of 5
==9689==    at 0x4006034B: malloc (vg_clientfuncs.c:100)
==9689==    by 0x804F175: stat_alloc (memory.c:338)
==9689==    by 0x805E12C: search_in_path (unix.c:78)
==9689==    by 0x805E19F: search_exe_in_path (unix.c:138)
==9689==
==9689== LEAK SUMMARY:
==9689==    definitely lost: 52 bytes in 1 blocks.
==9689==    possibly lost:   0 bytes in 0 blocks.
==9689==    still reachable: 1530146 bytes in 24 blocks.
==9689== Reachable blocks (those to which a pointer was found) are not shown.
==9689== To see them, rerun with: --show-reachable=yes
==9689==

This fix is a one line addition to byterun/startup.c. I'm attaching a patch
that modifies a couple of things:

  1. Fix the memory leak in exe_name.
  2. Include limits.h to get PATH_MAX. Use it to give proc_self_exe enough space
    for an arbitrary path.
  3. Check the result of lseek in read_trailer.
  4. Add a comment in a case statement to make it clear that failling through
    without doing anything is the expected behavior.
diff -x CVS -ru ../ocaml-3.06-no-compile/byterun/startup.c ./byterun/startup.c
--- ../ocaml-3.06-no-compile/byterun/startup.c  2002-06-05 05:26:08.000000000
-0700
+++ ./byterun/startup.c 2002-11-25 10:34:17.000000000 -0800
@@ -19,6 +19,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
+#include <limits.h>
 #include "config.h"
 #ifdef HAS_UNISTD
 #include <unistd.h>
@@ -84,11 +85,12 @@

 static int read_trailer(int fd, struct exec_trailer *trail)
 {
-  lseek(fd, (long) -TRAILER_SIZE, SEEK_END);
+  if (lseek(fd, (long) -TRAILER_SIZE, SEEK_END) == (off_t)-1)
+    return BAD_BYTECODE;
   if (read(fd, (char *) trail, TRAILER_SIZE) < TRAILER_SIZE)
     return BAD_BYTECODE;
   fixup_endianness_trailer(&trail->num_sections);
-  if (strncmp(trail->magic, EXEC_MAGIC, 12) == 0)
+  if (strncmp(trail->magic, EXEC_MAGIC, sizeof(trail->magic)) == 0)
     return 0;
   else
     return BAD_BYTECODE;
@@ -321,7 +323,7 @@
   char * shared_lib_path, * shared_libs, * req_prims;
   char * exe_name;
 #ifdef __linux__
-  static char proc_self_exe[256];
+  static char proc_self_exe[PATH_MAX+1];
   int retcode;
 #endif

@@ -351,6 +353,7 @@
     pos = parse_command_line(argv);
     if (argv[pos] == 0)
       fatal_error("No bytecode file specified.\n");
+    stat_free(exe_name);
     exe_name = argv[pos];
     fd = attempt_open(&exe_name, &trail, 1);
     switch(fd) {
@@ -362,6 +365,10 @@
         "Fatal error: the file %s is not a bytecode executable file\n",
         argv[pos]);
       break;
+    default:
+      /* There were no errors in opening and checking the bytecode
+       * file for its magic number, so fall through and use it. */
+      break;
     }
   }
   /* Read the table of contents (section descriptors) */
@vicuna
Copy link
Author

vicuna commented Dec 7, 2016

Comment author: @mshinwell

Related to #71

@github-actions
Copy link

github-actions bot commented May 6, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 6, 2020
@stedolan stedolan self-assigned this May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants