[9fans] ureg alignment

[9fans] ureg alignment

[erik quanstrom]

on 64-bit machines, the unions in the ureg.h can lead to
internal padding.  (power64 avoids this issue because everything
is 64-bit aligned anyway.)  to sidestep the issue, i think
it might make sense to use #defines.  for example, for arm
the conversion would look something like this:

typedef struct Ureg {
	ulong	r0;
	ulong	r1;
	ulong	r2;
	ulong	r3;
	ulong	r4;
	ulong	r5;
	ulong	r6;
	ulong	r7;
	ulong	r8;
	ulong	r9;
	ulong	r10;
	ulong	r11;
	ulong	r12;	/* sb */
	ulong	r13;
#define	sp	r13
	ulong	r14;
#define	link	r14
	ulong	type;	/* of exception */
	ulong	psr;
	ulong	pc;	/* interrupted addr */
} Ureg;

is there any reason not to do this?

- erik

Re: [9fans] ureg alignment

[Steven Stallion]

On Fri, May 9, 2014 at 9:21 AM, erik quanstrom <quanstro@quanstro.net> wrote:
> on 64-bit machines, the unions in the ureg.h can lead to
> internal padding.  (power64 avoids this issue because everything
> is 64-bit aligned anyway.)  to sidestep the issue, i think
> it might make sense to use #defines.  for example, for arm
> the conversion would look something like this:
>
> typedef struct Ureg {
>         ulong   r0;
>         ulong   r1;
>         ulong   r2;
>         ulong   r3;
>         ulong   r4;
>         ulong   r5;
>         ulong   r6;
>         ulong   r7;
>         ulong   r8;
>         ulong   r9;
>         ulong   r10;
>         ulong   r11;
>         ulong   r12;    /* sb */
>         ulong   r13;
> #define sp      r13
>         ulong   r14;
> #define link    r14
>         ulong   type;   /* of exception */
>         ulong   psr;
>         ulong   pc;     /* interrupted addr */
> } Ureg;
>
> is there any reason not to do this?

Ugh, no! Is there a case where the padding is a problem? Normally
registers belonging to the same union are uniform in size. Those
defines will expand anywhere and not behave as expected:

ulong sp = ureg->sp;

This could lead to all sorts of entertaining problems.

Steve

Re: [9fans] ureg alignment

[erik quanstrom]

> Ugh, no! Is there a case where the padding is a problem? Normally
> registers belonging to the same union are uniform in size. Those
> defines will expand anywhere and not behave as expected:
>
> ulong sp = ureg->sp;
>
> This could lead to all sorts of entertaining problems.

any union on a 64-bit machine will get padded out to 0%8 bytes.

also your example

	ulong sp = ureg->sp

will get expanded to

	ulong r13 = ureg->r13

and this is just fine!

- erik

Re: [9fans] ureg alignment

[Charles Forsyth]

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

On 9 May 2014 15:21, erik quanstrom <quanstro@quanstro.net> wrote:

> on 64-bit machines, the unions in the ureg.h can lead to
> internal padding.
>

although one tactic not yet implemented, but soon ..., is to make struct
and union alignment
match the least alignment required, as is done for arrays, the approach on
amd64 and arm64
is simply not to bother with the unions. if it's sp, it's sp. link is link.
i think the need for the alias
has gone. there aren't any references in port, which i think might have
caused the aliasing
in the first place, and xyz/trap.c should know what's what.

[-- Attachment #2: Type: text/html, Size: 1087 bytes --]

Re: [9fans] ureg alignment

[erik quanstrom]

> > on 64-bit machines, the unions in the ureg.h can lead to internal
> > padding.
>
> although one tactic not yet implemented, but soon ..., is to make
> struct and union alignment match the least alignment required, as is
> done for arrays, the approach on amd64 and arm64 is simply not to
> bother with the unions.  if it's sp, it's sp.  link is link.  i think
> the need for the alias has gone.  there aren't any references in port,
> which i think might have caused the aliasing in the first place, and
> xyz/trap.c should know what's what.

good point. i hadn't considered that it might simply be unnecessary.
perhaps because i was also fretting about this not being done consistently
across various codebases.  do you think that's perhaps worth leaving the
#define in as a temporary measure?

- erik

Re: [9fans] ureg alignment

[Charles Forsyth]

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

On 9 May 2014 16:29, erik quanstrom <quanstro@quanstro.net> wrote:

> do you think that's perhaps worth leaving the
> #define in as a temporary measure?
>

but if you're working with arm, the existing union is fine as it is, so why
not just leave it?

[-- Attachment #2: Type: text/html, Size: 612 bytes --]

Re: [9fans] ureg alignment

[erik quanstrom]

On Fri May  9 11:46:05 EDT 2014, charles.forsyth@gmail.com wrote:

> On 9 May 2014 16:29, erik quanstrom <quanstro@quanstro.net> wrote:
>
> > do you think that's perhaps worth leaving the
> > #define in as a temporary measure?
> >
>
> but if you're working with arm, the existing union is fine as it is, so why
> not just leave it?

the point is to make things like debugging arm, mips or 386 snapshots work on
amd64.  without a change, cross-platform debugging doesn't work in all cases.

- erik