Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #77000 > unrolled thread

Aliasing through a union pointer

Back to article view | Back to comp.lang.c

Contents

  Aliasing through a union pointer Ian Pilcher <arequipeno@gmail.com> - 2015-11-23 10:14 -0600
    Re: Aliasing through a union pointer Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-11-23 11:43 -0500
      Re: Aliasing through a union pointer Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-11-23 11:49 -0500
      Re: Aliasing through a union pointer Ian Pilcher <arequipeno@gmail.com> - 2015-11-23 13:22 -0600
        Re: Aliasing through a union pointer Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-11-23 14:38 -0500
          Re: Aliasing through a union pointer Tim Rentsch <txr@alumni.caltech.edu> - 2015-11-25 10:38 -0800
            Re: Aliasing through a union pointer Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-11-25 14:17 -0500
              Re: Aliasing through a union pointer Tim Rentsch <txr@alumni.caltech.edu> - 2015-11-27 13:16 -0800
        Re: Aliasing through a union pointer David Brown <david.brown@hesbynett.no> - 2015-11-23 21:46 +0100
          Re: Aliasing through a union pointer Jorgen Grahn <grahn+nntp@snipabacken.se> - 2015-11-23 22:20 +0000
            Re: Aliasing through a union pointer Stephen Sprunk <stephen@sprunk.org> - 2015-11-23 16:52 -0600
              Re: Aliasing through a union pointer Jorgen Grahn <grahn+nntp@snipabacken.se> - 2015-11-23 23:10 +0000
                Re: Aliasing through a union pointer Stephen Sprunk <stephen@sprunk.org> - 2015-11-23 19:05 -0600
            Re: Aliasing through a union pointer Nick Bowler <nbowler@draconx.ca> - 2015-11-23 22:51 +0000
            Re: Aliasing through a union pointer Keith Thompson <kst-u@mib.org> - 2015-11-23 15:49 -0800
              Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-23 16:01 -0800
            Re: Aliasing through a union pointer David Brown <david.brown@hesbynett.no> - 2015-11-24 09:55 +0100
              Re: Aliasing through a union pointer Jorgen Grahn <grahn+nntp@snipabacken.se> - 2015-11-24 14:29 +0000
                Re: Aliasing through a union pointer David Brown <david.brown@hesbynett.no> - 2015-11-24 22:58 +0100
                  Re: Aliasing through a union pointer Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-11-24 17:30 -0500
                    Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-24 15:27 -0800
                    Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-25 12:57 +1300
                      Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-24 16:31 -0800
                        Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-25 18:28 +1300
                          Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-25 06:54 -0800
                            Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-26 08:59 +1300
                              Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-25 13:01 -0800
                                Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-26 12:58 +1300
                                  Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-25 16:44 -0800
                                    Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-26 14:12 +1300
                                      Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-25 21:34 -0800
                                        Re: Aliasing through a union pointer Malcolm McLean <malcolm.mclean5@btinternet.com> - 2015-11-25 21:55 -0800
                                        Re: Aliasing through a union pointer Ian Collins <ian-news@hotmail.com> - 2015-11-26 20:51 +1300
                                          Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-26 11:39 -0800
                    Re: Aliasing through a union pointer Jorgen Grahn <grahn+nntp@snipabacken.se> - 2015-11-26 00:06 +0000
              Re: Aliasing through a union pointer Keith Thompson <kst-u@mib.org> - 2015-11-24 11:51 -0800
                Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-24 12:20 -0800
                  Re: Aliasing through a union pointer Keith Thompson <kst-u@mib.org> - 2015-11-24 21:52 -0800
                    Re: Aliasing through a union pointer supercat@casperkitty.com - 2015-11-25 08:30 -0800
                Re: Aliasing through a union pointer David Brown <david.brown@hesbynett.no> - 2015-11-24 23:04 +0100
    Re: Aliasing through a union pointer Tim Rentsch <txr@alumni.caltech.edu> - 2015-11-25 10:10 -0800
      Re: Aliasing through a union pointer Richard Damon <Richard@Damon-Family.org> - 2015-11-25 11:02 -0800
        Re: Aliasing through a union pointer Philip Lantz <prl@canterey.us> - 2015-11-26 13:06 -0800
          Re: Aliasing through a union pointer Richard Damon <Richard@Damon-Family.org> - 2015-11-26 13:09 -0800
        Re: Aliasing through a union pointer Tim Rentsch <txr@alumni.caltech.edu> - 2015-11-27 13:08 -0800

Page 1 of 3  [1] 2 3  Next page →

#77000 — Aliasing through a union pointer

I am working with a "header" struct, all of whose members are
uint64_t's.  I would like to treat the struct as an array of
uint64_t when calculating a checksum, byte swapping for endian
compatibility, etc.

Is casting a pointer to the struct to a pointer to union a safe
way to do this (from an aliasing point of view)?  Is there a
better way to do it?

For example:

struct header {
     uint64_t magic;
     uint64_t version;
     uint64_t size;
     uint64_t uuid_lo;
     uint64_t uuid_hi;
     uint64_t type;
     uint64_t offset;
     uint64_t size;
     uint64_t cksum;
};

#define HEADER_ARRAY_SIZE  (sizeof(struct header) / sizeof(uint64_t))

union header_u {
     struct header h;
     uint64_t      u[HEADER_ARRAY_SIZE];
};

/* Broken if the struct has any padding */
uint64_t naive_cksum(struct header *const hdr)
{
     union header_u *const hu = (union header_u *)hdr;

     uint64_t save, sum;
     unsigned i;


     save = hu->h.cksum;
     hu->h.cksum = 0;

     for (sum = 0, i = 0; i < HEADER_ARRAY_SIZE; ++i)
         sum += hu->u[i];

     hu->h.cksum = save;

     return sum;
}

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
-------- "I grew up before Mark Zuckerberg invented friendship" --------
========================================================================

[toc] | [next] | [standalone]

#77005

On 11/23/2015 11:14 AM, Ian Pilcher wrote:
> I am working with a "header" struct, all of whose members are
> uint64_t's.  I would like to treat the struct as an array of
> uint64_t when calculating a checksum, byte swapping for endian
> compatibility, etc.
>
> Is casting a pointer to the struct to a pointer to union a safe
> way to do this (from an aliasing point of view)?  Is there a
> better way to do it?
>
> For example:
>
> struct header {
>      uint64_t magic;
>      uint64_t version;
>      uint64_t size;
>      uint64_t uuid_lo;
>      uint64_t uuid_hi;
>      uint64_t type;
>      uint64_t offset;
>      uint64_t size;
>      uint64_t cksum;
> };
>
> #define HEADER_ARRAY_SIZE  (sizeof(struct header) / sizeof(uint64_t))
>
> union header_u {
>      struct header h;
>      uint64_t      u[HEADER_ARRAY_SIZE];
> };
>
> /* Broken if the struct has any padding */
> uint64_t naive_cksum(struct header *const hdr)
> {
>      union header_u *const hu = (union header_u *)hdr;
>
>      uint64_t save, sum;
>      unsigned i;
>
>
>      save = hu->h.cksum;
>      hu->h.cksum = 0;
>
>      for (sum = 0, i = 0; i < HEADER_ARRAY_SIZE; ++i)
>          sum += hu->u[i];
>
>      hu->h.cksum = save;
>
>      return sum;
> }

     The first difficulty that occurs to me is one that's already
occurred to you: You're screwed if the struct has any padding,
either interstitial or at the end.  That seems fairly unlikely,
but is permitted by the language.  As a "Just In Case" measure,
you might try asserting that

	offsetof(struct header, cksum) + sizeof(uint64_t)
	    == sizeof(struct header)

... which would detect the presence of padding.

     The next thing I think of is alignment: What if the alignment
of the union type is stricter than that of the struct?  If anything
this is even less likely than the presence of padding, but I still
believe it's permitted.  If it happens, then the pointer conversion
at the start of the naive_cksum() function could go awry.

     A more serious matter -- an Actual Problem, I believe -- is
the modification (twice!) of an object referenced through a const-
qualified pointer.  (Didn't your compiler complain?)  You'd better
find another way to leave the stored checksum out of the computation.
In naive_cksum() you could just initialize `sum' to `-hu->h.cksum'
instead of to zero, but you'd need to be cagier with CRC's or other
less easily-inverted algorithms (where supplying a zero `cksum' might
not be appropriate, either).

     ... or you could assume that `cksum' is always the final element,
and stop the loop one iteration sooner.  Or you could use offsetof
to determine which array index corresponds to `cksum', and skip that
one.  Or you could ditch `const'.

     It seems to me that once you've ensured there's no padding --
which you ought to do in any case -- the union doesn't buy you much:
Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
virtue of avoiding the (highly unlikely) alignment problem, because
the struct alignment is at least good enough for a uint64_t.

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#77007

On 11/23/2015 11:43 AM, Eric Sosman wrote:
>[...]
>      A more serious matter -- an Actual Problem, I believe -- is
> the modification (twice!) of an object referenced through a const-
> qualified pointer. [...]

     An even Bigger Actual Problem is the dope slap I'm about
to administer to myself for reading too hastily.  "Sorry about
that, Chief."

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#77022

On 11/23/2015 10:43 AM, Eric Sosman wrote:
>      The first difficulty that occurs to me is one that's already
> occurred to you: You're screwed if the struct has any padding,
> either interstitial or at the end.  That seems fairly unlikely,
> but is permitted by the language.  As a "Just In Case" measure,
> you might try asserting that
>
>      offsetof(struct header, cksum) + sizeof(uint64_t)
>          == sizeof(struct header)
>
> ... which would detect the presence of padding.

Ah nice.  I'm planning using gcc's __attribute__((packed)), so this
shouldn't be an issue, but I like the idea of the assert, or better
yet static_assert.

>      It seems to me that once you've ensured there's no padding --
> which you ought to do in any case -- the union doesn't buy you much:
> Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
> virtue of avoiding the (highly unlikely) alignment problem, because
> the struct alignment is at least good enough for a uint64_t.

For some reason, I thought that I wasn't allowed to do that.  I have
vague memories of gcc warnings about type punning and strict aliasing.

I definitely don't get any such warnings when doing that, however, so
it looks like a case of over-thinking.  (Root cause being that I'm
still not confident that I grasp the aliasing rules.)

On 11/23/2015 10:49 AM, Eric Sosman wrote:
 >      An even Bigger Actual Problem is the dope slap I'm about
 > to administer to myself for reading too hastily.  "Sorry about
 > that, Chief."

At least you caught it on your own.  Always more dignified that way.

Thanks for the help!

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
-------- "I grew up before Mark Zuckerberg invented friendship" --------
========================================================================

[toc] | [prev] | [next] | [standalone]

#77024

On 11/23/2015 2:22 PM, Ian Pilcher wrote:
> On 11/23/2015 10:43 AM, Eric Sosman wrote:
>> [...]
>>      It seems to me that once you've ensured there's no padding --
>> which you ought to do in any case -- the union doesn't buy you much:
>> Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
>> virtue of avoiding the (highly unlikely) alignment problem, because
>> the struct alignment is at least good enough for a uint64_t.
>
> For some reason, I thought that I wasn't allowed to do that.  I have
> vague memories of gcc warnings about type punning and strict aliasing.

     You're "lying to the compiler" whether you transform the
struct pointer to a union pointer or to a uint64_t pointer, and
I see nothing about the first lie that would make it any less
sinful than the second.  So, I'd suggest lying the simpler lie.

	"Oh! What a tangled web we weave
	When first we practise to deceive!"
	-- Sir Walter Scott

> On 11/23/2015 10:49 AM, Eric Sosman wrote:
>  >      An even Bigger Actual Problem is the dope slap I'm about
>  > to administer to myself for reading too hastily.  "Sorry about
>  > that, Chief."
>
> At least you caught it on your own.  Always more dignified that way.

     "More dignified" seems an overstatement, but I'll settle
for "less undignified."  Sigh.

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#77177

Eric Sosman <esosman@comcast-dot-net.invalid> writes:

> On 11/23/2015 2:22 PM, Ian Pilcher wrote:
>> On 11/23/2015 10:43 AM, Eric Sosman wrote:
>>> [...]
>>>      It seems to me that once you've ensured there's no padding --
>>> which you ought to do in any case -- the union doesn't buy you much:
>>> Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
>>> virtue of avoiding the (highly unlikely) alignment problem, because
>>> the struct alignment is at least good enough for a uint64_t.
>>
>> For some reason, I thought that I wasn't allowed to do that.  I have
>> vague memories of gcc warnings about type punning and strict aliasing.
>
>     You're "lying to the compiler" whether you transform the
> struct pointer to a union pointer or to a uint64_t pointer, and
> I see nothing about the first lie that would make it any less
> sinful than the second.  [...]

That's true in the first case, but not the second:  the conversion
is guaranteed to work under 6.7.2.1 p15.

Next the question arises as to whether that pointer may be used
to index all the elements.  This gets us into a murky area.  For
example, if the struct is in (and starts at) memory allocated by
malloc(), then we are allowed to use a pointer to the start of
the malloc()'ed block to index an array of any kind of object.
Since all the accesses use type uint64_t, there is no violation
of effective type rules.  I don't know how to get a clearcut
answer to this question;  certainly not just by reading the
Standard.

In any case I agree with your recommendation to convert directly
to the uint64_t pointer type, and avoid using a pointer-to-union
type.

[toc] | [prev] | [next] | [standalone]

#77181

On 11/25/2015 1:38 PM, Tim Rentsch wrote:
> Eric Sosman <esosman@comcast-dot-net.invalid> writes:
>
>> On 11/23/2015 2:22 PM, Ian Pilcher wrote:
>>> On 11/23/2015 10:43 AM, Eric Sosman wrote:
>>>> [...]
>>>>       It seems to me that once you've ensured there's no padding --
>>>> which you ought to do in any case -- the union doesn't buy you much:
>>>> Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
>>>> virtue of avoiding the (highly unlikely) alignment problem, because
>>>> the struct alignment is at least good enough for a uint64_t.
>>>
>>> For some reason, I thought that I wasn't allowed to do that.  I have
>>> vague memories of gcc warnings about type punning and strict aliasing.
>>
>>      You're "lying to the compiler" whether you transform the
>> struct pointer to a union pointer or to a uint64_t pointer, and
>> I see nothing about the first lie that would make it any less
>> sinful than the second.  [...]
>
> That's true in the first case, but not the second:  the conversion
> is guaranteed to work under 6.7.2.1 p15.

     I think we're referring to different lies -- or, I'm applying
the single word "lie" to two different abuses:

     - Converting the struct* to a union* might not work at all, so
       applying the cast -- telling the compiler "Perform this valid
       conversion" -- is lying to the compiler.

     - Converting the struct* to a uint64_t* will work, no question
       (for any struct whose first element is a uint64_t).  The "lie"
       in this case is using the converted pointer to access not only
       the first element (perfectly all right), but to access later
       elements as if the struct were an array of uint64_t.

    So: I'll 'fess up to loose use of "lie" to cover different abuses,
but I'm allowed that liberty under the terms of my artistic lie-cense.

> Next the question arises as to whether that pointer may be used
> to index all the elements.  This gets us into a murky area.  For
> example, if the struct is in (and starts at) memory allocated by
> malloc(), then we are allowed to use a pointer to the start of
> the malloc()'ed block to index an array of any kind of object.
> Since all the accesses use type uint64_t, there is no violation
> of effective type rules.  I don't know how to get a clearcut
> answer to this question;  certainly not just by reading the
> Standard.

     The Standard will tell us it's OK to manipulate an array of
uint64_t within the memory allocated for the struct, so long as we
don't step outside the allocation.  But the Standard will also tell
us we might be manipulating nameless padding bytes that do not
belong to any of the struct's elements, and that's where the
trouble arises.

> In any case I agree with your recommendation to convert directly
> to the uint64_t pointer type, and avoid using a pointer-to-union
> type.

     Right.  The union gains nothing, and could even lose a smidgen.
Cut to the chase, I say.

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#77275

Eric Sosman <esosman@comcast-dot-net.invalid> writes:

> On 11/25/2015 1:38 PM, Tim Rentsch wrote:
>> Eric Sosman <esosman@comcast-dot-net.invalid> writes:
>>
>>> On 11/23/2015 2:22 PM, Ian Pilcher wrote:
>>>> On 11/23/2015 10:43 AM, Eric Sosman wrote:
>>>>> [...]
>>>>>       It seems to me that once you've ensured there's no padding --
>>>>> which you ought to do in any case -- the union doesn't buy you much:
>>>>> Just cast `hdr' to a `uint64_t*' and blaze away.  This also has the
>>>>> virtue of avoiding the (highly unlikely) alignment problem, because
>>>>> the struct alignment is at least good enough for a uint64_t.
>>>>
>>>> For some reason, I thought that I wasn't allowed to do that.  I have
>>>> vague memories of gcc warnings about type punning and strict aliasing.
>>>
>>>      You're "lying to the compiler" whether you transform the
>>> struct pointer to a union pointer or to a uint64_t pointer, and
>>> I see nothing about the first lie that would make it any less
>>> sinful than the second.  [...]
>>
>> That's true in the first case, but not the second:  the conversion
>> is guaranteed to work under 6.7.2.1 p15.
>
>     I think we're referring to different lies -- or, I'm applying
> the single word "lie" to two different abuses:
>
>     - Converting the struct* to a union* might not work at all, so
>       applying the cast -- telling the compiler "Perform this valid
>       conversion" -- is lying to the compiler.
>
>     - Converting the struct* to a uint64_t* will work, no question
>       (for any struct whose first element is a uint64_t).  The "lie"
>       in this case is using the converted pointer to access not only
>       the first element (perfectly all right), but to access later
>       elements as if the struct were an array of uint64_t.
>
>    So:  I'll 'fess up to loose use of "lie" to cover different abuses,
> but I'm allowed that liberty under the terms of my artistic lie-cense.

I agree that using the pointer to access an array is, or at least
might be, a lie.  My point is that it is not a lie to the compiler.

>> Next the question arises as to whether that pointer may be used
>> to index all the elements.  This gets us into a murky area.  For
>> example, if the struct is in (and starts at) memory allocated by
>> malloc(), then we are allowed to use a pointer to the start of
>> the malloc()'ed block to index an array of any kind of object.
>> Since all the accesses use type uint64_t, there is no violation
>> of effective type rules.  I don't know how to get a clearcut
>> answer to this question;  certainly not just by reading the
>> Standard.
>
>     The Standard will tell us it's OK to manipulate an array of
> uint64_t within the memory allocated for the struct, so long as we
> don't step outside the allocation.  But the Standard will also
> tell us we might be manipulating nameless padding bytes that do
> not belong to any of the struct's elements, and that's where the
> trouble arises.

Right.  I'm assuming there will be a check to make sure that
situation doesn't occur;  I didn't bring it up because I thought
it was obvious.

[toc] | [prev] | [next] | [standalone]

#77026

On 23/11/15 20:22, Ian Pilcher wrote:
> On 11/23/2015 10:43 AM, Eric Sosman wrote:
>>      The first difficulty that occurs to me is one that's already
>> occurred to you: You're screwed if the struct has any padding,
>> either interstitial or at the end.  That seems fairly unlikely,
>> but is permitted by the language.  As a "Just In Case" measure,
>> you might try asserting that
>>
>>      offsetof(struct header, cksum) + sizeof(uint64_t)
>>          == sizeof(struct header)
>>
>> ... which would detect the presence of padding.
>
> Ah nice.  I'm planning using gcc's __attribute__((packed)), so this
> shouldn't be an issue, but I like the idea of the assert, or better
> yet static_assert.
>

Be generous with your static assertions.  They cost nothing in code size 
or run-time, and protect against silly mistakes or incorrect 
assumptions, as well as unexpected compiler effects.  (If you don't have 
C11, there are ways to make a static_assert macro.)

I'd advise against __attribute__((packed)) (or equivalents on other 
compilers) - either it makes no difference because there is no padding, 
or you will end up with structures with fields that don't match their 
alignment.  On some targets, that means run-time errors - on others, it 
means seriously degraded code speed.  So it is rarely a good idea.

Personally, I often put the padding spaces in explicitly in my structs - 
especially if the exact layout is important.  It also makes it easier to 
manually count offsets if you want to do that.  And gcc's "-Wpadded" 
flag will warn you if you've got it wrong - as will your static assertions.

[toc] | [prev] | [next] | [standalone]

#77033

On Mon, 2015-11-23, David Brown wrote:
...
> I'd advise against __attribute__((packed)) (or equivalents on other 
> compilers) - either it makes no difference because there is no padding, 
> or you will end up with structures with fields that don't match their 
> alignment.  On some targets, that means run-time errors - on others, it 
> means seriously degraded code speed.  So it is rarely a good idea.

I think I agree, but are there really cases where you say
__attribute__((packed)) and then suffer a crash because of it?  The
compiler shouldn't generate a struct, and code to access it which will
crash if it's ever run.

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [next] | [standalone]

#77038

On 23-Nov-15 16:20, Jorgen Grahn wrote:
> On Mon, 2015-11-23, David Brown wrote: ....
>> I'd advise against __attribute__((packed)) (or equivalents on other
>>  compilers) - either it makes no difference because there is no
>> padding, or you will end up with structures with fields that don't
>> match their alignment.  On some targets, that means run-time errors
>> - on others, it means seriously degraded code speed.  So it is
>> rarely a good idea.
> 
> I think I agree, but are there really cases where you say 
> __attribute__((packed)) and then suffer a crash because of it?  The 
> compiler shouldn't generate a struct, and code to access it which
> will crash if it's ever run.

As long as you access a misaligned member via a packed struct that is
itself properly aligned, then GCC will generate the necessary code to
safely do so.

If the struct itself is misaligned, or if you dereference a pointer to a
misaligned member, then all bets are off.

S

-- 
Stephen Sprunk         "God does not play dice."  --Albert Einstein
CCIE #3723         "God is an inveterate gambler, and He throws the
K5SSS        dice at every possible opportunity." --Stephen Hawking

[toc] | [prev] | [next] | [standalone]

#77042

On Mon, 2015-11-23, Stephen Sprunk wrote:
> On 23-Nov-15 16:20, Jorgen Grahn wrote:
>> On Mon, 2015-11-23, David Brown wrote: ....
>>> I'd advise against __attribute__((packed)) (or equivalents on other
>>>  compilers) - either it makes no difference because there is no
>>> padding, or you will end up with structures with fields that don't
>>> match their alignment.  On some targets, that means run-time errors
>>> - on others, it means seriously degraded code speed.  So it is
>>> rarely a good idea.
>> 
>> I think I agree, but are there really cases where you say 
>> __attribute__((packed)) and then suffer a crash because of it?  The 
>> compiler shouldn't generate a struct, and code to access it which
>> will crash if it's ever run.
>
> As long as you access a misaligned member via a packed struct that is
> itself properly aligned, then GCC will generate the necessary code to
> safely do so.
>
> If the struct itself is misaligned,

But that's broken code even if you don't use 'packed', right?
(Although you tend to use 'packed' when you want to pretend a char
array is a struct Foo, so the two tend to go hand in hand.)

> or if you dereference a pointer to a
> misaligned member, then all bets are off.

So saying &foo.bar suddenly becomes a bug waiting to happen, because
foo is a packed struct?  Yes, I can see how that would be unavoidable.
But surely a compiler which provides 'packed' would emit a warning for
that code?  In a sense foo.bar has no more of an address than a
bitfield has.

If it doesn't, then I agree about staying away from packed structs
(which I do anyway, but that strengthens my argument a lot).

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [next] | [standalone]

#77051

On 23-Nov-15 17:10, Jorgen Grahn wrote:
> On Mon, 2015-11-23, Stephen Sprunk wrote:
>> On 23-Nov-15 16:20, Jorgen Grahn wrote:
>>> I think I agree, but are there really cases where you say 
>>> __attribute__((packed)) and then suffer a crash because of it?
>>> The compiler shouldn't generate a struct, and code to access it
>>> which will crash if it's ever run.
>> 
>> As long as you access a misaligned member via a packed struct that
>> is itself properly aligned, then GCC will generate the necessary
>> code to safely do so.
>> 
>> If the struct itself is misaligned,
> 
> But that's broken code even if you don't use 'packed', right?

Right, but it's worth mentioning anyway.  GCC will only generate fixup
code for members it _knows_ are misaligned due to packing, and only for
the degree of misalignment that it expects.

For instance, a 4-byte member expected to be 2-byte misaligned may not
work if actually 1- or 3-byte misaligned--or vice versa.

> (Although you tend to use 'packed' when you want to pretend a char 
> array is a struct Foo, so the two tend to go hand in hand.)

In most cases where I've found this useful, the buffer was known to be
properly aligned for the struct, so I could just cast a pointer and go.
 In general, one must memcpy() from the buffer into a separate struct
object, but that is still usually easier than extracting each of the
members individually from the buffer.

>> or if you dereference a pointer to a misaligned member, then all
>> bets are off.
> 
> So saying &foo.bar suddenly becomes a bug waiting to happen, because 
> foo is a packed struct?

Yes.  Here be dragons.

> Yes, I can see how that would be unavoidable. But surely a compiler
> which provides 'packed' would emit a warning for that code?
> ...
> If it doesn't, then I agree about staying away from packed structs
> (which I do anyway, but that strengthens my argument a lot).

I'd hope so, but I wouldn't count on it.

> In a sense foo.bar has no more of an address than a bitfield has.

The former has an address that is merely misaligned; the latter has no
address at all because individual bits aren't addressable.

S

-- 
Stephen Sprunk         "God does not play dice."  --Albert Einstein
CCIE #3723         "God is an inveterate gambler, and He throws the
K5SSS        dice at every possible opportunity." --Stephen Hawking

[toc] | [prev] | [next] | [standalone]

#77039

On Mon, 23 Nov 2015 22:20:35 +0000, Jorgen Grahn wrote:
> On Mon, 2015-11-23, David Brown wrote:
> ...
>> I'd advise against __attribute__((packed)) (or equivalents on other 
>> compilers) - either it makes no difference because there is no padding, 
>> or you will end up with structures with fields that don't match their 
>> alignment.  On some targets, that means run-time errors - on others, it 
>> means seriously degraded code speed.  So it is rarely a good idea.
> 
> I think I agree, but are there really cases where you say
> __attribute__((packed)) and then suffer a crash because of it?  The
> compiler shouldn't generate a struct, and code to access it which will
> crash if it's ever run.

Yes.  Since the packed struct members will not have the correct alignment,
using the & operator on the struct members will usually result in problems
if the pointer is subsequently dereferenced.

Note that gcc's packed attribute also relaxes the alignment requirements of
the struct iself, so in practice gcc almost always has to assume that any
member accesses are potentially unaligned, even if the struct has no
padding.  This usually results in worse code.

[toc] | [prev] | [next] | [standalone]

#77046

Jorgen Grahn <grahn+nntp@snipabacken.se> writes:
> On Mon, 2015-11-23, David Brown wrote:
> ...
>> I'd advise against __attribute__((packed)) (or equivalents on other 
>> compilers) - either it makes no difference because there is no padding, 
>> or you will end up with structures with fields that don't match their 
>> alignment.  On some targets, that means run-time errors - on others, it 
>> means seriously degraded code speed.  So it is rarely a good idea.
>
> I think I agree, but are there really cases where you say
> __attribute__((packed)) and then suffer a crash because of it?  The
> compiler shouldn't generate a struct, and code to access it which will
> crash if it's ever run.

Yes -- at least on systems where a misaligned access causes a crash.
(On x86 and x86_64, it doesn't; it's just a little slower.)

For example, given:

    struct foo {
        char c;
        int x;
    } __attribute__((packed));

if a struct foo object is, say, 4-byte aligned, then the member x is
likely to be misaligned.  If you refer to the x member of a struct foo
object:

    struct foo obj;
    obj.x;

then gcc knows it's a member of a packed struct, and will generate
whatever code is necessary to access it.  But if you take the address:

    int *ptr = &obj.x;
    ... *ptr ...

then gcc will assume that *ptr is properly aligned, and a crash is
likely.

This could be fixed by disallowing taking the address of misaligned
members (as is done for bit fields), or of *any* member of a packed
struct, or, less realistically, by generating inefficient code
for all dereferences of pointers to types that require nontrivial
alignment.

See also <http://stackoverflow.com/q/8568432/827263>.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#77047

On Monday, November 23, 2015 at 5:50:05 PM UTC-6, Keith Thompson wrote:
> For example, given:
> 
>     struct foo {
>         char c;
>         int x;
>     } __attribute__((packed));
> 
> ...            But if you take the address:
> 
>     int *ptr = &obj.x;
>     ... *ptr ...
> 
> then gcc will assume that *ptr is properly aligned, and a crash is
> likely.
> 
> This could be fixed by disallowing taking the address of misaligned
> members (as is done for bit fields), or of *any* member of a packed
> struct, or, less realistically, by generating inefficient code
> for all dereferences of pointers to types that require nontrivial
> alignment.

Or by requiring that the above be written as something like:

>     int __attribute__((packed)) *ptr = &obj.x;

I'm not sure if that's the right syntax for any particular compiler, but
the above would in some platforms cause all accesses made via *ptr to be
slower than they otherwise would, but not affect the performance of other
pointers to integers.

[toc] | [prev] | [next] | [standalone]

#77062

On 23/11/15 23:20, Jorgen Grahn wrote:
> On Mon, 2015-11-23, David Brown wrote:
> ...
>> I'd advise against __attribute__((packed)) (or equivalents on other 
>> compilers) - either it makes no difference because there is no padding, 
>> or you will end up with structures with fields that don't match their 
>> alignment.  On some targets, that means run-time errors - on others, it 
>> means seriously degraded code speed.  So it is rarely a good idea.
> 
> I think I agree, but are there really cases where you say
> __attribute__((packed)) and then suffer a crash because of it?  The
> compiler shouldn't generate a struct, and code to access it which will
> crash if it's ever run.
> 

The attribute tells gcc that it should generate the struct layout
exactly as /you/ tell it - /not/ as the target system normally requires
with respect to alignment.  On some systems, misaligned accesses leads
to a bus fault or similar hardware exception - a crash, in most cases.
On some other systems, you will not get a hardware exception but you
will access incorrect data (for example, a 16-bit access will simply
ignore the lowest bit of the address).  That means disaster of some sort.

If you use "packed", then /you/ take full responsibility for making sure
the accesses are correct.  Perhaps you want to create a pointer to that
struct that is not fully aligned (that's going to involve some
implementation-dependent pointer casts).  Perhaps you use memcpy() to
access the fields, or some other method that you know is safe.

And perhaps the compiler will generate combinations of smaller accesses
in order to make it safe on your particular target.  That is all up to
the compiler in question - it is all implementation-dependent.

So if it is critical, you can use "packed" if you make sure it is safe
to do so on /your/ target, with /your/ compiler, using /your/ access
methods - and treat it as highly implementation-dependent.

Or just use explicit padding with a static assertion on the final size
(or particular offsets, if you prefer) to make the code clear, safe, and
portable (or at least giving clear error messages when you hit
portability issues).

[toc] | [prev] | [next] | [standalone]

#77097

On Tue, 2015-11-24, David Brown wrote:
> On 23/11/15 23:20, Jorgen Grahn wrote:
>> On Mon, 2015-11-23, David Brown wrote:
>> ...
>>> I'd advise against __attribute__((packed)) (or equivalents on other 
>>> compilers) - either it makes no difference because there is no padding, 
>>> or you will end up with structures with fields that don't match their 
>>> alignment.  On some targets, that means run-time errors - on others, it 
>>> means seriously degraded code speed.  So it is rarely a good idea.
>> 
>> I think I agree, but are there really cases where you say
>> __attribute__((packed)) and then suffer a crash because of it?  The
>> compiler shouldn't generate a struct, and code to access it which will
>> crash if it's ever run.
>> 
>
> The attribute tells gcc that it should generate the struct layout
> exactly as /you/ tell it - /not/ as the target system normally requires
> with respect to alignment.
...
> So if it is critical, you can use "packed" if you make sure it is safe
> to do so on /your/ target, with /your/ compiler, using /your/ access
> methods - and treat it as highly implementation-dependent.

Makes sense.  I just wish the gcc documentation had expressed it that
way, so I could point people to it when they abuse it.

> Or just use explicit padding with a static assertion on the final size
> (or particular offsets, if you prefer) to make the code clear, safe, and
> portable (or at least giving clear error messages when you hit
> portability issues).

I prefer reading uint8_t by uint8_t from a buffer in most cases where
I could have used a struct "overlay".  But yes, your way works too in
many important cases (and importantly will not fail at runtime).

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [next] | [standalone]

#77129

On 24/11/15 15:29, Jorgen Grahn wrote:
> On Tue, 2015-11-24, David Brown wrote:
>> On 23/11/15 23:20, Jorgen Grahn wrote:
>>> On Mon, 2015-11-23, David Brown wrote:
>>> ...
>>>> I'd advise against __attribute__((packed)) (or equivalents on other
>>>> compilers) - either it makes no difference because there is no padding,
>>>> or you will end up with structures with fields that don't match their
>>>> alignment.  On some targets, that means run-time errors - on others, it
>>>> means seriously degraded code speed.  So it is rarely a good idea.
>>>
>>> I think I agree, but are there really cases where you say
>>> __attribute__((packed)) and then suffer a crash because of it?  The
>>> compiler shouldn't generate a struct, and code to access it which will
>>> crash if it's ever run.
>>>
>>
>> The attribute tells gcc that it should generate the struct layout
>> exactly as /you/ tell it - /not/ as the target system normally requires
>> with respect to alignment.
> ...
>> So if it is critical, you can use "packed" if you make sure it is safe
>> to do so on /your/ target, with /your/ compiler, using /your/ access
>> methods - and treat it as highly implementation-dependent.
>
> Makes sense.  I just wish the gcc documentation had expressed it that
> way, so I could point people to it when they abuse it.
>
>> Or just use explicit padding with a static assertion on the final size
>> (or particular offsets, if you prefer) to make the code clear, safe, and
>> portable (or at least giving clear error messages when you hit
>> portability issues).
>
> I prefer reading uint8_t by uint8_t from a buffer in most cases where
> I could have used a struct "overlay".  But yes, your way works too in
> many important cases (and importantly will not fail at runtime).
>

memcpy is good for doing such copies - it has the advantage that if it 
could be done faster using 16-bit or bigger moves, the compiler can do 
that (if it is reasonably smart), with no worries about aliasing :-)

Also explicit packing "dummies" have the advantage that there is no 
chance of undefined, implementation-defined, unspecified or generally 
unexpected or undesired behaviour when they are accessed.  (When storing 
something in a struct, padding bytes get unspecified values.)  This is 
particularly important if you want to compare structs, or do something 
like calculate a checksum over them.

And as noted before, be generous with your static assertions.  You find 
out problems at compile time, with no run-time costs.

[toc] | [prev] | [next] | [standalone]

#77132

On 11/24/2015 4:58 PM, David Brown wrote:
> [... "packed" structs and similar underhanded artifices ...]
> Also explicit packing "dummies" have the advantage that there is no
> chance of undefined, implementation-defined, unspecified or generally
> unexpected or undesired behaviour when they are accessed.  (When storing
> something in a struct, padding bytes get unspecified values.)  This is
> particularly important if you want to compare structs, or do something
> like calculate a checksum over them.

     It's been my experience that these questions of packing and so on
nearly always arise in connection with an externally-imposed format of
some kind: You want to use a struct to represent an IP header or a
disk partition table entry or something of that sort.  If that's the
situation, I've almost always found that trying to sculpt the struct
into the desired shape is inferior to just letting the struct take its
own preferred form and using a mediating buffer to translate to and
from the imposed format.  Use separate "native" and "serialized" forms,
and you're quite likely to be better off than by trying to force both
forms into one object.

     Not only does the separation of forms avoid all the alignment
issues discussed elsethread, but it also offers a convenient hook for
dealing with other matters.  Does the external form require a specific
endianness that may or may not match your machine's?  Does it use a
weird representation for negative numbers, or for floating-point
quantities?  Are text strings supposed to be a count word followed
by a payload in some strange five-bit encoding?  The import() and
export() functions can handle all such idiosyncracies cleanly and
quietly, so the rest of the program can deal with perfectly normal,
native-format structs with native-format numbers, zero-terminated
native-encoded strings, and so on.  The translation functions are
also good places to compute and verify things like checksums, which
are usually specified in terms of the external format.

     Somewhere, somebody is shrieking "Efficiency!  All that copying?
Why should I un-pickle an entire object just to test the one field
that tells me whether I care about the others?  Eeeek!"  This shrieker
is overly fearful and insufficiently imaginative; there are plenty of
ways to attack such issues if they should prove important.  (Also,
for some reason the shrieker seldom yowls about the speed penalties
of using the misaligned elements in his packed structs.  Go figure.)

     There are many situations where it is tempting to map an imposed
format with a struct, but resisting temptation is nearly always a
virtue, and a virtue with rewards.

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | comp.lang.c

csiph-web