linux-kernel - Re: [Suggest] arch: metag: compiler: Are they compiler's issues?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 10 Jan 2014 16:02:42 +0000
From:	James Hogan <james.hogan@...tec.com>
To:	Chen Gang <gang.chen.5i5j@...il.com>
CC:	<linux-metag@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [Suggest] arch: metag: compiler: Are they compiler's issues?

On 10/01/14 15:57, Chen Gang wrote:
> On 01/08/2014 11:01 PM, Chen Gang wrote:
>> On 01/06/2014 06:31 PM, James Hogan wrote:
>>> I suspect this is due to bad assumptions in the code. The metag ABI is
>>> unusual in padding the size of structs to a 32bit boundary even if all
>>> members are <32bit. This is actually permitted by the C standard but
>>> it's a bit of a pain. e.g.
>>>
>>> struct s {
>>> 	short x
>>> 	struct {
>>> 		short x[3];
>>> 	} y;
>>> 	short z;
>>> };
>>>
>>> on x86
>>> 	alignof(s::y) == 2
>>> 	s::y at offset 2
>>> 	sizeof(s::y) == 6
>>> 	s::z at offset 6+2 = 8
>>> 	sizeof(struct s) == 10
>>>
>>> but on metag
>>> 	alignof(s::y) == 4
>>> 	s::y at offset 4
>>> 	sizeof(s::y) == 8 (padding, this is what catches people out)
>>> 	s::z at offset 4+8 = 12
>>> 	sizeof(struct s) == 16 (and here too)
>>>
>>> Adding packed attribute on outer struct reduces sizeof(struct s) to 12
>>> on metag:
>>> 	alignof(s::y) == 4
>>> 	s::y at offset 2 (packed)
>>> 	sizeof(s::y) == 8 (still padded)
>>
>> In my memory, when packed(2), it breaks the C standard (although I am
>> not quit sure).
>>
>> And I guess, all C programmers will assume it will be 6 when within
>> pack(2) or pack(1).
>>
>>> 	s::z at offset 2+8 = 10
>>> 	sizeof(struct s) == 12 (packed)
>>>
>>> Also reduced to 12 if only inner struct is marked packed:
>>> 	alignof(s::y) == 2
>>> 	s::y at offset 2
>>> 	sizeof(s::y) == 6 (packed)
>>> 	s::z at offset 2+6 = 8
>>> 	sizeof(struct s) == 12 (still padded)
>>>
>>> Adding packed attribute on both outer and inner struct reduces
>>> sizeof(struct s) to 10 to match x86.
>>>
>>> Unfortunately it's years too late to change this ABI, so we're stuck
>>> with it.
>>>
>>
>> Unfortunately too, most using cases are related with API (the related
>> structure definition must be the same in binary data).
>>
>> I am sure there are still another ways to bypass this issue, but that
>> will make the code looks very strange (especially they are API).
>>
>> :-(
>>
> 
> I guess most C programmers will use this way to describe protocol/data
> format, and keep compatible for it (since it is API).
> 
> So even if it really does not break C standard, I still recommend our
> compiler to improve itself to support this features.

The compiler cannot change this without breaking the ABI.

If the structure describes a set-in-stone data layout (which it sounds
like it does since it asserts the size of it) then the correct fix is to
pack the structures in such a way as to guarantee the correct offsets
and sizes on all compliant compilers. Otherwise if it's just an internal
programming API it shouldn't be using compile time asserts to enforce
things that vary between ABIs.

Cheers
James

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/