romcc issue with bitfields and chars in static.c

[Martin Roth]

I've run into what looks like an issue with how romcc handles bitfields 
and chars vs how the structure in static.c is created.
When I'm accessing a member of a structure that comes after a bitfield 
or a single char  between dwords or pointers, the code is grabbing data 
from a different address than the actual location of the data.  I've 
found that if I add extra "reserved" bits to the bitfield to fill it up 
to a dword, and extra chars to do the same, everything works as expected.

I'm currently running into this trying to access data from the 'device' 
structure from static.c in romstage.  If I don't add any changes, I'm  
getting a null pointer returned because I'm trying to access my data 6 
bytes too early: mov esi,DWORD PTR [eax+0x86]

Here's what the structure currently looks like:

unsigned int    enabled : 1;    /* set if we should enable the 
device */
     unsigned int    initialized : 1; /* set if we have initialized the 
device */
     unsigned int    on_mainboard : 1;
     struct pci_irq_info pci_irq_info[4];
     u8 command;
...
     ROMSTAGE_CONST void *chip_info;

Here's how I need to fix it to access chip_info correctly:
     unsigned int    enabled : 1;    /* set if we should enable the 
device */
     unsigned int    initialized : 1; /* set if we have initialized the 
device */
     unsigned int    on_mainboard : 1;
     unsigned int    reserved : 29;
     struct pci_irq_info pci_irq_info[4];
     u8 command;
     u8 res_1;
     u8 res_2;
     u8 res_3;
..
     ROMSTAGE_CONST void *chip_info;

This gives me the correct disassembly:  mov esi,DWORD PTR [eax+0x8c]
and I pick up the pointer to chip_info correctly.  Note that with these 
changes, the structure in memory doesn't change at all.

TLDR: Structure is getting set up as dword aligned, but accessed by 
romcc as if it were packed.

Martin

[ron minnich]

On Mon, Jan 21, 2013 at 8:58 PM, Martin Roth  wrote:
> I've run into what looks like an issue with how romcc handles bitfields and
> chars vs how the structure in static.c is created.
> When I'm accessing a member of a structure that comes after a bitfield or a
> single char  between dwords or pointers, the code is grabbing data from a
> different address than the actual location of the data.  I've found that if
> I add extra "reserved" bits to the bitfield to fill it up to a dword, and
> extra chars to do the same, everything works as expected.
>
> I'm currently running into this trying to access data from the 'device'
> structure from static.c in romstage.  If I don't add any changes, I'm
> getting a null pointer returned because I'm trying to access my data 6 bytes
> too early: mov esi,DWORD PTR [eax+0x86]
>
> Here's what the structure currently looks like:
> unsigned int    enabled : 1;    /* set if we should enable the device */
> unsigned int    initialized : 1; /* set if we have initialized the
>
> device */
>     unsigned int    on_mainboard : 1;
>     struct pci_irq_info pci_irq_info[4];
>     u8 command;
> ...
>     ROMSTAGE_CONST void *chip_info;
>
> Here's how I need to fix it to access chip_info correctly:
>     unsigned int    enabled : 1;    /* set if we should enable the device */
>     unsigned int    initialized : 1; /* set if we have initialized the
> device */
>     unsigned int    on_mainboard : 1;

Actually the :1 stuff is old-school for "bool". Bit-fields were
proclaimed a botch by their creator; here is another examply why. I
would feel better if we just got rid of
the :1 completely and kept it simple. I doubt the storage we save is
worth the effort.

ron

[Martin Roth]

Bleh.  This was a false path - it had nothing to do with romcc.  The 
issue was a #pragma pack(1) in one of the .h files.  Please see my 
subsequent commit for more information.

http://review.coreboot.org/#/c/2190/

Martin

On 01/21/2013 09:58 PM, Martin Roth wrote:
> I've run into what looks like an issue with how romcc handles 
> bitfields and chars vs how the structure in static.c is created.
> When I'm accessing a member of a structure that comes after a bitfield 
> or a single char  between dwords or pointers, the code is grabbing 
> data from a different address than the actual location of the data.  
> I've found that if I add extra "reserved" bits to the bitfield to fill 
> it up to a dword, and extra chars to do the same, everything works as 
> expected.
>
> I'm currently running into this trying to access data from the 
> 'device' structure from static.c in romstage.  If I don't add any 
> changes, I'm  getting a null pointer returned because I'm trying to 
> access my data 6 bytes too early: mov esi,DWORD PTR [eax+0x86]
>
> Here's what the structure currently looks like:
> unsigned int    enabled : 1;    /* set if we should enable the 
>
> device */
>     unsigned int    initialized : 1; /* set if we have initialized the 
> device */
>     unsigned int    on_mainboard : 1;
>     struct pci_irq_info pci_irq_info[4];
>     u8 command;
> ...
>     ROMSTAGE_CONST void *chip_info;
>
> Here's how I need to fix it to access chip_info correctly:
>     unsigned int    enabled : 1;    /* set if we should enable the 
> device */
>     unsigned int    initialized : 1; /* set if we have initialized the 
> device */
>     unsigned int    on_mainboard : 1;
>     unsigned int    reserved : 29;
>     struct pci_irq_info pci_irq_info[4];
>     u8 command;
>     u8 res_1;
>     u8 res_2;
>     u8 res_3;
> ..
>     ROMSTAGE_CONST void *chip_info;
>
> This gives me the correct disassembly:  mov esi,DWORD PTR [eax+0x8c]
> and I pick up the pointer to chip_info correctly.  Note that with 
> these changes, the structure in memory doesn't change at all.
>
> TLDR: Structure is getting set up as dword aligned, but accessed by 
> romcc as if it were packed.
>
> Martin