eat your own dog food?

Hey Arnaldo,

You ever tasted what you are cooking?;->

---

hadi@lilsol:~/git-trees/arnaldo/jamal/build$ pahole ./pahole| grep XXX
        /* XXX last struct has 1 byte of padding */
        /* XXX 1 bit hole, try to pack */
        /* XXX 1 byte hole, try to pack */
        /* XXX 2 bytes hole, try to pack */
        /* XXX 2 bits hole, try to pack */
        /* XXX 1 byte hole, try to pack */
        /* XXX last struct has 2 bytes of padding */
        /* XXX 1 byte hole, try to pack */
hadi@lilsol:~/git-trees/arnaldo/jamal/build$
---

On a serious note, feature request:

I am trying to put words to what i am looking for and it's a little hard without providing a lot of boring context. For brevity, lets say i have an inherited 500K lines of code. Assume i compiled a program for 32-bit target and now i have to move it to a 128-bit playstation 3 with whatever strange alignment rules.
I want to do achieve 2 and a half things:

a)
i) check the structure for holes for X-bit access alignment where X is 16 (old 680xx), 64, 128. I want to do this without compiling the program for any of those targets i.e i want to do this on my x86 32-bit laptop ;-> i.e call this cross-pahole

ii) If i can do the above; i should then be able to say "optimize" (what you call --reorganize) for X-bit

b) I want to do a multiple of these example "optimize for 32-bit and 64-bit" or "check if it is safe for 128-bit and 16-bit alignemnt"

Note, clearly if i can make it read config files with rules for something like the MIPS EABI in the first part of this email:
http://sourceware.org/ml/binutils/2003-06/msg00436.html
then i can say "check if it safe for mips 32 bit and x86_64" ;-> And i can make sure my apps will run on that new cpu i dont have access to.

I could ask for more, but let me start/stop here incase you are going to lynch me;->

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 07:42:48AM -0500, jamal escreveu:

:-) I'll check that later :-P But some holes are inevitable, the best thing is to try:

[acme@doppio pahole]$ pahole --packable build/pahole

argp_child      32      24      8
argp_option     48      40      8
_IO_FILE        216     208     8

[acme@doppio pahole]$

Which is kinda interesting, huh? Only glibc stuff :-)

--packable means: run --reorganize in all structs and compare the result with the original struct, if we have savings, tell us (original size, reorganized size, savings).

No need for lynching this time. What we have to do is:

. make the addr_size configurable and when the user specifies an addr_size that is different than the one in the DWARF info being processed, fixup it in all the data structures. The --reorganize code already does many kinds of fixups, it would be one more.

. moreover, specify alignment rules, that way the reorganization code would process all the structs, applying this rule.

IOW: its possible, should be fairly easy to implement the addr_size and from that the alignment rules.

I'll try to work on the addr_size first, perhaps this weekend as I've been promising to do way too many things in many fronts :-\

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

hehe;-> damn those dog-food cans.

ok;->

ok, cool - so i should be able say something like "--add_size 64-bit" and it will follow something like the MIPS EABI i posted earlier? [I think x86_64 will work the same way too if i am not mistaken].

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 08:55:41AM -0500, jamal escreveu:

Exactly, you will be able to change the addr_size, I think that there is even code that can be right away used for that, lemme check.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 12:06:35PM -0200, Arnaldo Carvalho de Melo escreveu:

First stab, doesn't handles unions shrinking because the largest member shrunk, and handles only binaries built for 64 bits (or bigger) having it addr_size (word_size) shrinking.

So:

[acme@doppio pahole]$ file build/pahole
build/pahole: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.9, not stripped

A 64 bit app:

[acme@doppio pahole]$ build/pahole -C structure build/pahole struct structure {

        struct list_head  node;                 /*     0    16 */
        struct class *    class;                /*    16     8 */
        const struct cu  *cu;                   /*    24     8 */
        uint32_t          nr_files;             /*    32     4 */
        uint32_t          nr_methods;           /*    36     4 */

        /* size: 40, cachelines: 1 */
        /* last cacheline: 40 bytes */
};

[acme@doppio pahole]$

Lets change the word_size from 8 to 4 (64 bits -> 32 bits)

[acme@doppio pahole]$ build/pahole -w 4 -C structure build/pahole struct structure {

	struct list_head  node;                 /*     0     8 */
	struct class *    class;                /*     8     4 */
	const struct cu  *cu;                   /*    12     4 */
	uint32_t          nr_files;             /*    16     4 */
	uint32_t          nr_methods;           /*    20     4 */

	/* size: 24, cachelines: 1 */
	/* last cacheline: 24 bytes */
};

See that it recursively notices structs shrinking, because...

[acme@doppio pahole]$ build/pahole -C list_head build/pahole

struct list_head {
	struct list_head *next;                 /*     0     8 */
	struct list_head *prev;                 /*     8     8 */

	/* size: 16, cachelines: 1 */
	/* last cacheline: 16 bytes */
};

When using a 4 bytes word_size:

[acme@doppio pahole]$ build/pahole -w 4 -C list_head build/pahole

struct list_head {
	struct list_head *next;                 /*     0     4 */
	struct list_head *prev;                 /*     4     4 */

	/* size: 8, cachelines: 1 */
	/* last cacheline: 8 bytes */
};

Lets go all the way back to the 8088:

[acme@doppio pahole]$ build/pahole -w 1 -C structure build/pahole struct structure {

	struct list_head           node;                 /*     0     2 */
	struct class *             class;                /*     2     1 */
	const struct cu  *         cu;                   /*     3     1 */
	uint32_t                   nr_files;             /*     4     4 */
	uint32_t                   nr_methods;           /*     8     4 */

	/* size: 12, cachelines: 1 */
	/* last cacheline: 12 bytes */
};

Not really because we're only change 'long' from the original word size to the one specified.

Please test and find the missing bits for me :-)

- Arnaldo

index f5edb65..526335f 100644
@@ -107,6 +107,7 @@ struct type {
 	struct list_head node;
 	Dwarf_Off	 specification;
 	size_t		 size;
+	size_t		 size_diff; /* use when changing cu->addr_size */
 	uint16_t	 nr_members;
 	uint8_t		 declaration; /* only one bit used */
 	uint8_t		 definition_emitted:1;

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Only for 64-bit machines?
My laptop is still 32-bit - from the patch i cant see anything that'll stop me from testing for 64 bit from a 32-bit compilation.

[..]

;->

Will do later today.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 02:39:59PM -0200, Arnaldo Carvalho de Melo escreveu:

Working now on unions, etc, have you had the chance of testing this patch?

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I should be able to test it in a couple of hours. If you have the unions change before then, send me a unified patch - else an incremental version.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 04:37:14PM -0500, jamal escreveu:

for 32 bit binaries we'll need to add to the offsets after pointers and long ints that will be 64 bits when the compiler marked them as 32.

good data point from you, that your laptop is 32 bits, I'll implement the small -> big transition now.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I'll take either incremental or full patch including one you sent earlier.

Btw, thinking about this a little more, it may be trickier than i thought initially and will have to including eabi considerations even in the case of two machines of equivalent bit width. Tell me if i am bad crack:

Consider ia32 and MIPS 32 bit (as per eabi defined here
http://sourceware.org/ml/binutils/2003-06/msg00436.html)

consider this:

typedef unsigned int __u32;
typedef unsigned long long __u64;
struct tc_stats
{
__u64 bytes;
__u32 packets;
__u32 drops;
__u32 overlimits;
__u32 bps;
__u32 pps;
__u32 qlen;
__u32 backlog;
};

On mips:
sizeof (struct tc_stats) == 40 and it is aligned 8

On my laptop:
sizeof (struct tc_stats) == 36 aligned at 4

The mips alignment explains the padding because long long is 8-byte aligned.

i.e eventually you may need to have an eabi file per architecture

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I guess i am not on bad crack - heres ia32 data represantation:
http://www.linux-foundation.org/spec/book/ELF-IA32/ELF-IA32/lowlevel.html#DATAREPRESENTATION

That would explain it.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Thu, Jan 10, 2008 at 04:41:16PM -0500, jamal escreveu:

Remove the non-commited patches and do a git-pull, basic work is done, reusing code I had in ctracer to do some simple alignment fixups.

Please let me know about any bugs you may find.

- Arnaldo

commit f6eb9ed92dafbd8cac082737fe0352c0ff6e7fdc

Author: Arnaldo Carvalho de Melo <[email protected]>
Date:   Sat Jan 12 13:14:40 2008 -0200

    [PAHOLE]: Allow changing the architecture word-size
    
    Using a x86_64 binary:
    
    [acme@doppio pahole]$ build/pahole -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int          (*fn)(struct restart_block *); /*     0     8 */
    	union {
    		struct {
    			long unsigned int arg0;          /*     8     8 */
    			long unsigned int arg1;          /*    16     8 */
    			long unsigned int arg2;          /*    24     8 */
    			long unsigned int arg3;          /*    32     8 */
    		};                                       /*          32 */
    		struct {
    			u32 *      uaddr;                /*     8     8 */
    			u32        val;                  /*    16     4 */
    			u32        flags;                /*    20     4 */
    			u64        time;                 /*    24     8 */
    		} fu;                                    /*          24 */
    	};                                               /*     8    32 */
    
    	/* size: 40, cachelines: 1 */
    	/* last cacheline: 40 bytes */
    };
    
    Changing the word-size from 8 to 4 bytes:
    
    [acme@doppio pahole]$ build/pahole -w 4 -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int       (*fn)(struct restart_block *); /*     0     4 */
    	union {
    		struct {
    			long unsigned int arg0;          /*     4     4 */
    			long unsigned int arg1;          /*     8     4 */
    			long unsigned int arg2;          /*    12     4 */
    			long unsigned int arg3;          /*    16     4 */
    		};                                       /*          16 */
    		struct {
    			u32 *      uaddr;                /*     4     4 */
    			u32        val;                  /*     8     4 */
    			u32        flags;                /*    12     4 */
    			u64        time;                 /*    16     8 */
    		} fu;                                    /*          20 */
    	};                                               /*     4    20 */
    
    	/* size: 24, cachelines: 1 */
    	/* last cacheline: 24 bytes */
    };
    
    And from 8 to 16:
    
    [acme@doppio pahole]$ build/pahole -w 16 -C restart_block /usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug
    struct restart_block {
    	long int          (*fn)(struct restart_block *); /*     0    16 */
    	union {
    		struct {
    			long unsigned int arg0;          /*    16    16 */
    			long unsigned int arg1;          /*    32    16 */
    			long unsigned int arg2;          /*    48    16 */
    			long unsigned int arg3;          /*    64    16 */
    			/* --- cacheline 1 boundary (64 bytes) --- */
    		};                                       /*          64 */
    		struct {
    			u32 *      uaddr;                /*    16    16 */
    			u32        val;                  /*    32     4 */
    			u32        flags;                /*    36     4 */
    			u64        time;                 /*    40     8 */
    		} fu;                                    /*          32 */
    	};                                               /*    16    64 */
    	/* --- cacheline 1 boundary (64 bytes) was 16 bytes ago --- */
    
    	/* size: 80, cachelines: 2 */
    	/* last cacheline: 16 bytes */
    };
    
    More work is required to specify different alignment rules.
    
    Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sat, Jan 12, 2008 at 01:21:09PM -0200, Arnaldo Carvalho de Melo escreveu:

There are still some problems:

[acme@doppio pahole]$ build/pahole -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug

struct e1000_opt_list {
	int                        i;                    /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	char *                     str;                  /*     8     8 */

	/* size: 16, cachelines: 1 */
	/* sum members: 12, holes: 1, sum holes: 4 */
	/* last cacheline: 16 bytes */
};

So on 64 bits we have a hole because pointers are aligned at word-size boundaries, changing it to 4 bytes:

[acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug

struct e1000_opt_list {
	int                        i;                    /*     0     4 */

	/* XXX 4 bytes hole, try to pack */

	char *                     str;                  /*     8     4 */

	/* size: 12, cachelines: 1 */
	/* sum members: 8, holes: 1, sum holes: 4 */
	/* last cacheline: 12 bytes */
};

The pointer size was correctly changed to 4 and the size reduced to 12, but we should have subtracted 4 bytes from 'str' offset and the hole would disappear and the size becomes 8.

Working on it.

This new feature should be more exploited on an upcoming cool feature, one where we'll be able to ask pahole if a struct has optimal layout in a range of architectures.

That should be a piece of cake to implement as we'll just clone each struct, change the word size of it and check if with the new word-size there are holes where previously there wasn't.

With full fledged alignment rules in description files for each architecture we will be able to check on all arches if a struct is The Holy Grail :-)

Ah, and if we find out that on one arch it is unholy, i.e. has non-optimal layout, we can ask pahole to reorganize it and check again.

Humm, the reorganize (that jamal calls 'optimize') machinery will have to take into account the rules for all the architectures... That should be fun! :-P

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sat, Jan 12, 2008 at 01:54:04PM -0200, Arnaldo Carvalho de Melo escreveu:

Partially fixed, which says a lot about how bad class__fixup_alignment is :-\ Only holes after unions are not being fixed up, still scratching my head on why this is the case.

Anyway:

[acme@doppio pahole]$ build/pahole -w 4 -C e1000_opt_list \
/usr/lib/debug/lib/modules/2.6.21-65.el5rt/kernel/drivers/net/e1000/e1000.ko.debug

struct e1000_opt_list {
	int                        i;                    /*     0     4 */
	char *                     str;                  /*     4     4 */

	/* size: 8, cachelines: 1 */
	/* last cacheline: 8 bytes */
};

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sat, Jan 12, 2008 at 02:28:08PM -0200, Arnaldo Carvalho de Melo escreveu:

Problem wasn't in class__fixup_alignment, we have to check if the type of members were already resized when resizing a struct/union. Fixed'n'pushed.

Now what seems to be the last thing to do is to follow typedefs and find unnamed structs, think about spinlock_t, socket_lock_t, etc.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Good stuff Arnaldo - updated local tree.
I should be able to test later today.

Dont know much about cmake, is it possible to check for elfutils version and not depend on build_id if version < xxx?
This way i dont have to go out of my way to maintain my own versions of elf utilities. I have to do the patch you sent earlier manually now. I will remove it if you will fix cmake.

BTW, while perusing archs i managed i think to gather sufficiently accurate info (;->) on the different alignments; i got lazy at the end, but quiet a few are in the attached txt file incase you find them useful. Double check sparc for example. I think most 64-bit hardware looks consistent; the funnies are in 32-bit archs.

cheers,
jamal

		MIPS EABI
			=========

Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 (32-bit mode)
			8		8 (64-bit mode)
	long long	8		8
	float		4		4
	double		8		8
	pointers	4		4 (32-bit mode)
			8		8 (64-bit mode)

* alignment within aggregates (structs and unions) is as above, with padding added if needed
* aggregates have alignment equal to that of their most aligned member
* aggregates have sizes which are a multiple of their alignment

ia32 Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 
	long long	8		4
	float		4		4
	double		8		4
	pointers	4		4 

* alignment within aggregates (structs and unions) is as above, with padding added if needed
* aggregates have alignment equal to that of their most aligned member
* aggregates have sizes which are a multiple of their alignment This may result in tail padding.

ia64 Sizes and alignments
--------------------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		8		8 
	long long	8		8
	float		4		4
	double		8		8
	pointers	8		8 

    * An entire structure or union object shall be aligned on the same 
      boundary as its most strictly aligned member.
    * Each member shall be assigned to the lowest available offset with the 
      appropriate alignment. This may require internal padding, depending 
      on the previous member.
    * A structure's size shall be increased, if necessary, to make it a 
      multiple of the alignment. This may require tail padding, depending 
      on the last member. 

AMD 64
-------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long		4		4 
			8		8 (64-bit mode)
	long long	8		4
			8		8 (64-bit mode)
	float		4		4
	double		8		4
	pointers	4		4 
			8		8 (64-bit mode)

powerpc - 32bit

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		16
	pointers	4		4 

powerpc - 64bit

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        8		8
	long long	8		8
	int128  	16		16
	float		4		4
	double		8		8
	long double	16		16
	pointers	8		8 

sparc:

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		8
	pointers	4		4 

s390:
------

	Type		Size (bytes)	Alignment (bytes)

	char		1		1
	short		2		2
	int		4		4
	unsigned	4		4
	long	        4		4
	long long	8		8
	float		4		4
	double		8		8
	long double	16		16
	pointers	4		4 

s390-zseries
---------

Em Sat, Jan 12, 2008 at 01:08:41PM -0500, jamal escreveu:

As I said in the cset commit, I'm no cmake/autoconf guy, but I'll check that, after all this is a prime area where these otherwise distracting things should help.

I'll check that, thanks for the research, that may well be the starting point for arch descriptions, that if libebl from elfutils doesn't already provide that to us, something I haven't checked.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sat, Jan 12, 2008 at 03:28:56PM -0200, Arnaldo Carvalho de Melo escreveu:

OK, typedefs are being followed now, now we get to arrays, working on that.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sat, Jan 12, 2008 at 07:36:35PM -0200, Arnaldo Carvalho de Melo escreveu:

Fixed'n'pushed, now I'll wait for your comments, I guess everything is right now when just changing the word-size, and should be OK mostly apart from unusual alignment rules.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Ok, sorry - didnt see that until now; dont worry about it for now. The fix is good enough for me as is; i just changed #if 1 to 0.

youre more than likely right; i think that libel will have all the details somewhere ... and reading from it would be the better option since you dont have to maintain that.

I am going to pull your latest change and test.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Just one bug to report: you dont adjust your tail padding when the size changes from native; example:

------

hadi@lilsol:~/pahole-tests$ pahole -C s4 ./a.out
struct s4 {
        char                       a;                 /*     0     1 */

        /* XXX 3 bytes hole, try to pack */

        double                     b;                /*     4     8 */
        char                       c;                /*    12     1 */

        /* size: 16, cachelines: 1 */
        /* sum members: 10, holes: 1, sum holes: 3 */
        /* padding: 3 */
        /* last cacheline: 16 bytes */
};
hadi@lilsol:~/pahole-tests$ pahole -w 8 -C s4 ./a.out
struct s4 {
        char                       a;                 /*     0     1 */

        /* XXX 7 bytes hole, try to pack */

        double                     b;                /*     8     8 */
        char                       c;                /*    16     1 */

        /* size: 20, cachelines: 1 */
        /* sum members: 10, holes: 1, sum holes: 7 */
        /* padding: 3 */
        /* last cacheline: 20 bytes */
};

hadi@lilsol:~/pahole-tests$

---------------

In this case the tail padding should be 7.

The rules say:
1) aggregates have alignment equal to that of their most aligned member - which happens to be 8 above
2) aggregates have sizes which are a multiple of their alignment This may result in tail padding - the tail contains a char.

So the padding above should be 7.

Heres something that initially puzzled me:

---

hadi@lilsol:~/pahole-tests$ pahole -C s1 ./a.out
struct s1 {
        short int                  a1;               /*     0     2 */
        short int                  a2;               /*     2     2 */
        short int                  a3;               /*     4     2 */

        /* size: 6, cachelines: 1 */
        /* last cacheline: 6 bytes */
};
hadi@lilsol:~/pahole-tests$ pahole -C s1 -w 8 ./a.out
struct s1 {
        short int                  a1;               /*     0     2 */
        short int                  a2;               /*     2     2 */
        short int                  a3;              /*     4     2 */

        /* size: 6, cachelines: 1 */
        /* last cacheline: 6 bytes */
};

------------

It puzzled me initially because i thought we'd need a tail padding of some sort for both 32 bit and 64 bit - but it turns out from rule #1 that the most aligned member was 2 bytes, so no padding was needed for either. So whatever was done here to follow the rules is key.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sun, Jan 13, 2008 at 02:31:06PM -0200, Arnaldo Carvalho de Melo escreveu:

Should be fixed now for non-packed structs, please ACK.

Regards,

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sun, Jan 13, 2008 at 04:07:30PM -0200, Arnaldo Carvalho de Melo escreveu:

Oops, I concentrated on the tail padding, have to check how we're handling rule number 1, but now we infrastructure is there, should be easy.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Em Sun, Jan 13, 2008 at 04:19:01PM -0200, Arnaldo Carvalho de Melo escreveu:

Looks OK:

[acme@doppio examples]$ file align
align: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.9, not stripped [acme@doppio examples]$ ../build/pahole align

struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 7 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     8     4 */
		struct {
			long unsigned int c;             /*    16     8 */
		};                                       /*    16     8 */
	};                                               /*     8    16 */
	char                       a;                    /*    24     1 */

	/* size: 32, cachelines: 1 */
	/* sum members: 18, holes: 1, sum holes: 7 */
	/* padding: 7 */
	/* last cacheline: 32 bytes */
};

[acme@doppio examples]$ ../build/pahole -w4 align

struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */
		struct {
			long unsigned int c;             /*     8     4 */
		};                                       /*     8     4 */
	};                                               /*     4     8 */
	char                       a;                    /*    12     1 */

	/* size: 16, cachelines: 1 */
	/* sum members: 10, holes: 1, sum holes: 3 */
	/* padding: 3 */
	/* last cacheline: 16 bytes */
};

Looks ok when going from 32-bit to 64-bit

[acme@doppio examples]$ gcc -m32 -g align.c -o align [acme@doppio examples]$ file align
align: ELF 32-bit LSB executable, Intel 80386, version 1 (SYSV), dynamically linked (uses shared libs), for GNU/Linux 2.6.9, not stripped

[acme@doppio examples]$ ../build/pahole align

struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */
		struct {
			long unsigned int c;             /*     8     4 */
		};                                       /*     8     4 */
	};                                               /*     4     8 */
	char                       a;                    /*    12     1 */

	/* size: 16, cachelines: 1 */
	/* sum members: 10, holes: 1, sum holes: 3 */
	/* padding: 3 */
	/* last cacheline: 16 bytes */
};

[acme@doppio examples]$ ../build/pahole -w8 align

struct foo {
	char                       l;                    /*     0     1 */

	/* XXX 3 bytes hole, try to pack */

	struct {
		unsigned int       b;                    /*     4     4 */

		/* XXX 4 bytes hole, try to pack */

		struct {
			long unsigned int c;             /*    12     8 */
		};                                       /*    12     8 */
	};                                               /*     4    16 */

	/* Bitfield combined with next fields */

	char                       a;                    /*    16     1 */

	/* size: 24, cachelines: 1 */
	/* sum members: 14, holes: 1, sum holes: 3 */
	/* padding: 7 */
	/* last cacheline: 24 bytes */
};

But not when going from 32-bit to 64-bit, sigh :-\

And there is another bug, the bitfield combining code got confused, because the offset for 'a' is not at 20 as it should even when not correctly aligning the outernmost unnamed struct.

Will fix.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

packed structs _always_ have alignment = 1 afaik.

I updated tree and recompiled - retested and all looks ok to me ....

got same results here. Other than that annoyance the results look correct.

Note: So far what you have seems to work well (sans the little bug above) for say x86 32<->64. The bizare part imo is the alignment difference with different archs (eg -w 4 for ppc vs ia32 would result in different holes/paddings).
Q: Wouldnt it be better to just build arrays/tables of all the rules as per eabi? i.e you have a sizeof and alignment rows and then you can do table lookup of arch/type where key could infact be a dwarf tag. What that would mean is to have cli interface for --arch ppc32 or --arch mips64 etc. If you do that, then allow users to also add new archs for you.

Ok, let me know.

cheers,
jamal

-
To unsubscribe from this list: send the line "unsubscribe dwarves" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html