threads / discuss / 11333

FLEX_ARRAY=1 causes SIGSEGV on SPARC

Subject: FLEX_ARRAY=1 causes SIGSEGV on SPARC

## tl;dr

3 messages between Dec 18, 2007 and Dec 18, 2007.

replies: 2people: 3as markdown or json

Shawn O. Pearce· Dec 18, 2007, 01:01 UTC · lore

So today I discovered latest master will cause a SIGSEGV on Solaris/SPARC if FLEX_ARRAY is defined to the default value of 1.

The issue is the (old) compiler I'm using from Sun doesn't pass the tests defined in 8e9739914972419baad820e76b44d9720ed885c2 (aka "git-compat-util.h: auto-adjust to compiler support of FLEX_ARRAY a bit better") so we fallback to #define FLEX_ARRAY 1.

Then git-pack-objects finds something unaligned and segfaults. It always segfaults. During the deltification phase. Probably it was working on deltifying loose objects; the set I fed it was about 100 most recent objects so they were probably all loose.

I'll try to track it down tomorrow. But the immediate workaround was to just add '-DFLEX_ARRAY=/* empty */' to my CFLAGS and recompile the world. This compiler accepts the empty FLEX_ARRAY macro but I'm not sure what feature test(s) would be necessary to make Git able to automatically set that, seeing as how the tests defined in 8e97 are perfectly reasonable and didn't pass.

Yea, yea, shame on me for not testing Git since Nov 20th on this system... I should have caught the failure sooner. I'm buried in non-Git work these days, but will try to come up with a reasonable detection patch. Maybe someone will beat me to it. :-)

-- 
Shawn.

Junio C Hamano· Dec 18, 2007, 01:08 UTC · re: Shawn O. Pearce · lore

Re: FLEX_ARRAY=1 causes SIGSEGV on SPARC

"Shawn O. Pearce" <spearce@spearce.org> writes:

Show 8 quoted lines

> I'll try to track it down tomorrow.  But the immediate workaround was
> to just add '-DFLEX_ARRAY=/* empty */' to my CFLAGS and recompile
> the world.  This compiler accepts the empty FLEX_ARRAY macro but
> I'm not sure what feature test(s) would be necessary to make Git
> able to automatically set that, seeing as how the tests defined in
> 8e97 are perfectly reasonable and didn't pass.
> ..., but will try to come up with a reasonable
> detection patch....

Actually I would be more worried about the breakage in FLEX_ARRAY=1 case than misdetection. Even if your compiler supports the flexible array members, the fallback to FLEX_ARRAY=1 ought to work and you are seeing a case where it doesn't.

David Kastrup· Dec 18, 2007, 01:32 UTC · re: Junio C Hamano · lore

[PATCH] diff-delta.c: make FLEX_ARRAY=1 work.

I remarked previously that diff-delta.c does not work with FLEX_ARRAY=1. Here is one attempt to change this. It conceivably still suffers from potential misalignment problems (which would likely need some union type magic to avoid 100%), but at least the index calculation should not go as horribly wrong as previously.

--- Junio C Hamano <gitster@pobox.com> writes:

Show 15 quoted lines

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> I'll try to track it down tomorrow.  But the immediate workaround was
>> to just add '-DFLEX_ARRAY=/* empty */' to my CFLAGS and recompile
>> the world.  This compiler accepts the empty FLEX_ARRAY macro but
>> I'm not sure what feature test(s) would be necessary to make Git
>> able to automatically set that, seeing as how the tests defined in
>> 8e97 are perfectly reasonable and didn't pass.
>> ..., but will try to come up with a reasonable
>> detection patch....
>
> Actually I would be more worried about the breakage in FLEX_ARRAY=1 case
> than misdetection.  Even if your compiler supports the flexible array
> members, the fallback to FLEX_ARRAY=1 ought to work and you are seeing a
> case where it doesn't.

 diff-delta.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff-delta.c b/diff-delta.c
index 9e440a9..099235c 100644
--- a/diff-delta.c
+++ b/diff-delta.c
@@ -247,7 +247,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
 	/* Now create the packed index in array form rather than
 	 * linked lists */
 
-	memsize = sizeof(*index)
+	memsize = (char *)&index->hash - (char *)index
 		+ sizeof(*packed_hash) * (hsize+1)
 		+ sizeof(*packed_entry) * entries;
 
@@ -264,7 +264,7 @@ struct delta_index * create_delta_index(const void *buf, unsigned long bufsize)
 	index->src_size = bufsize;
 	index->hash_mask = hmask;
 
-	mem = index + 1;
+	mem = &index->hash;
 	packed_hash = mem;
 	mem = packed_hash + (hsize+1);
 	packed_entry = mem;

-- 
1.5.3.6.995.ge8abd