Possible ARM struct stat problem.

Possible ARM struct stat problem.

[Richard Pennington]

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

Hi,

I think a found a problem with the struct stat definition for the ARM. It 
needed a bit more padding. I've attached a diff and wrote about it on my blog 
if you'd like more information.

http://ellcc.org/blog/?p=121

-Rich

[-- Attachment #2: stat.diff --]
[-- Type: text/x-patch, Size: 570 bytes --]

Index: stat.h
===================================================================
--- stat.h	(revision 2928)
+++ stat.h	(working copy)
@@ -11,12 +11,19 @@
 	uid_t st_uid;
 	gid_t st_gid;
 	dev_t st_rdev;
-	int __st_rdev_padding;
+	int __st_rdev_padding[2];
 	off_t st_size;
 	blksize_t st_blksize;
+	int __st_rdev_padding2[1];
 	blkcnt_t st_blocks;
 	struct timespec st_atim;
 	struct timespec st_mtim;
 	struct timespec st_ctim;
 	ino_t st_ino;
 };
+
+#ifdef _BSD_SOURCE
+#define st_atimespec st_atim
+#define st_mtimespec st_mtim
+#define st_ctimespec st_ctim
+#endif

Re: Possible ARM struct stat problem.

[Rich Felker]

On Sun, May 27, 2012 at 01:35:07PM -0500, Richard Pennington wrote:
> Hi,
> 
> I think a found a problem with the struct stat definition for the ARM. It 
> needed a bit more padding. I've attached a diff and wrote about it on my blog 
> if you'd like more information.
> 
> http://ellcc.org/blog/?p=121

This looks like a clang bug. Per the ABI, long long has 8-byte
alignment on ARM. Thus, the structure definition musl has right now
will give the right layout to match the ABI, and in fact it works
perfectly when compiled with GCC. 

I'm not entirely opposed to putting the explicit padding in there,
since this is an arch-specific structure anyway, but I think you
should check your compiler. The same issue might come up elsewhere and
might not be so easy to work around.

Let me know what you find.

Rich

Re: Possible ARM struct stat problem.

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2012-05-27 15:43:19 -0400]:
> On Sun, May 27, 2012 at 01:35:07PM -0500, Richard Pennington wrote:
> > Hi,
> > 
> > I think a found a problem with the struct stat definition for the ARM. It 
> > needed a bit more padding. I've attached a diff and wrote about it on my blog 
> > if you'd like more information.
> > 
> > http://ellcc.org/blog/?p=121
> 
> This looks like a clang bug. Per the ABI, long long has 8-byte
> alignment on ARM. Thus, the structure definition musl has right now
> will give the right layout to match the ABI, and in fact it works
> perfectly when compiled with GCC. 
> 
> I'm not entirely opposed to putting the explicit padding in there,
> since this is an arch-specific structure anyway, but I think you
> should check your compiler. The same issue might come up elsewhere and
> might not be so easy to work around.
> 
> Let me know what you find.

hm i have an /usr/include/asm-generic/stat.h (kernel header)
and it has a struct stat and struct stat64 and they seem
to have different padding than the musl definition

Re: Possible ARM struct stat problem.

[Richard Pennington]

On Sunday, May 27, 2012 03:43:19 PM Rich Felker wrote:
> On Sun, May 27, 2012 at 01:35:07PM -0500, Richard Pennington wrote:
> > Hi,
> > 
> > I think a found a problem with the struct stat definition for the ARM. It
> > needed a bit more padding. I've attached a diff and wrote about it on my
> > blog if you'd like more information.
> > 
> > http://ellcc.org/blog/?p=121
> 
> This looks like a clang bug. Per the ABI, long long has 8-byte
> alignment on ARM. Thus, the structure definition musl has right now
> will give the right layout to match the ABI, and in fact it works
> perfectly when compiled with GCC.
> 
> I'm not entirely opposed to putting the explicit padding in there,
> since this is an arch-specific structure anyway, but I think you
> should check your compiler. The same issue might come up elsewhere and
> might not be so easy to work around.
> 
> Let me know what you find.
> 
> Rich

You were right, there is a problem with my compiler's ABI handling. I've 
patched it and have removed the padding. Thanks.

-Rich

Re: Possible ARM struct stat problem.

[John Spencer]

On 05/27/2012 09:43 PM, Rich Felker wrote:
> On Sun, May 27, 2012 at 01:35:07PM -0500, Richard Pennington wrote:
>> Hi,
>>
>> I think a found a problem with the struct stat definition for the ARM. It
>> needed a bit more padding. I've attached a diff and wrote about it on my blog
>> if you'd like more information.
>>
>> http://ellcc.org/blog/?p=121
> This looks like a clang bug. Per the ABI, long long has 8-byte
> alignment on ARM. Thus, the structure definition musl has right now
> will give the right layout to match the ABI, and in fact it works
> perfectly when compiled with GCC.

i have the same issue with gcc 4.2.4. hunting the bug costed me hours.

> I'm not entirely opposed to putting the explicit padding in there,
> since this is an arch-specific structure anyway, but I think you
> should check your compiler. The same issue might come up elsewhere and
> might not be so easy to work around.
>
please apply the explicit padding.

Re: Possible ARM struct stat problem.

[Rich Felker]

On Tue, Jun 26, 2012 at 04:15:25AM +0200, John Spencer wrote:
> >I'm not entirely opposed to putting the explicit padding in there,
> >since this is an arch-specific structure anyway, but I think you
> >should check your compiler. The same issue might come up elsewhere and
> >might not be so easy to work around.
> >
> please apply the explicit padding.

All this would have done is hide the issue that you're using the wrong
ABI (oabi instead of eabi) and make it harder to find the more-subtle
resulting bugs later (mildly different calling convention and
padding).

Rich

Re: Possible ARM struct stat problem.

[Richard Pennington]

On Monday, June 25, 2012 10:50:02 PM Rich Felker wrote:
> On Tue, Jun 26, 2012 at 04:15:25AM +0200, John Spencer wrote:
> > >I'm not entirely opposed to putting the explicit padding in there,
> > >since this is an arch-specific structure anyway, but I think you
> > >should check your compiler. The same issue might come up elsewhere and
> > >might not be so easy to work around.
> > 
> > please apply the explicit padding.
> 
> All this would have done is hide the issue that you're using the wrong
> ABI (oabi instead of eabi) and make it harder to find the more-subtle
> resulting bugs later (mildly different calling convention and
> padding).
> 
> Rich
I agree with Rich on this one.

-Rich

Re: Possible ARM struct stat problem.

[Rich Felker]

On Mon, Jun 25, 2012 at 09:52:02PM -0500, Richard Pennington wrote:
> On Monday, June 25, 2012 10:50:02 PM Rich Felker wrote:
> > On Tue, Jun 26, 2012 at 04:15:25AM +0200, John Spencer wrote:
> > > >I'm not entirely opposed to putting the explicit padding in there,
> > > >since this is an arch-specific structure anyway, but I think you
> > > >should check your compiler. The same issue might come up elsewhere and
> > > >might not be so easy to work around.
> > > 
> > > please apply the explicit padding.
> > 
> > All this would have done is hide the issue that you're using the wrong
> > ABI (oabi instead of eabi) and make it harder to find the more-subtle
> > resulting bugs later (mildly different calling convention and
> > padding).
> > 
> I agree with Rich on this one.

I'm wondering if there's somewhere in the source I could put a static
assertion to test for wrong ABI. Perhaps this belongs in the configure
script (but I know *ahem* some folks don't run configure).
Unfortunately the only C code that's arch-specific is in header files,
and none of them really seem like the appropriate place for static
assertions, so I'm not sure where to put it.

Rich

Re: Possible ARM struct stat problem.

[John Spencer]

On 06/26/2012 04:50 AM, Rich Felker wrote:
> On Tue, Jun 26, 2012 at 04:15:25AM +0200, John Spencer wrote:
>>> I'm not entirely opposed to putting the explicit padding in there,
>>> since this is an arch-specific structure anyway, but I think you
>>> should check your compiler. The same issue might come up elsewhere and
>>> might not be so easy to work around.
>>>
>> please apply the explicit padding.
> All this would have done is hide the issue that you're using the wrong
> ABI (oabi instead of eabi) and make it harder to find the more-subtle
> resulting bugs later (mildly different calling convention and
> padding).
>
> Rich
>
indeed.
i'm trying to fix the toolchain by passing an explicit 
--target=arm-linux-gnueabi to configure.
since ARM toolchains appear to full of such hidden traps, i vote +1 for 
adding some assertions that the right ABI is used.