Discussion of the jack audio server and jack applications

Basil Nutmeg | 19 Jul 2012 05:41

packed data structures in jack2

Hello,

I found Jack2 was crashing on my ARM cpu, and I traced it down to a
misuse of __attribute__((packed)).

This CPU doesn't support misaligned memory references. packed structs are
supported, by having the compiler emit a longer sequence of instructions.
However, consider this code from common/JackAtomicArrayState.h:

... CAS(Counter1(old_val), Counter1(new_val), (UInt32*)&fCounter) ...

fCounter is a member of a class which is packed, and this happens to make
it misaligned. Its address is taken and passed off to a different
function, where it appears to the compiler to be an ordinary pointer. The
compiler justifiably assumes the pointer is aligned, so it emits normal
code, and the code crashes at runtime because the pointer isn't actually
aligned.

Looking around, I was surprised to see that lots of classes in Jack2 are
marked packed, for example classes like JackConnectionManager,
JackClientControl, and JackEngineControl, and I'm puzzled.

Making JackEngineControl packed, for example, shrinks its nominal size
from 1040 to 1032 (on this platform). It doesn't look like there are that
many JackEngineControl instances floating around at runtime, so this isn't
actually saving much memory. And it doesn't appear to be serialized
anywhere. And on the down side, packed makes many of JackEngineControl's
fields misaligned, making them slower to access.

Also, it's possible to shrink JackEngineControl to size 1032 just by

(Continue reading)

headers

Stéphane Letz | 19 Jul 2012 06:54

Re: packed data structures in jack2

The point to pack memory is to allow a 64 bits server to be used with 32 bits clients or the contrary. Since
shared memory is going to be used by "both sides", the only safe way we found was to use packed memory struct.

If 64/32 bits code cohabitation is not mandatory, then I guess "packed" can be disabled.

Stephane

Le 19 juil. 2012 à 05:41, Basil Nutmeg a écrit :

> Hello,
> 
> I found Jack2 was crashing on my ARM cpu, and I traced it down to a
> misuse of __attribute__((packed)).
> 
> This CPU doesn't support misaligned memory references. packed structs are
> supported, by having the compiler emit a longer sequence of instructions.
> However, consider this code from common/JackAtomicArrayState.h:
> 
> ... CAS(Counter1(old_val), Counter1(new_val), (UInt32*)&fCounter) ...
> 
> fCounter is a member of a class which is packed, and this happens to make
> it misaligned. Its address is taken and passed off to a different
> function, where it appears to the compiler to be an ordinary pointer. The
> compiler justifiably assumes the pointer is aligned, so it emits normal
> code, and the code crashes at runtime because the pointer isn't actually
> aligned.
> 
> Looking around, I was surprised to see that lots of classes in Jack2 are
> marked packed, for example classes like JackConnectionManager,
> JackClientControl, and JackEngineControl, and I'm puzzled.

(Continue reading)

headers

Basil Nutmeg | 19 Jul 2012 17:41

Re: packed data structures in jack2

On Thu, Jul 19, 2012 at 06:54:41AM +0200, Stéphane Letz wrote:
> The point to pack memory is to allow a 64 bits server to be used with 32 bits clients or the contrary. Since
shared memory is going to be used by "both sides", the only safe way we found was to use packed memory struct.

Ah, thanks, I see now. I see how putting packed on everything does make
this easier. However, it also makes the code unusable on some platforms
and slow on others. Would you be open to alternative approaches here? I'd
be willing to do some work to help out.

For example, here's one idea:

/*
 * To facilitate sharing data between 32-bit and 64-bit processes, we
 * use types that are fully aligned (aligned to their size), to avoid
 * differences in alignment rules. This macro facilitates creating
 * specially-aligned versions of basic types for this purpose.
 *
 * For example,
 *
 *     declare_aligned_type(int64_t);
 *
 * declares a typedef aligned_int64_t to be an int64_t that is always
 * aligned to an 8-byte boundary (assuming sizeof(int64_t) is 8...).
 */
#define declare_aligned_type(T) \
    typedef T aligned_##T __attribute__((aligned(sizeof(T))))

declare_aligned_type(char);
declare_aligned_type(int64_t);
/* ... and more */

(Continue reading)

headers

Stéphane Letz | 19 Jul 2012 17:54

Re: packed data structures in jack2

They are some global PRE_PACKED_STRUCTURE and POST_PACKED_STRUCTURE

Why not just define them empty for your needs?

Stephane 

Le 19 juil. 2012 à 17:41, Basil Nutmeg a écrit :

> On Thu, Jul 19, 2012 at 06:54:41AM +0200, Stéphane Letz wrote:
>> The point to pack memory is to allow a 64 bits server to be used with 32 bits clients or the contrary. Since
shared memory is going to be used by "both sides", the only safe way we found was to use packed memory struct.
> 
> Ah, thanks, I see now. I see how putting packed on everything does make
> this easier. However, it also makes the code unusable on some platforms
> and slow on others. Would you be open to alternative approaches here? I'd
> be willing to do some work to help out.
> 
> For example, here's one idea:
> 
> /*
> * To facilitate sharing data between 32-bit and 64-bit processes, we
> * use types that are fully aligned (aligned to their size), to avoid
> * differences in alignment rules. This macro facilitates creating
> * specially-aligned versions of basic types for this purpose.
> *
> * For example,
> *
> *     declare_aligned_type(int64_t);
> *
> * declares a typedef aligned_int64_t to be an int64_t that is always

(Continue reading)

headers

Paul Davis | 19 Jul 2012 17:57

Re: packed data structures in jack2

On Thu, Jul 19, 2012 at 11:41 AM, Basil Nutmeg <basil+jack <at> li95-58.members.linode.com> wrote:

On Thu, Jul 19, 2012 at 06:54:41AM +0200, Stéphane Letz wrote:
> The point to pack memory is to allow a 64 bits server to be used with 32 bits clients or the contrary. Since shared memory is going to be used by "both sides", the only safe way we found was to use packed memory struct.

Ah, thanks, I see now. I see how putting packed on everything does make
this easier. However, it also makes the code unusable on some platforms
and slow on others. Would you be open to alternative approaches here? I'd
be willing to do some work to help out

[ ... ]

/* ... this type has the same layout between 32-bit and 64-bit. */
struct A_with_alignment {
aligned_char x;
aligned_int64_t i;
};

this translates to:

     struct A_with_alignment {
          char     x    __attribute__((aligned(sizeof(char));
          int64_t i __attribute__((aligned(sizeof(int64_t));
      };

which i think is not the same as what "packed" does at all. it could be enough though, and arguably is better because it preserves aligned access.

however, as stephane noted: far easier to just change the macros that do this stuff so that they are no-ops.

As another idea would be to have a source file which simply includes all
the important headers and is compiled with -Wpadded and maybe even
-Werror, to catch any mistakes.

i don't this is portable to non-gnu compilers. i could be wrong.

_______________________________________________
Jack-Devel mailing list
Jack-Devel <at> lists.jackaudio.org
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org

headers

Basil Nutmeg | 19 Jul 2012 19:32

Re: packed data structures in jack2

On Thu, Jul 19, 2012 at 11:57:42AM -0400, Paul Davis wrote:
> On Thu, Jul 19, 2012 at 11:41 AM, Basil Nutmeg <
> basil+jack <at> li95-58.members.linode.com> wrote:
> 
> >
> > /* ... this type has the same layout between 32-bit and 64-bit. */
> > struct A_with_alignment {
> >   aligned_char x;
> >   aligned_int64_t i;
> > };
> >
> 
> this translates to:
> 
>      struct A_with_alignment {
>           char     x    __attribute__((aligned(sizeof(char));
>           int64_t i     __attribute__((aligned(sizeof(int64_t));
>       };
> 
> which i think is not the same as what "packed" does at all. it could be
> enough though, and arguably is better because it preserves aligned access.

Right, the goal isn't to emulate "packed" exactly; the goal is to solve a
specific problem without incurring the undesirable side effects .

> 
> however, as stephane noted: far easier to just change the macros that do
> this stuff so that they are no-ops.

This is easy to do, and it works for my project today. However, I was
thinking about what would be best for Jack in the long term.

I think it would be preferable to have a solution which could support
mixed 64/32 mode even on platforms which don't have misaligned accesses
(64-bit ARM chips are coming...), which would avoid the performance
issues, and which would avoid "breaking the rules" even on platforms
where it works, because clever compiler writers can still theoretically 
find ways to take advantage of alignment assumptions there. And it would
undo what is technically an ABI break in the recent addition of "packed"
to <jack/types.h>.

I think it's even worthwhile to make moderately more invasive source
changes to achieve this. And, I'm willing to do some work to help make it
happen. But if the maintainers disagree, I'll move on.

> 
> > As another idea would be to have a source file which simply includes all
> > the important headers and is compiled with -Wpadded and maybe even
> > -Werror, to catch any mistakes.
> >
> 
> i don't this is portable to non-gnu compilers. i could be wrong.

Perhaps so, or perhaps other compilers have analogous features; I don't
know offhand. I'd be willing to do some research here. It might not be a
complete solution, but it might be a nice sanity check, regardless.

-- Basil

headers

Stéphane Letz | 19 Jul 2012 19:37

Re: packed data structures in jack2


Le 19 juil. 2012 à 19:32, Basil Nutmeg a écrit :

> On Thu, Jul 19, 2012 at 11:57:42AM -0400, Paul Davis wrote:
>> On Thu, Jul 19, 2012 at 11:41 AM, Basil Nutmeg <
>> basil+jack <at> li95-58.members.linode.com> wrote:
>> 
>>> 
>>> /* ... this type has the same layout between 32-bit and 64-bit. */
>>> struct A_with_alignment {
>>>  aligned_char x;
>>>  aligned_int64_t i;
>>> };
>>> 
>> 
>> this translates to:
>> 
>>     struct A_with_alignment {
>>          char     x    __attribute__((aligned(sizeof(char));
>>          int64_t i     __attribute__((aligned(sizeof(int64_t));
>>      };
>> 
>> which i think is not the same as what "packed" does at all. it could be
>> enough though, and arguably is better because it preserves aligned access.
> 
> Right, the goal isn't to emulate "packed" exactly; the goal is to solve a
> specific problem without incurring the undesirable side effects .
> 
>> 
>> however, as stephane noted: far easier to just change the macros that do
>> this stuff so that they are no-ops.
> 
> This is easy to do, and it works for my project today. However, I was
> thinking about what would be best for Jack in the long term.
> 
> I think it would be preferable to have a solution which could support
> mixed 64/32 mode even on platforms which don't have misaligned accesses
> (64-bit ARM chips are coming...), which would avoid the performance
> issues, and which would avoid "breaking the rules" even on platforms
> where it works, because clever compiler writers can still theoretically 
> find ways to take advantage of alignment assumptions there. And it would
> undo what is technically an ABI break in the recent addition of "packed"
> to <jack/types.h>.
> 
> I think it's even worthwhile to make moderately more invasive source
> changes to achieve this. And, I'm willing to do some work to help make it
> happen. But if the maintainers disagree, I'll move on.

Use a git branch.

> 
>> 
>>> As another idea would be to have a source file which simply includes all
>>> the important headers and is compiled with -Wpadded and maybe even
>>> -Werror, to catch any mistakes.
>>> 
>> 
>> i don't this is portable to non-gnu compilers. i could be wrong.
> 
> Perhaps so, or perhaps other compilers have analogous features; I don't
> know offhand. I'd be willing to do some research here. It might not be a
> complete solution, but it might be a nice sanity check, regardless.
> 
> -- Basil

It has to work on Windows with VC++ and Mingw kid of compiler at least.

Stéphane

headers

Nedko Arnaudov | 19 Jul 2012 20:20

Re: packed data structures in jack2

[In plain C] the proper solution for the problem is to use appropriate
aritifical padding members in structs and eventually enforce packing as
well. The padding must be such that pointer used to access a give data
type must be aligned to that size (this is in the C standard). RISC
processors tend to generate exception for this, CISC only executes
slower. Some operating systems catch the [hardware] unaligned access
exception and do the byte-by-byte copy and some of them even generate a
log line.

The solution should be appropriate for C++ as well but I'll leave more
C++ affiliated ones to do such claims.

--

-- 
Nedko Arnaudov <GnuPG KeyID: 5D1B58ED>

_______________________________________________
Jack-Devel mailing list
Jack-Devel <at> lists.jackaudio.org
http://lists.jackaudio.org/listinfo.cgi/jack-devel-jackaudio.org