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

[PATCH v2 06/10] rerere: fix crash when conflict goes unresolved

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Jun 5, 2018, 21:52 UTC
Message-ID
<20180605215219.28783-7-t.gummerer@gmail.com>
In-Reply-To
<20180605215219.28783-1-t.gummerer@gmail.com>

Currently when a user doesn't resolve a conflict in a file, but commits the file with the conflict markers, and later the file ends up in a state in which rerere can't handle it, subsequent rerere operations that are interested in that path, such as 'rerere clear' or 'rerere forget <path>' will fail, or even worse in the case of 'rerere clear' segfault.

Such states include nested conflicts, or an extra conflict marker that doesn't have any match.

This is because the first 'git rerere' when there was only one conflict in the file leaves an entry in the MERGE_RR file behind. The next 'git rerere' will then pick the rerere ID for that file up, and not assign a new ID as it can't successfully calculate one. It will however still try to do the rerere operation, because of the existing ID. As the handle_file function fails, it will remove the 'preimage' for the ID in the process, while leaving the ID in the MERGE_RR file.

Now when 'rerere clear' for example is run, it will segfault in 'has_rerere_resolution', because status is NULL.

To fix this, remove the rerere ID from the MERGE_RR file in the case when we can't handle it, and remove the corresponding variant from .git/rr-cache/. Removing it unconditionally is fine here, because if the user would have resolved the conflict and ran rerere, the entry would no longer be in the MERGE_RR file, so we wouldn't have this problem in the first place, while if the conflict was not resolved, the only thing that's left in the folder is the 'preimage', which by itself will be regenerated by git if necessary, so the user won't loose any work.

Note that other variants that have the same conflict ID will not be touched.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 rerere.c          | 12 +++++++-----
 t/t4200-rerere.sh | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/rerere.c b/rerere.c
index ef23abe4dd..220020187b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -824,10 +824,7 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		struct rerere_id *id;
 		unsigned char sha1[20];
 		const char *path = conflict.items[i].string;
-		int ret;
-
-		if (string_list_has_string(rr, path))
-			continue;
+		int ret, has_string;
 
 		/*
 		 * Ask handle_file() to scan and assign a
@@ -835,7 +832,12 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 		 * yet.
 		 */
 		ret = handle_file(path, sha1, NULL);
-		if (ret < 1)
+		has_string = string_list_has_string(rr, path);
+		if (ret < 0 && has_string) {
+			remove_variant(string_list_lookup(rr, path)->util);
+			string_list_remove(rr, path, 1);
+		}
+		if (ret < 1 || has_string)
 			continue;
 
 		id = new_rerere_id(sha1);
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index eaf18c81cb..5ce411b70d 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -580,4 +580,26 @@ test_expect_success 'multiple identical conflicts' '
 	count_pre_post 0 0
 '
 
+test_expect_success 'rerere with unexpected conflict markers does not crash' '
+	git reset --hard &&
+
+	git checkout -b branch-1 master &&
+	echo "bar" >test &&
+	git add test &&
+	git commit -q -m two &&
+
+	git reset --hard &&
+	git checkout -b branch-2 master &&
+	echo "foo" >test &&
+	git add test &&
+	git commit -q -a -m one &&
+
+	test_must_fail git merge branch-1 &&
+	sed "s/bar/>>>>>>> a/" >test.tmp <test &&
+	mv test.tmp test &&
+	git rerere &&
+
+	git rerere clear
+'
+
 test_done
-- 
2.17.0.410.g65aef3a6c4
Previous: Thomas GummererNext: Thomas Gummerer
Message 27 of 84 in “rerere: handle nested conflicts”
  1. 0/7 rerere: handle nested conflictsThomas Gummerer, May 20, 2018
  2. 1/7 rerere: unify error message when read_cache failsThomas Gummerer, May 20, 2018
  3. Stefan BellerMay 21, 2018
  4. 2/7 rerere: mark strings for translationThomas Gummerer, May 20, 2018
  5. Junio C HamanoMay 24, 2018
  6. 3/7 rerere: add some documentationThomas Gummerer, May 20, 2018
  7. Junio C HamanoMay 24, 2018
  8. Thomas GummererJun 3, 2018
  9. 4/7 rerere: fix crash when conflict goes unresolvedThomas Gummerer, May 20, 2018
  10. Junio C HamanoMay 24, 2018
  11. Thomas GummererMay 24, 2018
  12. Junio C HamanoMay 25, 2018
  13. 5/7 rerere: only return whether a path has conflicts or notThomas Gummerer, May 20, 2018
  14. Junio C HamanoMay 24, 2018
  15. 6/7 rerere: factor out handle_conflict functionThomas Gummerer, May 20, 2018
  16. 7/7 rerere: teach rerere to handle nested conflictsThomas Gummerer, May 20, 2018
  17. Junio C HamanoMay 24, 2018
  18. Thomas GummererMay 24, 2018
  19. 00/10 rerere: handle nested conflictsThomas Gummerer, Jun 5, 2018
  20. 01/10 rerere: unify error messages when read_cache failsThomas Gummerer, Jun 5, 2018
  21. 03/10 rerere: wrap paths in output in sqThomas Gummerer, Jun 5, 2018
  22. 05/10 rerere: add some documentationThomas Gummerer, Jun 5, 2018
  23. 07/10 rerere: only return whether a path has conflicts or notThomas Gummerer, Jun 5, 2018
  24. 09/10 rerere: teach rerere to handle nested conflictsThomas Gummerer, Jun 5, 2018
  25. 10/10 rerere: recalculate conflict ID when unresolved conflict is committedThomas Gummerer, Jun 5, 2018
  26. 02/10 rerere: lowercase error messagesThomas Gummerer, Jun 5, 2018
  27. 06/10 rerere: fix crash when conflict goes unresolvedThomas Gummerer, Jun 5, 2018
  28. 04/10 rerere: mark strings for translationThomas Gummerer, Jun 5, 2018
  29. 08/10 rerere: factor out handle_conflict functionThomas Gummerer, Jun 5, 2018
  30. Thomas GummererJul 3, 2018
  31. Junio C HamanoJul 6, 2018
  32. Thomas GummererJul 10, 2018
  33. 00/11 rerere: handle nested conflictsThomas Gummerer, Jul 14, 2018
  34. 01/11 rerere: unify error messages when read_cache failsThomas Gummerer, Jul 14, 2018
  35. 03/11 rerere: wrap paths in output in sqThomas Gummerer, Jul 14, 2018
  36. 02/11 rerere: lowercase error messagesThomas Gummerer, Jul 14, 2018
  37. 04/11 rerere: mark strings for translationThomas Gummerer, Jul 14, 2018
  38. Simon RuderichJul 15, 2018
  39. Thomas GummererJul 16, 2018
  40. 06/11 rerere: fix crash when conflict goes unresolvedThomas Gummerer, Jul 14, 2018
  41. Junio C HamanoJul 30, 2018
  42. Thomas GummererJul 30, 2018
  43. 05/11 rerere: add documentation for conflict normalizationThomas Gummerer, Jul 14, 2018
  44. Junio C HamanoJul 30, 2018
  45. Thomas GummererJul 30, 2018
  46. 07/11 rerere: only return whether a path has conflicts or notThomas Gummerer, Jul 14, 2018
  47. Junio C HamanoJul 30, 2018
  48. Thomas GummererJul 30, 2018
  49. 08/11 rerere: factor out handle_conflict functionThomas Gummerer, Jul 14, 2018
  50. Junio C HamanoJul 30, 2018
  51. 09/11 rerere: return strbuf from handle pathThomas Gummerer, Jul 14, 2018
  52. Junio C HamanoJul 30, 2018
  53. 10/11 rerere: teach rerere to handle nested conflictsThomas Gummerer, Jul 14, 2018
  54. Junio C HamanoJul 30, 2018
  55. Thomas GummererJul 30, 2018
  56. 11/11 rerere: recalculate conflict ID when unresolved conflict is committedThomas Gummerer, Jul 14, 2018
  57. Junio C HamanoJul 30, 2018
  58. Thomas GummererJul 30, 2018
  59. 00/11 rerere: handle nested conflictsThomas Gummerer, Aug 5, 2018
  60. 01/11 rerere: unify error messages when read_cache failsThomas Gummerer, Aug 5, 2018
  61. 02/11 rerere: lowercase error messagesThomas Gummerer, Aug 5, 2018
  62. 03/11 rerere: wrap paths in output in sqThomas Gummerer, Aug 5, 2018
  63. 04/11 rerere: mark strings for translationThomas Gummerer, Aug 5, 2018
  64. 05/11 rerere: add documentation for conflict normalizationThomas Gummerer, Aug 5, 2018
  65. 06/11 rerere: fix crash with files rerere can't handleThomas Gummerer, Aug 5, 2018
  66. 07/11 rerere: only return whether a path has conflicts or notThomas Gummerer, Aug 5, 2018
  67. 08/11 rerere: factor out handle_conflict functionThomas Gummerer, Aug 5, 2018
  68. 09/11 rerere: return strbuf from handle pathThomas Gummerer, Aug 5, 2018
  69. 10/11 rerere: teach rerere to handle nested conflictsThomas Gummerer, Aug 5, 2018
  70. Ævar Arnfjörð BjarmasonAug 22, 2018
  71. Junio C HamanoAug 22, 2018
  72. Thomas GummererAug 22, 2018
  73. Junio C HamanoAug 22, 2018
  74. Thomas GummererAug 24, 2018
  75. 1/2 rerere: remove documentation for "nested conflicts"Thomas Gummerer, Aug 24, 2018
  76. 2/2 rerere: add not about files with existing conflict markersThomas Gummerer, Aug 24, 2018
  77. 1/2 rerere: mention caveat about unmatched conflict markersThomas Gummerer, Aug 28, 2018
  78. 2/2 rerere: add note about files with existing conflict markersThomas Gummerer, Aug 28, 2018
  79. Junio C HamanoAug 29, 2018
  80. Thomas GummererSep 1, 2018
  81. Junio C HamanoAug 27, 2018
  82. Thomas GummererAug 28, 2018
  83. Junio C HamanoAug 27, 2018
  84. 11/11 rerere: recalculate conflict ID when unresolved conflict is committedThomas Gummerer, Aug 5, 2018

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.