On Thu, Feb 2, 2012 at 6:46 PM, Jeff King <peff@xxxxxxxx> wrote:
> 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?
If I apply this patch:
---8<---
diff --git a/strbuf.c b/strbuf.c
index ff0b96b..52dfdd6 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -218,7 +218,7 @@ void strbuf_vaddf(struct strbuf *sb, const char
*fmt, va_list ap)
len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, cp);
va_end(cp);
if (len < 0)
- die("BUG: your vsnprintf is broken (returned %d)", len);
+ die_errno("BUG: your vsnprintf is broken (returned %d)", len);
if (len > strbuf_avail(sb)) {
strbuf_grow(sb, len);
len = vsnprintf(sb->buf + sb->len, sb->alloc - sb->len, fmt, ap);
---8<---
Then I get "fatal: BUG: your vsnprintf is broken (returned -1): Result
too large". This goes both for both failure cases I described. I
assume this means errno=ERANGE.
> 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.
No. SNPRINTF_RETURNS_BOGUS is only set for the MSVC target, not for
the MinGW target. I'm assuming that means MinGW-runtime has a sane
vsnprintf implementation. But even if I enable SNPRINTF_RETURNS_BOGUS,
the problem occurs. And it's still "Result too large".
So I decided to do a bit of stepping, and it seems libintl takes over
vsnprintf, directing us to libintl_vsnprintf instead. I guess this is
so it can ensure we support reordering the parameters with $1 etc...
And aparently this vsnprintf implementation calls the system vnsprintf
if the format string does not contain '$', and it's using _vsnprintf
rather than vsnprintf on Windows. _vsnprintf is the MSVCRT-version,
and not the MinGW-runtime, which needs SNPRINTF_RETURNS_BOGUS.
So I guess I can patch libintl to call vsnprintf from MinGW-runtime instead.
> 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.
Well, now at least I have some better leads, but I'm still not able to
explain the "if I add a random function, the problem is more
reproducible" bit. :(
>> 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?
Compiling with NO_GETTEXT does make the symptoms go away, but that's
not curing the problem ;)
But, I have a lead now. I'll see if I can find out *why* libintl calls
_vsnprintf on MinGW. I expect it's so the MSVC and the MinGW versions
behave similarly, MSVC doesn't have a sane vsnprintf. Perhaps I should
back-port SNPRINTF_RETURNS_BOGUS-workaround to libintl, so our MSVC
builds doesn't break also?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html