[DTrace-devel] [PATCH 01/15 v2] Fix various memory leaks
Kris Van Hees
kris.van.hees at oracle.com
Fri May 29 10:59:17 PDT 2020
Testing dtrace with valgrind identified a bunch of significant memory
leaks. These have been fixed.
There is still some loss of memory due to string elements in probe
descriptions not consistently using strdup() to ensure that they can
be freed when no longer needed, and also in the buffer handling in
the lexer. These are left for future investigation and resolution.
Orabug: 31386368
Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
---
cmd/dtrace.c | 7 +++-
libdtrace/dt_dlibs.c | 95 ++++++++++++++++++++++++++++++++++----------
libdtrace/dt_htab.c | 16 ++++----
libdtrace/dt_htab.h | 6 ++-
libdtrace/dt_ident.c | 3 +-
libdtrace/dt_open.c | 4 ++
libdtrace/dt_probe.c | 21 +++++++---
libdtrace/dt_subr.c | 1 +
8 files changed, 115 insertions(+), 38 deletions(-)
diff --git a/cmd/dtrace.c b/cmd/dtrace.c
index 3dc49800..cef86ef2 100644
--- a/cmd/dtrace.c
+++ b/cmd/dtrace.c
@@ -760,7 +760,7 @@ chew(const dtrace_probedata_t *data, void *arg)
if (!g_flowindent) {
if (!g_quiet) {
size_t len = strlen(pd->fun) + strlen(pd->prb) + 2;
- char *name = malloc(len);
+ char *name = alloca(len);
if (name == NULL)
fatal("failed to allocate memory for name");
@@ -1562,5 +1562,10 @@ main(int argc, char *argv[])
dtrace_proc_release(g_dtp, g_psv[i]);
dtrace_close(g_dtp);
+
+ free(g_argv);
+ free(g_cmdv);
+ free(g_psv);
+
return (g_status);
}
diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
index 2b8fe21d..445654d7 100644
--- a/libdtrace/dt_dlibs.c
+++ b/libdtrace/dt_dlibs.c
@@ -29,7 +29,7 @@ struct dt_bpf_reloc {
typedef struct dt_bpf_func dt_bpf_func_t;
struct dt_bpf_func {
- const char *name;
+ char *name;
int id;
uint64_t offset;
size_t size;
@@ -46,7 +46,8 @@ static dtrace_attribute_t dt_bpf_attr = DT_ATTR_STABCMN;
#define DT_BPF_SYMBOL_ID(name, type, id) \
{ __stringify(name), (type), DT_IDFLG_BPF, (id), \
DT_ATTR_STABCMN, DT_VERS_2_0, }
-static const dt_ident_t dt_bpf_symbols[] = {
+
+static const dt_ident_t dt_bpf_symbols[] = {
/* BPF built-in functions */
DT_BPF_SYMBOL(dt_predicate, DT_IDENT_FUNC),
DT_BPF_SYMBOL(dt_program, DT_IDENT_FUNC),
@@ -75,6 +76,43 @@ static const dt_ident_t dt_bpf_symbols[] = {
#undef DT_BPF_SYMBOL
#undef DT_BPF_SYMBOL_ID
+static void
+dt_idcook_none(dt_node_t *dnp, dt_ident_t *idp, int argc, dt_node_t *args)
+{
+}
+
+static void
+dt_iddtor_bpf(dt_ident_t *idp)
+{
+ dt_bpf_func_t *fp = idp->di_data;
+ dtrace_hdl_t *dtp = idp->di_iarg;
+
+ if (fp == NULL)
+ return;
+
+ if (fp->name != NULL)
+ free(fp->name);
+ if (fp->difo != NULL)
+ dt_difo_free(dtp, fp->difo);
+
+ dt_free(dtp, fp);
+}
+
+static size_t
+dt_idsize_none(dt_ident_t *idp)
+{
+ return 0;
+}
+
+/*
+ * Identifier operations for entries in the identifier hash.
+ */
+static const dt_idops_t dt_idops_bpf = {
+ dt_idcook_none,
+ dt_iddtor_bpf,
+ dt_idsize_none
+};
+
/*
* Perform initialization for the BPF function library handling.
*/
@@ -113,7 +151,8 @@ dt_dlib_add_sym(dtrace_hdl_t *dtp, const char *name, int kind, void *data)
dt_ident_t *idp;
idp = dt_idhash_insert(dhp, name, kind, DT_IDFLG_BPF, DT_IDENT_UNDEF,
- dt_bpf_attr, DT_VERS_2_0, NULL, NULL, 0);
+ dt_bpf_attr, DT_VERS_2_0, &dt_idops_bpf, dtp,
+ 0);
dt_ident_set_data(idp, data);
return idp;
@@ -260,9 +299,9 @@ relcmp(const void *a, const void *b)
* gcc BPF compiler not emitting symbol sizes), and then associate any listed
* relocations with each function.
*/
-static dt_bpf_func_t **
+static int
get_symbols(dtrace_hdl_t *dtp, Elf *elf, int syms_idx, int strs_idx,
- int text_idx, int text_len, int relo_idx, int maps_idx, int *cnt)
+ int text_idx, int text_len, int relo_idx, int maps_idx)
{
int idx, fid, fun0;
Elf_Scn *scn;
@@ -292,7 +331,7 @@ get_symbols(dtrace_hdl_t *dtp, Elf *elf, int syms_idx, int strs_idx,
* Allocate the symbol list.
*/
symc = symtab->d_size / sizeof(GElf_Sym);
- syms = calloc(symc, sizeof(dt_bpf_func_t *));
+ syms = dt_calloc(dtp, symc, sizeof(dt_bpf_func_t *));
if (syms == NULL) {
fprintf(stderr, "Failed to allocate symbol list\n");
symc = 0;
@@ -336,9 +375,14 @@ get_symbols(dtrace_hdl_t *dtp, Elf *elf, int syms_idx, int strs_idx,
syms[idx] = fp;
idp = dt_dlib_get_func(dtp, name);
- if (idp != NULL)
+ if (idp != NULL) {
+ /*
+ * Nasty trick to get our idops in place.
+ */
+ dt_ident_morph(idp, idp->di_kind,
+ &dt_idops_bpf, dtp);
dt_ident_set_data(idp, fp);
- else {
+ } else {
idp = dt_dlib_add_func(dtp, name, fp);
if (idp == NULL) {
fprintf(stderr,
@@ -458,6 +502,7 @@ get_symbols(dtrace_hdl_t *dtp, Elf *elf, int syms_idx, int strs_idx,
*/
if (fp->size % sizeof(struct bpf_insn)) {
fprintf(stderr, "'%s' seems truncated\n", fp->name);
+ dt_free(dtp, dp);
goto out;
}
if (*(uint64_t *)((char*)text->d_buf + fp->offset + fp->size -
@@ -490,7 +535,7 @@ get_symbols(dtrace_hdl_t *dtp, Elf *elf, int syms_idx, int strs_idx,
* DIFO. This will help us when we construct the actual DIFO relocs.
*/
relc = data->d_size / sizeof(GElf_Rel);
- rels = calloc(relc, sizeof(dt_bpf_reloc_t));
+ rels = dt_calloc(dtp, relc, sizeof(dt_bpf_reloc_t));
for (idx = 0, rp = rels; idx < relc; idx++, rp++) {
GElf_Rel rel;
@@ -599,13 +644,27 @@ done:
dt_strtab_destroy(stab);
}
+ /*
+ * Clean up fake function symbols.
+ */
+ for (idx = 0; idx < symc; idx++) {
+ fp = syms[idx];
+ if (fp == NULL ||
+ fp->id != -1 || fp->offset > 0 || fp->size > 0)
+ continue;
+
+ if (fp->name)
+ free(fp->name);
+
+ dt_free(dtp, fp);
+ }
+
out:
+ dt_free(dtp, syms);
dt_free(dtp, rels);
dt_free(dtp, funcs);
- *cnt = symc;
-
- return syms;
+ return symc == 0 ? -1 : 0;
}
static int
@@ -624,8 +683,6 @@ readBPFFile(dtrace_hdl_t *dtp, const char *fn)
Elf_Scn *scn;
GElf_Ehdr ehdr;
GElf_Shdr shdr;
- dt_bpf_func_t **syms;
- int symc;
/*
* Open the ELF object file and perform basic validation.
@@ -718,12 +775,8 @@ readBPFFile(dtrace_hdl_t *dtp, const char *fn)
break;
}
- syms = get_symbols(dtp, elf, syms_idx, strs_idx, text_idx, text_len,
- relo_idx, maps_idx, &symc);
- if (syms == NULL)
- goto out;
-
- rc = 0;
+ rc = get_symbols(dtp, elf, syms_idx, strs_idx, text_idx, text_len,
+ relo_idx, maps_idx);
goto out;
err_elf:
@@ -988,7 +1041,7 @@ dt_load_libs_dir(dtrace_hdl_t *dtp, const char *path)
if (type == DT_DLIB_BPF) {
readBPFFile(dtp, fname);
- return 0;
+ continue;
}
if ((fp = fopen(fname, "r")) == NULL) {
diff --git a/libdtrace/dt_htab.c b/libdtrace/dt_htab.c
index 5342a518..e8e7c13b 100644
--- a/libdtrace/dt_htab.c
+++ b/libdtrace/dt_htab.c
@@ -29,7 +29,7 @@
#include <stdint.h>
#include <stdlib.h>
-#include "dt_htab.h"
+#include "dt_impl.h"
typedef struct dt_hbucket dt_hbucket_t;
struct dt_hbucket {
@@ -50,9 +50,9 @@ struct dt_htab {
/*
* Create a new (empty) hashtable.
*/
-dt_htab_t *dt_htab_create(dt_htab_ops_t *ops)
+dt_htab_t *dt_htab_create(dtrace_hdl_t *dtp, dt_htab_ops_t *ops)
{
- dt_htab_t *htab = malloc(sizeof(dt_htab_t));
+ dt_htab_t *htab = dt_alloc(dtp, sizeof(dt_htab_t));
if (!htab)
return NULL;
@@ -62,9 +62,9 @@ dt_htab_t *dt_htab_create(dt_htab_ops_t *ops)
htab->nbuckets = 0;
htab->ops = ops;
- htab->tab = calloc(htab->size, sizeof(dt_hbucket_t *));
+ htab->tab = dt_calloc(dtp, htab->size, sizeof(dt_hbucket_t *));
if (!htab->tab) {
- free(htab);
+ dt_free(dtp, htab);
return NULL;
}
@@ -74,13 +74,13 @@ dt_htab_t *dt_htab_create(dt_htab_ops_t *ops)
/*
* Destroy a hashtable.
*/
-void dt_htab_destroy(dt_htab_t *htab)
+void dt_htab_destroy(dtrace_hdl_t *dtp, dt_htab_t *htab)
{
if (!htab)
return;
- free(htab->tab);
- free(htab);
+ dt_free(dtp, htab->tab);
+ dt_free(dtp, htab);
}
/*
diff --git a/libdtrace/dt_htab.h b/libdtrace/dt_htab.h
index bf51a29c..00a480a8 100644
--- a/libdtrace/dt_htab.h
+++ b/libdtrace/dt_htab.h
@@ -12,6 +12,8 @@
extern "C" {
#endif
+struct dtrace_hdl;
+
typedef uint32_t (*htab_hval_fn)(const void *);
typedef int (*htab_cmp_fn)(const void *, const void *);
typedef void *(*htab_add_fn)(void *, void *);
@@ -31,8 +33,8 @@ typedef struct dt_hentry {
typedef struct dt_htab dt_htab_t;
-extern dt_htab_t *dt_htab_create(dt_htab_ops_t *ops);
-extern void dt_htab_detroy(dt_htab_t *htab);
+extern dt_htab_t *dt_htab_create(struct dtrace_hdl *dtp, dt_htab_ops_t *ops);
+extern void dt_htab_destroy(struct dtrace_hdl *dtp, dt_htab_t *htab);
extern int dt_htab_insert(dt_htab_t *htab, void *entry);
extern void *dt_htab_lookup(const dt_htab_t *htab, const void *entry);
extern int dt_htab_delete(dt_htab_t *htab, void *entry);
diff --git a/libdtrace/dt_ident.c b/libdtrace/dt_ident.c
index 28c59158..e148821f 100644
--- a/libdtrace/dt_ident.c
+++ b/libdtrace/dt_ident.c
@@ -946,7 +946,8 @@ void
dt_ident_morph(dt_ident_t *idp, ushort_t kind,
const dt_idops_t *ops, void *iarg)
{
- idp->di_ops->di_dtor(idp);
+ if (idp->di_ops)
+ idp->di_ops->di_dtor(idp);
idp->di_kind = kind;
idp->di_ops = ops;
idp->di_iarg = iarg;
diff --git a/libdtrace/dt_open.c b/libdtrace/dt_open.c
index 0209840f..2c57a8a3 100644
--- a/libdtrace/dt_open.c
+++ b/libdtrace/dt_open.c
@@ -1144,6 +1144,8 @@ dtrace_close(dtrace_hdl_t *dtp)
if (dtp == NULL)
return;
+ dt_free(dtp, dtp->dt_conf.cpuids);
+
if (dtp->dt_procs != NULL)
dt_proc_hash_destroy(dtp);
@@ -1170,6 +1172,8 @@ dtrace_close(dtrace_hdl_t *dtp)
dt_idhash_destroy(dtp->dt_globals);
if (dtp->dt_tls != NULL)
dt_idhash_destroy(dtp->dt_tls);
+ if (dtp->dt_bpfsyms != NULL)
+ dt_idhash_destroy(dtp->dt_bpfsyms);
while ((dmp = dt_list_next(&dtp->dt_modlist)) != NULL)
dt_module_destroy(dtp, dmp);
diff --git a/libdtrace/dt_probe.c b/libdtrace/dt_probe.c
index 823f0dd2..b6c79354 100644
--- a/libdtrace/dt_probe.c
+++ b/libdtrace/dt_probe.c
@@ -1259,11 +1259,11 @@ dtrace_probe_iter(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp,
void
dt_probe_init(dtrace_hdl_t *dtp)
{
- dtp->dt_byprv = dt_htab_create(&prv_htab_ops);
- dtp->dt_bymod = dt_htab_create(&mod_htab_ops);
- dtp->dt_byfun = dt_htab_create(&fun_htab_ops);
- dtp->dt_byprb = dt_htab_create(&prb_htab_ops);
- dtp->dt_byfqn = dt_htab_create(&fqn_htab_ops);
+ dtp->dt_byprv = dt_htab_create(dtp, &prv_htab_ops);
+ dtp->dt_bymod = dt_htab_create(dtp, &mod_htab_ops);
+ dtp->dt_byfun = dt_htab_create(dtp, &fun_htab_ops);
+ dtp->dt_byprb = dt_htab_create(dtp, &prb_htab_ops);
+ dtp->dt_byfqn = dt_htab_create(dtp, &fqn_htab_ops);
dtp->dt_probes = NULL;
dtp->dt_probes_sz = 0;
@@ -1283,6 +1283,17 @@ dt_probe_fini(dtrace_hdl_t *dtp)
dt_probe_destroy(prp);
}
+
+ dt_htab_destroy(dtp, dtp->dt_byprv);
+ dt_htab_destroy(dtp, dtp->dt_bymod);
+ dt_htab_destroy(dtp, dtp->dt_byfun);
+ dt_htab_destroy(dtp, dtp->dt_byprb);
+ dt_htab_destroy(dtp, dtp->dt_byfqn);
+
+ dt_free(dtp, dtp->dt_probes);
+ dtp->dt_probes = NULL;
+ dtp->dt_probes_sz = 0;
+ dtp->dt_probe_id = 1;
}
void
diff --git a/libdtrace/dt_subr.c b/libdtrace/dt_subr.c
index 337827f4..3dcb92d8 100644
--- a/libdtrace/dt_subr.c
+++ b/libdtrace/dt_subr.c
@@ -739,6 +739,7 @@ dt_difo_free(dtrace_hdl_t *dtp, dtrace_difo_t *dp)
dt_free(dtp, dp->dtdo_buf);
dt_free(dtp, dp->dtdo_strtab);
dt_free(dtp, dp->dtdo_vartab);
+ dt_free(dtp, dp->dtdo_breltab);
dt_free(dtp, dp->dtdo_kreltab);
dt_free(dtp, dp->dtdo_ureltab);
dt_free(dtp, dp->dtdo_xlmtab);
--
2.26.0
More information about the DTrace-devel
mailing list