git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 1/2] negotiator/default: avoid stack overflow

From
Han Xin <hanxin.hx@bytedance.com>
Date
Apr 26, 2023, 04:05 UTC
Message-ID
<935be72eb92cd2eda7aff43c8cc2306b78b2a146.1682473718.git.hanxin.hx@bytedance.com>
In-Reply-To
<cover.1682473718.git.hanxin.hx@bytedance.com>

mark_common() in negotiator/default.c may overflow the stack due to recursive function calls. Avoid this by instead recursing using a heap-allocated data structure.

This is the same case as [1].
1. 4654134976f (negotiator/skipping: avoid stack overflow, 2022-10-25)
Reported-by: Xin Xing <xingxin.xx@bytedance.com>
Signed-off-by: Han Xin <hanxin.hx@bytedance.com>
---
 negotiator/default.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/negotiator/default.c b/negotiator/default.c
index f4b78eb47d..635cdd6483 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -55,30 +55,49 @@ static int clear_marks(const char *refname, const struct object_id *oid,
 static void mark_common(struct negotiation_state *ns, struct commit *commit,
 		int ancestors_only, int dont_parse)
 {
-	if (commit != NULL && !(commit->object.flags & COMMON)) {
-		struct object *o = (struct object *)commit;
+	struct prio_queue queue = { NULL };
+
+	if (!commit || (commit->object.flags & COMMON))
+		return;
+
+	prio_queue_put(&queue, commit);
+	if (!ancestors_only) {
+		commit->object.flags |= COMMON;
 
-		if (!ancestors_only)
-			o->flags |= COMMON;
+		if ((commit->object.flags & SEEN) && !(commit->object.flags & POPPED))
+			ns->non_common_revs--;
+	}
+	while ((commit = prio_queue_get(&queue))) {
+		struct object *o = (struct object *)commit;
 
 		if (!(o->flags & SEEN))
 			rev_list_push(ns, commit, SEEN);
 		else {
 			struct commit_list *parents;
 
-			if (!ancestors_only && !(o->flags & POPPED))
-				ns->non_common_revs--;
 			if (!o->parsed && !dont_parse)
 				if (repo_parse_commit(the_repository, commit))
-					return;
+					continue;
 
 			for (parents = commit->parents;
 					parents;
-					parents = parents->next)
-				mark_common(ns, parents->item, 0,
-					    dont_parse);
+					parents = parents->next) {
+				struct commit *p = parents->item;
+
+				if (p->object.flags & COMMON)
+					continue;
+
+				p->object.flags |= COMMON;
+
+				if ((p->object.flags & SEEN) && !(p->object.flags & POPPED))
+					ns->non_common_revs--;
+
+				prio_queue_put(&queue, parents->item);
+			}
 		}
 	}
+
+	clear_prio_queue(&queue);
 }
 
 /*
-- 
2.40.0
Previous: Han XinNext: Derrick Stolee
Message 9 of 20 in “negotiator/default.c: avoid stack overflow”
  1. negotiator/default.c: avoid stack overflowHan Xin, Apr 24, 2023
  2. Derrick StoleeApr 24, 2023
  3. Han XinApr 25, 2023
  4. Derrick StoleeApr 25, 2023
  5. 0/2 negotiator/default: avoid stack overflowHan Xin, Apr 26, 2023
  6. 2/2 negotiator/skipping: fix some problems in mark_common()Han Xin, Apr 26, 2023
  7. Derrick StoleeApr 26, 2023
  8. Han XinApr 26, 2023
  9. 1/2 negotiator/default: avoid stack overflowHan Xin, Apr 26, 2023
  10. Derrick StoleeApr 26, 2023
  11. Han XinApr 26, 2023
  12. 0/2 negotiator/default: avoid stack overflowHan Xin, Apr 26, 2023
  13. 1/2 negotiator/default: avoid stack overflowHan Xin, Apr 26, 2023
  14. Junio C HamanoApr 26, 2023
  15. Derrick StoleeApr 26, 2023
  16. Junio C HamanoApr 26, 2023
  17. 2/2 negotiator/skipping: fix some problems in mark_common()Han Xin, Apr 26, 2023
  18. Junio C HamanoMay 1, 2023
  19. Derrick StoleeMay 2, 2023
  20. Junio C HamanoMay 2, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.