common/memleak: take over dump_memleak(), allow print pointer.

This will let plugins use it. Signed-off-by: Rusty Russell <[email protected]>
johnongit · Sep 8, 2021 · a5fee67 · a5fee67
1 parent b34953d
commit a5fee67
Show file tree

Hide file tree

Showing 17 changed files with 127 additions and 91 deletions.
diff --git a/channeld/channeld.c b/channeld/channeld.c
@@ -3496,7 +3496,7 @@ static void handle_dev_memleak(struct peer *peer, const u8 *msg)
 	/* Now delete peer and things it has pointers to. */
 	memleak_remove_region(memtable, peer, tal_bytelen(peer));
 
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	wire_sync_write(MASTER_FD,
 			 take(towire_channeld_dev_memleak_reply(NULL,
 							       found_leak)));

diff --git a/closingd/closingd.c b/closingd/closingd.c
@@ -485,7 +485,7 @@ static void closing_dev_memleak(const tal_t *ctx,
 	memleak_remove_pointer(memtable, scriptpubkey[REMOTE]);
 	memleak_remove_pointer(memtable, funding_wscript);
 
-	dump_memleak(memtable);
+	dump_memleak(memtable, memleak_status_broken);
 }
 #endif /* DEVELOPER */
 

diff --git a/common/memleak.c b/common/memleak.c
@@ -290,6 +290,59 @@ void memleak_init(void)
 	if (backtrace_state)
 		add_backtrace_notifiers(NULL);
 }
+
+static int dump_syminfo(void *data, uintptr_t pc UNUSED,
+			const char *filename, int lineno,
+			const char *function)
+{
+	void PRINTF_FMT(1,2) (*print)(const char *fmt, ...) = data;
+	/* This can happen in backtraces. */
+	if (!filename || !function)
+		return 0;
+
+	print("    %s:%u (%s)", filename, lineno, function);
+	return 0;
+}
+
+static void dump_leak_backtrace(const uintptr_t *bt,
+				void PRINTF_FMT(1,2)
+				(*print)(const char *fmt, ...))
+{
+	if (!bt)
+		return;
+
+	/* First one serves as counter. */
+	print("  backtrace:");
+	for (size_t i = 1; i < bt[0]; i++) {
+		backtrace_pcinfo(backtrace_state,
+				 bt[i], dump_syminfo,
+				 NULL, print);
+	}
+}
+
+bool dump_memleak(struct htable *memtable,
+		  void PRINTF_FMT(1,2) (*print)(const char *fmt, ...))
+{
+	const tal_t *i;
+	const uintptr_t *backtrace;
+	bool found_leak = false;
+
+	while ((i = memleak_get(memtable, &backtrace)) != NULL) {
+		print("MEMLEAK: %p", i);
+		if (tal_name(i))
+			print("  label=%s", tal_name(i));
+
+		dump_leak_backtrace(backtrace, print);
+		print("  parents:");
+		for (tal_t *p = tal_parent(i); p; p = tal_parent(p)) {
+			print("    %s", tal_name(p));
+			p = tal_parent(p);
+		}
+		found_leak = true;
+	}
+
+	return found_leak;
+}
 #else /* !DEVELOPER */
 void *notleak_(const void *ptr, bool plus_children UNNEEDED)
 {

diff --git a/common/memleak.h b/common/memleak.h
@@ -121,4 +121,10 @@ const void *memleak_get(struct htable *memtable, const uintptr_t **backtrace);
 
 extern struct backtrace_state *backtrace_state;
 
+#if DEVELOPER
+/* Only defined if DEVELOPER */
+bool dump_memleak(struct htable *memtable,
+		  void PRINTF_FMT(1,2) (*print)(const char *fmt, ...));
+#endif
+
 #endif /* LIGHTNING_COMMON_MEMLEAK_H */
diff --git a/common/status.c b/common/status.c
@@ -226,3 +226,14 @@ void master_badmsg(u32 type_expected, const u8 *msg)
 		     "Error parsing %u: %s",
 		     type_expected, tal_hex(tmpctx, msg));
 }
+
+#if DEVELOPER
+/* Print BROKEN status: callback for dump_memleak. */
+void memleak_status_broken(const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	status_vfmt(LOG_BROKEN, NULL, fmt, ap);
+	va_end(ap);
+}
+#endif
diff --git a/common/status.h b/common/status.h
@@ -74,4 +74,10 @@ void status_send_fatal(const u8 *msg TAKES) NORETURN;
 
 /* Only for sync status! */
 void status_send_fd(int fd);
+
+#if DEVELOPER
+/* Print BROKEN status: callback for dump_memleak. */
+void memleak_status_broken(const char *fmt, ...);
+#endif
+
 #endif /* LIGHTNING_COMMON_STATUS_H */
diff --git a/common/subdaemon.c b/common/subdaemon.c
@@ -50,58 +50,3 @@ void subdaemon_setup(int argc, char *argv[])
 
 	daemon_setup(argv[0], status_backtrace_print, status_backtrace_exit);
 }
-
-#if DEVELOPER
- // Indented to avoid header-order check
- #include <backtrace.h>
- #include <common/memleak.h>
-
-static int dump_syminfo(void *data UNUSED, uintptr_t pc UNUSED,
-			const char *filename, int lineno,
-			const char *function)
-{
-	/* This can happen in backtraces. */
-	if (!filename || !function)
-		return 0;
-
-	status_debug("    %s:%u (%s)", filename, lineno, function);
-	return 0;
-}
-
-static void dump_leak_backtrace(const uintptr_t *bt)
-{
-	if (!bt)
-		return;
-
-	/* First one serves as counter. */
-	status_debug("  backtrace:");
-	for (size_t i = 1; i < bt[0]; i++) {
-		backtrace_pcinfo(backtrace_state,
-				 bt[i], dump_syminfo,
-				 NULL, NULL);
-	}
-}
-
-bool dump_memleak(struct htable *memtable)
-{
-	const tal_t *i;
-	const uintptr_t *backtrace;
-	bool found_leak = false;
-
-	while ((i = memleak_get(memtable, &backtrace)) != NULL) {
-		status_broken("MEMLEAK: %p", i);
-		if (tal_name(i))
-			status_broken("  label=%s", tal_name(i));
-
-		dump_leak_backtrace(backtrace);
-		status_broken("  parents:");
-		for (tal_t *p = tal_parent(i); p; p = tal_parent(p)) {
-			status_broken("    %s", tal_name(p));
-			p = tal_parent(p);
-		}
-		found_leak = true;
-	}
-
-	return found_leak;
-}
-#endif /* DEVELOPER */
diff --git a/common/subdaemon.h b/common/subdaemon.h
@@ -7,7 +7,4 @@ struct htable;
 /* daemon_setup, but for subdaemons */
 void subdaemon_setup(int argc, char *argv[]);
 
-/* Only defined if DEVELOPER */
-bool dump_memleak(struct htable *memtable);
-
 #endif /* LIGHTNING_COMMON_SUBDAEMON_H */
diff --git a/connectd/connectd.c b/connectd/connectd.c
@@ -1734,7 +1734,7 @@ static struct io_plan *dev_connect_memleak(struct io_conn *conn,
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_remove_region(memtable, daemon, sizeof(daemon));
 
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	daemon_conn_send(daemon->master,
 			 take(towire_connectd_dev_memleak_reply(NULL,
 							      found_leak)));

diff --git a/gossipd/gossipd.c b/gossipd/gossipd.c
@@ -1260,7 +1260,7 @@ static struct io_plan *dev_gossip_memleak(struct io_conn *conn,
 	/* Now delete daemon and those which it has pointers to. */
 	memleak_remove_region(memtable, daemon, sizeof(*daemon));
 
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	daemon_conn_send(daemon->master,
 			 take(towire_gossipd_dev_memleak_reply(NULL,
 							      found_leak)));

diff --git a/hsmd/hsmd.c b/hsmd/hsmd.c
@@ -580,7 +580,7 @@ static struct io_plan *handle_memleak(struct io_conn *conn,
 	memleak_remove_pointer(memtable, dev_force_privkey);
 	memleak_remove_pointer(memtable, dev_force_bip32_seed);
 
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	reply = towire_hsmd_dev_memleak_reply(NULL, found_leak);
 	return req_reply(conn, c, take(reply));
 }

diff --git a/onchaind/onchaind.c b/onchaind/onchaind.c
@@ -2162,7 +2162,7 @@ static bool handle_dev_memleak(struct tracked_output **outs, const u8 *msg)
 	memleak_remove_globals(memtable, tal_parent(outs));
 	memleak_remove_region(memtable, outs, tal_bytelen(outs));
 
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	wire_sync_write(REQ_FD,
 			take(towire_onchaind_dev_memleak_reply(NULL,
 							      found_leak)));

diff --git a/onchaind/test/run-grind_feerate-bug.c b/onchaind/test/run-grind_feerate-bug.c
@@ -25,9 +25,6 @@ bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
-/* Generated stub for dump_memleak */
-bool dump_memleak(struct htable *memtable UNNEEDED)
-{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
 { fprintf(stderr, "fromwire called!\n"); abort(); }
@@ -115,15 +112,6 @@ struct bitcoin_tx *htlc_success_tx(const tal_t *ctx UNNEEDED,
 /* Generated stub for master_badmsg */
 void master_badmsg(u32 type_expected UNNEEDED, const u8 *msg)
 { fprintf(stderr, "master_badmsg called!\n"); abort(); }
-/* Generated stub for memleak_find_allocations */
-struct htable *memleak_find_allocations(const tal_t *ctx UNNEEDED,
-					const void *exclude1 UNNEEDED,
-					const void *exclude2 UNNEEDED)
-{ fprintf(stderr, "memleak_find_allocations called!\n"); abort(); }
-/* Generated stub for memleak_remove_region */
-void memleak_remove_region(struct htable *memtable UNNEEDED,
-			   const void *p UNNEEDED, size_t bytelen UNNEEDED)
-{ fprintf(stderr, "memleak_remove_region called!\n"); abort(); }
 /* Generated stub for new_coin_chain_fees */
 struct chain_coin_mvt *new_coin_chain_fees(const tal_t *ctx UNNEEDED,
 					   const char *account_name UNNEEDED,
@@ -294,6 +282,25 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE
 { fprintf(stderr, "towire_u8_array called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
+#if DEVELOPER
+/* Generated stub for memleak_find_allocations */
+struct htable *memleak_find_allocations(const tal_t *ctx UNNEEDED,
+					const void *exclude1 UNNEEDED,
+					const void *exclude2 UNNEEDED)
+{ fprintf(stderr, "memleak_find_allocations called!\n"); abort(); }
+/* Generated stub for memleak_remove_region */
+void memleak_remove_region(struct htable *memtable UNNEEDED,
+			   const void *p UNNEEDED, size_t bytelen UNNEEDED)
+{ fprintf(stderr, "memleak_remove_region called!\n"); abort(); }
+/* Generated stub for memleak_status_broken */
+void memleak_status_broken(const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "memleak_status_broken called!\n"); abort(); }
+/* Generated stub for dump_memleak */
+bool dump_memleak(struct htable *memtable UNNEEDED,
+		  void  (*print)(const char *fmt UNNEEDED, ...))
+{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+#endif
+
 /* Stubs which do get called. */
 u8 *towire_hsmd_sign_local_htlc_tx(const tal_t *ctx UNNEEDED, u64 commit_num UNNEEDED, const struct bitcoin_tx *tx UNNEEDED, const u8 *wscript UNNEEDED, bool option_anchor_outputs UNNEEDED)
 {

diff --git a/onchaind/test/run-grind_feerate.c b/onchaind/test/run-grind_feerate.c
@@ -26,9 +26,6 @@ bool derive_keyset(const struct pubkey *per_commitment_point UNNEEDED,
 		   bool option_static_remotekey UNNEEDED,
 		   struct keyset *keyset UNNEEDED)
 { fprintf(stderr, "derive_keyset called!\n"); abort(); }
-/* Generated stub for dump_memleak */
-bool dump_memleak(struct htable *memtable UNNEEDED)
-{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)
 { fprintf(stderr, "fromwire called!\n"); abort(); }
@@ -320,6 +317,16 @@ bool wire_sync_write(int fd UNNEEDED, const void *msg TAKES UNNEEDED)
 { fprintf(stderr, "wire_sync_write called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
+#if DEVELOPER
+/* Generated stub for memleak_status_broken */
+void memleak_status_broken(const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "memleak_status_broken called!\n"); abort(); }
+/* Generated stub for dump_memleak */
+bool dump_memleak(struct htable *memtable UNNEEDED,
+		  void  (*print)(const char *fmt UNNEEDED, ...))
+{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+#endif
+
 int main(int argc, char *argv[])
 {
 	common_setup(argv[0]);

diff --git a/onchaind/test/run-onchainstress.c b/onchaind/test/run-onchainstress.c
@@ -44,16 +44,6 @@ struct htable *memleak_find_allocations(const tal_t *ctx UNNEEDED,
 					const void *exclude1 UNNEEDED,
 					const void *exclude2 UNNEEDED)
 { fprintf(stderr, "memleak_find_allocations called!\n"); abort(); }
-/* Generated stub for memleak_get */
-const void *memleak_get(struct htable *memtable UNNEEDED, const uintptr_t **backtrace UNNEEDED)
-{ fprintf(stderr, "memleak_get called!\n"); abort(); }
-/* Generated stub for memleak_init */
-void memleak_init(void)
-{ fprintf(stderr, "memleak_init called!\n"); abort(); }
-/* Generated stub for memleak_remove_region */
-void memleak_remove_region(struct htable *memtable UNNEEDED,
-			   const void *p UNNEEDED, size_t bytelen UNNEEDED)
-{ fprintf(stderr, "memleak_remove_region called!\n"); abort(); }
 /* Generated stub for new_coin_penalty_sat */
 struct chain_coin_mvt *new_coin_penalty_sat(const tal_t *ctx UNNEEDED,
 					    const char *account_name UNNEEDED,
@@ -105,6 +95,20 @@ const char *version(void)
 /* Generated stub for dev_disconnect_init */
 void dev_disconnect_init(int fd UNNEEDED)
 { fprintf(stderr, "dev_disconnect_init called!\n"); abort(); }
+/* Generated stub for dump_memleak */
+bool dump_memleak(struct htable *memtable UNNEEDED,
+		  void  (*print)(const char *fmt UNNEEDED, ...))
+{ fprintf(stderr, "dump_memleak called!\n"); abort(); }
+/* Generated stub for memleak_init */
+void memleak_init(void)
+{ fprintf(stderr, "memleak_init called!\n"); abort(); }
+/* Generated stub for memleak_remove_region */
+void memleak_remove_region(struct htable *memtable UNNEEDED,
+			   const void *p UNNEEDED, size_t bytelen UNNEEDED)
+{ fprintf(stderr, "memleak_remove_region called!\n"); abort(); }
+/* Generated stub for memleak_status_broken */
+void memleak_status_broken(const char *fmt UNNEEDED, ...)
+{ fprintf(stderr, "memleak_status_broken called!\n"); abort(); }
 #endif
 
 /* Noops */

diff --git a/openingd/dualopend.c b/openingd/dualopend.c
@@ -898,7 +898,7 @@ static void dualopend_dev_memleak(struct state *state)
 	memleak_remove_region(memtable, state, tal_bytelen(state));
 
 	/* If there's anything left, dump it to logs, and return true. */
-	dump_memleak(memtable);
+	dump_memleak(memtable, memleak_status_broken);
 }
 #endif /* DEVELOPER */
 

diff --git a/openingd/openingd.c b/openingd/openingd.c
@@ -1234,7 +1234,7 @@ static void handle_dev_memleak(struct state *state, const u8 *msg)
 	memleak_remove_region(memtable, state, sizeof(*state));
 
 	/* If there's anything left, dump it to logs, and return true. */
-	found_leak = dump_memleak(memtable);
+	found_leak = dump_memleak(memtable, memleak_status_broken);
 	wire_sync_write(REQ_FD,
 			take(towire_openingd_dev_memleak_reply(NULL,
 							      found_leak)));