lazymanifest: avoid reading uninitialized memory stable
authorJun Wu <quark@fb.com>
Tue, 30 Jan 2018 20:32:48 -0800
branchstable
changeset 35810 113a30b87716
parent 35809 f9a82b9b2c36
child 35811 e35320ce8043
lazymanifest: avoid reading uninitialized memory I got errors running tests with clang UBSAN [1] enabled. One of them is: ``` --- test-dirstate.t +++ test-dirstate.t.err @@ -85,9 +85,115 @@ $ echo "[extensions]" >> .hg/hgrc $ echo "dirstateex=../dirstateexception.py" >> .hg/hgrc $ hg up 0 - abort: simulated error while recording dirstateupdates - [255] + mercurial/cext/manifest.c:781:13: runtime error: load of value 190, which is not a valid value for type 'bool' + #0 0x7f668a8cf748 in lazymanifest_diff mercurial/cext/manifest.c:781 + #1 0x7f6692fc1dc4 in call_function Python-2.7.11/Python/ceval.c:4350 + ....... + SUMMARY: UndefinedBehaviorSanitizer: invalid-bool-load mercurial/cext/manifest.c:781:13 in + [1] $ hg log -r . -T '{rev}\n' 1 $ hg status - ? a ``` While the code is not technically wrong, but switching the condition would make clang UBSAN happy. So let's do it. The uninitialized memory could come from, for example, `lazymanifest_copy` allocates `self->maxlines` items but only writes the first `self->lines` items. [1]: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html Test Plan: Run `test-dirstate.t` with UBSAN and it no longer reports the issue. Differential Revision: https://phab.mercurial-scm.org/D1948
mercurial/cext/manifest.c
--- a/mercurial/cext/manifest.c	Fri Jan 26 11:42:47 2018 -0800
+++ b/mercurial/cext/manifest.c	Tue Jan 30 20:32:48 2018 -0800
@@ -778,11 +778,11 @@
 		PyObject *outer;
 		/* If we're looking at a deleted entry and it's not
 		 * the end of the manifest, just skip it. */
-		if (left->deleted && sneedle < self->numlines) {
+		if (sneedle < self->numlines && left->deleted) {
 			sneedle++;
 			continue;
 		}
-		if (right->deleted && oneedle < other->numlines) {
+		if (oneedle < other->numlines && right->deleted) {
 			oneedle++;
 			continue;
 		}