Re: [PATCH v4] reflog-walk: don't segfault on non-commit sha1's in the reflog

[Dennis Kaarsemaker <dennis@kaarsemaker.net>]

On di, 2016-01-05 at 20:52 -0500, Eric Sunshine wrote:
> 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