Re: Breakage in master?

[Jeff King <peff@peff.net>]

On Thu, Feb 02, 2012 at 01:14:19PM +0100, Erik Faye-Lund wrote:

> But here's the REALLY puzzling part: If I add a simple, unused
> function to diff-lib.c, like this:
> [...]
> "git status" starts to error out with that same vsnprintf complaint!
> 
> ---8<---
> $ git status
> # On branch master
> # Changes not staged for commit:
> #   (use "git add <file>..." to update what will be committed)
> fatal: BUG: your vsnprintf is broken (returned -1)
> ---8<---

OK, that's definitely odd.

At the moment of the die() in strbuf_vaddf, what does errno say?
vsnprintf should generally never be returning -1 (it should return the
number of characters that would have been written). Since you're on
Windows, I assume you're using the replacement version in
compat/snprintf.c.

That one will return -1 if realloc fails. So I'm curious if that is what
is happening (you might also instrument the call to realloc in
snprintf.c to see if it is failing, and if so, at what maxsize). And/or
check errno in git_vsnprintf after calling the native vsnprintf and
getting -1.

Here's one possible sequence of events that seems plausible to me (and
remember that this is a wild guess):

  1. gettext somehow munges the format string in a way that Windows
     vsnprintf doesn't like, and it returns -1.

  2. Our git_vsnprintf wrapper interprets this -1 as "you didn't give me
     enough space to store the result", and we grow our test-buffer to
     try again

  3. Eventually the test buffer gets unreasonably large, and realloc
     fails. We have no choice but to return -1 from our wrapper.

  4. strbuf_vaddf sees the -1 and thinks you are using a broken
     vsnprintf.

All of that would make sense to me, _except_ for your weird "if I add a
random function, the problem is more reproducible" bit. Which does seem
like something is invoking undefined behavior (of course, it could be
that undefined behavior or stack-smashing that is causing vsnprintf to
report an error). Lacking any better leads, it might be worth pursuing.

> I've bisected the issues down to 5e9637c (i18n: add infrastructure for
> translating Git with gettext). Trying to apply my unused-function
> patch on top of this commit starts giving the same "fatal: BUG: your
> vsnprintf is broken (returned -1)" error. It's ancestor, bc1bbe0(Git
> 1.7.8-rc2), does not yield any of the issues.

I've looked at 5e9637c, and it really doesn't do anything that looks
bad. I wonder if your gettext library is buggy. Does compiling with
NO_GETTEXT help?

-Peff