Patchwork D1948: lazymanifest: avoid reading uninitialized memory

login
register
mail settings
Submitter phabricator
Date Jan. 31, 2018, 2:42 a.m.
Message ID <differential-rev-PHID-DREV-m7bjnsmc6cgh4wq64mre-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/27084/
State Superseded
Headers show

Comments

phabricator - Jan. 31, 2018, 2:42 a.m.
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  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 scm/hg/mercurial/cext/manifest.c:781
    +      #1 0x7f6692fc1dc4 in call_function python/2.7.11/src/Python-2.7.11/Python/ceval.c:4350
    +      .......
    +  SUMMARY: UndefinedBehaviorSanitizer: invalid-bool-load scm/hg/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.

TEST PLAN
  Run `test-dirstate.t` with UBSAN and it no longer reports the issue.
  
  [1]: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1948

AFFECTED FILES
  mercurial/cext/manifest.c

CHANGE DETAILS




To: quark, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 31, 2018, 4:34 a.m.
indygreg added a comment.


  FWIW, I could not `hg phabread | hg import -` this revision because `hg import`'s patch parser got confused by the inline patch-looking content of the commit message. Good times.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1948

To: quark, #hg-reviewers, indygreg
Cc: mercurial-devel

Patch

diff --git a/mercurial/cext/manifest.c b/mercurial/cext/manifest.c
--- a/mercurial/cext/manifest.c
+++ b/mercurial/cext/manifest.c
@@ -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;
 		}