Re: [PATCH v4] reflog-walk: don't segfault on non-commit sha1's in the reflog
- From
Dennis Kaarsemaker <dennis@kaarsemaker.net>
- Date
- Jan 6, 2016, 09:13 UTC
- Message-ID
- <1452071624.2668.21.camel@kaarsemaker.net>
- In-Reply-To
- <CAPig+cQ7MineqezZXxpfAotVwoM9Ju1qwVGpEnEh9qNKBF1Pjg@mail.gmail.com>
On di, 2016-01-05 at 20:52 -0500, Eric Sunshine wrote:
Show 37 quoted lines
> On Tue, Jan 5, 2016 at 8:28 PM, Eric Sunshine <
> sunshine@sunshineco.com> wrote:
> > On Tue, Jan 5, 2016 at 8:20 PM, Dennis Kaarsemaker
> > <dennis@kaarsemaker.net> wrote:
> > > On di, 2016-01-05 at 20:05 -0500, Eric Sunshine wrote:
> > > > Hmm, this test is successful for me on OS X even without the
> > > > reflog-walk.c changes applied.
> > > >
> > > > And this test actually fails (inversely) because it's expecting
> > > > a
> > > > failure, but doesn't get one since the command produces the
> > > > expected
> > > > output.
> > >
> > > That's... surprising to say the least. What's the content of
> > > 'actual',
> > > and which git.git commit are you on?
> >
> > % cat t/trash\ directory.t1410-reflog/actual
> > b60a214 refs/tests/tree-in-reflog@{0}: Restoring to commit
> > 140c527 refs/tests/tree-in-reflog@{1}: Forcing tree
> > b60a214 refs/tests/tree-in-reflog@{2}: Creating ref
> > %
> >
> > This is with only the t/t1410-reflog.sh changes from your patch
> > applied atop current 'master' (SHA1 7548842).
>
> By the way, the segfault does occur for me on Linux and FreeBSD.
>
> And, in all cases, on all tested platforms, with the full patch
> applied, both tests behave sanely (in the expected fashion). So, even
> though the crash doesn't manifest everywhere, the fact that the tests
> are meaningfully testing it on the "affected" platforms may mean that
> it's not worth worrying about why it doesn't segfault on OS X.
>
> (Of course, practicality aside, one might want to satisfy one's
> intellectual curiosity about why it behaves differently on OS X.)The only explanation I can think of (and that's with practically no knowledge of OS X internals) is that OS X's memory allocation strategy is unlucky. Git is definitely writing to a location it should not write to. On linux and freebsd this is unallocated memory, so you get a segfault. On OS X, it happens to be memory actually allocated by git, resulting not in a segfault but in silent corruption of other in-memory data. I would argue that this is a much worse result, even though in this small test that corruption seems to not trigger a crash.
-- Dennis Kaarsemaker http://www.kaarsemaker.net