Skip to content

Alloc: Remove in tree heaplogging and avoid adding align to mallocs #1318

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.

Sign up for GitHub

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

Open
mkuoppal wants to merge 2 commits into
base: NetHack-3.7
Choose a base branch
from
Open

Alloc: Remove in tree heaplogging and avoid adding align to mallocs #1318

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4ee07d1
Remove MONITOR_HEAP debug functionality
mkuoppal Nov 8, 2024
b0c3497
alloc: restore trust to native malloc
mkuoppal Nov 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Remove MONITOR_HEAP debug functionality 
MONITOR_HEAP is a define which is used to track
memory allocations by file and line number. Altho this
has been useful to memory allocation problems, the
modern development environment have plethora of tools
just for this purpose: valgrind, mtrace,
electric fence, asan, msan etc.

Thus there should be no need to carry in tree malloc
debugging. Remove MONITOR_HEAP functionality.
  • Loading branch information
@mkuoppal
mkuoppal committed Jan 5, 2025
commit 4ee07d1b67091b3f1beb53651f8152c1b5b70f33
4 changes: 0 additions & 4 deletions include/extern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@

/* ### alloc.c ### */

#if 0
/* routines in alloc.c depend on MONITOR_HEAP and are declared in global.h */
extern long *alloc(unsigned int) NONNULL;
#endif
extern char *fmt_ptr(const void *) NONNULL;
/* moved from hacklib.c to alloc.c so that utility programs have access */
#define FITSint(x) FITSint_(x, __func__, __LINE__)
Expand Down
30 changes: 0 additions & 30 deletions include/global.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,44 +312,14 @@ typedef uchar nhsym;
* Null; if memory runs out, it calls panic() and does not return at all.
*/

/* dupstr() is unconditional in alloc.c but not used when MONITOR_HEAP
is enabled; some utility programs link with alloc.{o,obj} and need it
if nethack is built with MONITOR_HEAP enabled and they aren't; this
declaration has been moved out of the '#else' below to avoid getting
a complaint from -Wmissing-prototypes when building with MONITOR_HEAP */
extern char *dupstr(const char *) NONNULL NONNULLARG1;
/* same, but return strlen(string) in extra argument */
extern char *dupstr_n(const char *string,
unsigned *lenout) NONNULL NONNULLPTRS;

/*
* MONITOR_HEAP is conditionally used for primitive memory leak debugging.
* When enabled, NH_HEAPLOG (if defined in the environment) is used as the
* name of a log file to create for capturing allocations and releases.
* [The 'heaputil' program to analyze that file isn't included in releases.]
*
* See alloc.c.
*/
#ifdef MONITOR_HEAP
/* plain alloc() is not declared except in alloc.c */
extern long *nhalloc(unsigned int, const char *, int) NONNULL NONNULLARG2;
extern long *nhrealloc(long *, unsigned int, const char *,
int) NONNULL NONNULLARG3;
extern void nhfree(genericptr_t, const char *, int) NONNULLARG2;
extern char *nhdupstr(const char *, const char *, int) NONNULL NONNULLPTRS;
/* this predates C99's __func__; that is trickier to use conditionally
because it is not implemented as a preprocessor macro; MONITOR_HEAP
wouldn't gain much benefit from it anyway so continue to live without it;
if func's caller were accessible, that would be a very different issue */
#define alloc(a) nhalloc(a, __FILE__, (int) __LINE__)
#define re_alloc(a,n) nhrealloc(a, n, __FILE__, (int) __LINE__)
#define free(a) nhfree(a, __FILE__, (int) __LINE__)
#define dupstr(s) nhdupstr(s, __FILE__, (int) __LINE__)
#else /* !MONITOR_HEAP */
/* declare alloc.c's alloc(); allocations made with it use ordinary free() */
extern long *alloc(unsigned int) NONNULL; /* alloc.c */
extern long *re_alloc(long *, unsigned int) NONNULL;
#endif /* ?MONITOR_HEAP */

/* Used for consistency checks of various data files; declare it here so
that utility programs which include config.h but not hack.h can see it. */
Expand Down
127 changes: 3 additions & 124 deletions src/alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,6 @@ extern unsigned FITSuint_(unsigned long long, const char *, int) NONNULLARG2;

char *fmt_ptr(const genericptr) NONNULL;

#ifdef MONITOR_HEAP
#undef alloc
#undef re_alloc
#undef free
extern void free(genericptr_t);
staticfn void heapmon_init(void);

static FILE *heaplog = 0;
static boolean tried_heaplog = FALSE;
#endif

/*
* For historical reasons, nethack's alloc() returns 'long *' rather
* than 'void *' or 'char *'.
Expand All @@ -51,17 +40,8 @@ static boolean tried_heaplog = FALSE;
(LTH) += sizeof (long) - (LTH) % sizeof (long); \
} while (0)

#ifndef MONITOR_HEAP
long *alloc(unsigned int) NONNULL;
long *re_alloc(long *, unsigned int) NONNULL;
#else
/* for #if MONITOR_HEAP, alloc() might return Null but only nhalloc()
should be calling it; nhalloc() never returns Null */
long *alloc(unsigned int);
long *re_alloc(long *, unsigned int);
long *nhalloc(unsigned int, const char *, int) NONNULL;
long *nhrealloc(long *, unsigned int, const char *, int) NONNULL;
#endif
ATTRNORETURN extern void panic(const char *, ...) PRINTF_F(1, 2) NORETURN;

long *
Expand All @@ -71,12 +51,9 @@ alloc(unsigned int lth)

ForceAlignedLength(lth);
ptr = malloc(lth);
#ifndef MONITOR_HEAP
if (!ptr)
panic("Memory allocation failure; cannot get %u bytes", lth);
#else
/* for #if MONITOR_HEAP, failure is handled in nhalloc() */
#endif

return (long *) ptr;
}

Expand All @@ -88,13 +65,10 @@ re_alloc(long *oldptr, unsigned int newlth)

ForceAlignedLength(newlth);
newptr = (long *) realloc((genericptr_t) oldptr, (size_t) newlth);
#ifndef MONITOR_HEAP
/* "extend to": assume it won't ever fail if asked to shrink */
if (newlth && !newptr)
panic("Memory allocation failure; cannot extend to %u bytes", newlth);
#else
/* for #if MONITOR_HEAP, failure is handled in nhrealloc() */
#endif

return newptr;
}

Expand Down Expand Up @@ -134,102 +108,7 @@ fmt_ptr(const genericptr ptr)
return buf;
}

#ifdef MONITOR_HEAP

/* If ${NH_HEAPLOG} is defined and we can create a file by that name,
then we'll log the allocation and release information to that file. */
staticfn void
heapmon_init(void)
{
char *logname = getenv("NH_HEAPLOG");

if (logname && *logname)
heaplog = fopen(logname, "w");
tried_heaplog = TRUE;
}

long *
nhalloc(unsigned int lth, const char *file, int line)
{
long *ptr = alloc(lth);

if (!tried_heaplog)
heapmon_init();
if (heaplog)
(void) fprintf(heaplog, "+%5u %s %4d %s\n", lth,
fmt_ptr((genericptr_t) ptr), line, file);
/* potential panic in alloc() was deferred til here */
if (!ptr)
panic("Cannot get %u bytes, line %d of %s", lth, line, file);

return ptr;
}

/* re_alloc() with heap logging; we lack access to the old alloc size */
long *
nhrealloc(
long *oldptr,
unsigned int newlth,
const char *file,
int line)
{
long *newptr = re_alloc(oldptr, newlth);

if (!tried_heaplog)
heapmon_init();
if (heaplog) {
char op = '*'; /* assume realloc() will change size of previous
* allocation rather than make a new one */

if (newptr != oldptr) {
/* if oldptr wasn't Null, realloc() freed it */
if (oldptr)
(void) fprintf(heaplog, "%c%5s %s %4d %s\n", '<', "",
fmt_ptr((genericptr_t) oldptr), line, file);
op = '>'; /* new allocation rather than size-change of old one */
}
(void) fprintf(heaplog, "%c%5u %s %4d %s\n", op, newlth,
fmt_ptr((genericptr_t) newptr), line, file);
}
/* potential panic in re_alloc() was deferred til here;
"extend to": assume it won't ever fail if asked to shrink;
even if that assumption happens to be wrong, we lack access to
the old size so can't use alternate phrasing for that case */
if (newlth && !newptr)
panic("Cannot extend to %u bytes, line %d of %s", newlth, line, file);

return newptr;
}

void
nhfree(genericptr_t ptr, const char *file, int line)
{
if (!tried_heaplog)
heapmon_init();
if (heaplog)
(void) fprintf(heaplog, "- %s %4d %s\n",
fmt_ptr((genericptr_t) ptr), line, file);

free(ptr);
}

/* strdup() which uses our alloc() rather than libc's malloc(),
with caller tracking */
char *
nhdupstr(const char *string, const char *file, int line)
{
/* we've got some info about the caller, so use it instead of __func__ */
unsigned len = FITSuint_(strlen(string), file, line);

return strcpy((char *) nhalloc(len + 1, file, line), string);
}
#undef dupstr

#endif /* MONITOR_HEAP */

/* strdup() which uses our alloc() rather than libc's malloc();
not used when MONITOR_HEAP is enabled, but included unconditionally
in case utility programs get built using a different setting for that */
/* strdup() which uses our alloc() rather than libc's malloc(); */
char *
dupstr(const char *string)
{
Expand Down
3 changes: 0 additions & 3 deletions src/mdlib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,6 @@ static const char *const build_opts[] = {
#ifdef MAIL
"mail daemon",
#endif
#ifdef MONITOR_HEAP
"monitor heap - record memory usage for later analysis",
#endif
#if defined(GNUDOS) || defined(__DJGPP__)
"MSDOS protected mode",
#endif
Expand Down
5 changes: 0 additions & 5 deletions win/X11/winmap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,11 +361,6 @@ post_process_tiles(void)
tile_image, 0, 0, 0, 0, /* src, dest top left */
width, height);

#ifdef MONITOR_HEAP
/* if we let XDestroyImage() handle it, our tracking will be off */
if (tile_image->data)
free((genericptr_t) tile_image->data), tile_image->data = 0;
#endif
XDestroyImage(tile_image); /* data bytes free'd also */
tile_image = 0;

Expand Down
2 changes: 0 additions & 2 deletions win/share/gifread.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@
#include "config.h"
#include "tile.h"

#ifndef MONITOR_HEAP
extern long *alloc(unsigned int);
#endif

#define PPM_ASSIGN(p, red, grn, blu) \
do { \
Expand Down
2 changes: 0 additions & 2 deletions win/share/ppmwrite.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
#include "config.h"
#include "tile.h"

#ifndef MONITOR_HEAP
extern long *alloc(unsigned int);
#endif

FILE *ppm_file;

Expand Down
7 changes: 0 additions & 7 deletions win/share/tilemap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,6 @@
#define Snprintf(str, size, ...) \
nh_snprintf(__func__, __LINE__, str, size, __VA_ARGS__)

#ifdef MONITOR_HEAP
/* with heap monitoring enabled, free(ptr) is a macro which expands to
nhfree(ptr,__FILE__,__LINE__); since tilemap doesn't link with
src/alloc.o it doesn't have access to nhfree(); use actual free */
#undef free
#endif

#define Fprintf (void) fprintf

/*
Expand Down