golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.39k stars 17.71k forks source link

struct alloc 2 bytes for uint8, when there is other field uint16 #3274

Closed gopherbot closed 9 years ago

gopherbot commented 12 years ago

by efekty:

What steps will reproduce the problem?
package main
import(
    "unsafe"
    "fmt"
)
type ZZZ struct { a uint8;  b uint16; c uint8;  d uint8; e uint16 }  
//unsafe receive "a" 2 bytes[NOT OK],"c" 1byte,"d" 1byte

func main() {
        var z ZZZ = ZZZ{a:1, b:2, c:3, d:4, e:5}
        size := int(unsafe.Sizeof(z))
        fmt.Printf("# unsafe.Sizeof(z): %d\n", size)
        fmt.Printf("# sum  Sizeof(z.x): %d\n", 
            int(unsafe.Sizeof(z.a))+
            int(unsafe.Sizeof(z.b))+
            int(unsafe.Sizeof(z.c))+
            int(unsafe.Sizeof(z.d))+
            int(unsafe.Sizeof(z.e)))
        for i:= 0; i<size; i++ {
            fmt.Printf("%2d", *(*byte)(unsafe.Pointer(uintptr(unsafe.Pointer(&z)) + uintptr(i))))
        }
} 

What is the expected output?
# unsafe.Sizeof(z): 7
# sum  Sizeof(z.x): 7
 1 2 0 3 4 5 0

What do you see instead?
# unsafe.Sizeof(z): 8
# sum  Sizeof(z.x): 7
 1 0 2 0 3 4 5 0

Which compiler are you using (5g, 6g, 8g, gccgo)?
8g

Which operating system are you using?
32 bit XP

Which revision are you using?  (hg identify)
go.weekly.2012-03-04.windows-386.msi

Please provide any additional information below.
I'm trying to do a winapi for gdi32.dll, but this fails[hurd to debug]:
type BITMAPFILEHEADER struct {
    BfType      uint16
    BfSize      uint32
    BfReserved1 uint16
    BfReserved2 uint16
    BfOffBits   uint32
}

/*
http://msdn.microsoft.com/en-us/library/dd183374%28v=vs.85%29.aspx
typedef struct tagBITMAPFILEHEADER {
        WORD    bfType;
        DWORD   bfSize;
        WORD    bfReserved1;
        WORD    bfReserved2;
        DWORD   bfOffBits;
} BITMAPFILEHEADER, FAR *LPBITMAPFILEHEADER, *PBITMAPFILEHEADER;
*/
minux commented 12 years ago

Comment 1:

Fields in a struct has alignment requirements, 
see http://tip.golang.org/ref/spec#Size_and_alignment_guarantees
for details.
uint16 has alignment of 2 bytes, so after uint8, there must be a 1-byte pad
to meet the uint16 requirement.
Should we provide the ability to defined packed structs?
gopherbot commented 12 years ago

Comment 2 by efekty:

I don't understand go alignment. Try this:
type ZZZ struct { a uint32;  b uint64; c uint32;  d uint32; e uint64 }
I receive:
# unsafe.Sizeof(z): 28
# sum  Sizeof(z.x): 28
 1 0 0 0 2 0 0 0 0 0 0 0 3 0 0 0 4 0 0 0 5 0 0 0 0 0 0 0
Form dll (Ansi C, C++) by unsafe, I have no additional space.
gopherbot commented 12 years ago

Comment 3 by efekty:

C++ have __unaligned keyword
http://msdn.microsoft.com/en-us/library/ms253949.aspx
http://msdn.microsoft.com/en-us/library/ms253978.aspx
What is the golang way ?
One of the problem is to save bitmaps BITMAPFILEHEADER to file, the second is to send it
to/from dll by syscall winapi way.
dsymonds commented 12 years ago

Comment 4:

Go does not provide packed structures in that way. If you need to parse something in
that format you will need to slice up the bytes yourself.

Status changed to WorkingAsIntended.

gopherbot commented 12 years ago

Comment 5 by efekty:

C++ info:
// http://msdn.microsoft.com/en-us/library/dd145119%28v=vs.85%29.aspx
// Copy the BITMAPFILEHEADER into the .BMP file.  
WriteFile(hf, (LPVOID) &hdr, sizeof(BITMAPFILEHEADER), (LPDWORD) &dwTmp,  NULL)
sizeof(BITMAPFILEHEADER): 14
sizeof(hdr):16

golang needs:
1. I have no sizeof(BITMAPFILEHEADER). How to do this ?
2. What is the golang way to WriteFile of struct, but not to copy field, by field ? Is
there a struct.Writer ?
alexbrainman commented 12 years ago

Comment 6:

You were pointed to Go alignment rules:
http://tip.golang.org/ref/spec#Size_and_alignment_guarantees. If you reread them you
will see that 
2. For a variable x of struct type: unsafe.Alignof(x) is the largest of all the values
unsafe.Alignof(x.f) for each field f of x, but at least 1.
For your type of
type BITMAPFILEHEADER struct {
    BfType      uint16
    BfSize      uint32
    BfReserved1 uint16
    BfReserved2 uint16
    BfOffBits   uint32
}
all fields BfType, BfSize, BfReserved, BfReserved2 and BfOffBits will be allinged on 4
bytes (largest of all the values (uint16 and uint32)). So BfType will start at 0 bytes
offset, BfSize at 4, BfReserved at 8, BfReserved2 at 12 and BfOffBits at 16.
Go does not have support for "not alligned" struct. You have to deal with it yourself,
if outside world forces you to.
For example, if you want to write value of "non-alligned" BITMAPFILEHEADER struct, then
just write all fields of it one by one: write uint16 BfType, then uint32 BfSize, then
uint16 BfReserved1, then uint16 BfReserved6, then uint32 BfOffBits. There is nothing
wrong with writing to the file field by field, just make sure that you output is
buffered.
Similar, if you need to pass pass this structire to Windows api, pack all
BITMAPFILEHEADER struct fields into array of bytes (var b [14]byte), and pass pointer to
that array. Your BfType will go to b[0:1], BfSize to [2:5] and so on.
Alex
PS: I haven't come accross "unalligned" Windows structures yet, but, I beleive you, if
you say BITMAPFILEHEADER is. I didn't check it.
gopherbot commented 12 years ago

Comment 7 by efekty:

YES, I understand the alignment. Thx.
But I wonder of the "first Comment" question "Should we provide the ability to defined
packed structs?"
Maybe not ... in my case(golang winapi), without __unaligned, I stay with no answer for
my "fifth Comment" questions:
1. Howto receive sizeof(BITMAPFILEHEADER) ?
2. Howto receive struct.Writer ?
Sometime copy, by hand, field by field is not acceptable. Sometime copy is not
acceptable.
rsc commented 12 years ago

Comment 8:

No, there are no packed structs.  Sorry.
gopherbot commented 12 years ago

Comment 9 by efekty:

OK, I found offset in reflect package. Thanks. 
So this issue shoud stay as my request todo it sipler, faster :) 
func AlignmentDeltaofStruct(value interface{}) (delta int){
var offset  int = 0
var size    int = 0
t := reflect.TypeOf(value)
v := reflect.ValueOf(value)
if t.Kind() != reflect.Struct { 
    panic("# TODO 1: not a struct")
}
for i := 0; i < v.NumField(); i++ {
    fv := v.Field(i)    
    ft:= t.Field(i)
    switch ft.Type.Kind() {
    case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, 
    reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, 
    reflect.Uintptr, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128:
        var foffset int = int(ft.Offset)
        delta += foffset - offset  - size
        offset = foffset
        size = int(fv.Type().Size()) // the same us unsafe
//  case ...TODO:
    case reflect.Struct:
        delta+=AlignmentDeltaofStruct(fv.Interface())
    default:
        panic("# TODO 2: not implemented yet")
    }
}
return
}
alexbrainman commented 12 years ago

Comment 10:

You can also change BITMAPFILEHEADER struct definition to
type BITMAPFILEHEADER struct {
    BfType  uint16
    BfSize1 uint16
    BfSize2 uint16
    BfReserved1 uint16
    BfReserved2 uint16
    BfOffBits1  uint16
    BfOffBits2  uint16
}
This way all fields will be aligned properly. You just have to combine BfSize1 and
BfSize2 field pair (and BfOffBits1/BfOffBits2) into single value before you use it and
vice verse.
Alex
gopherbot commented 12 years ago

Comment 11 by efekty:

The problem is not todo it manualy. 
1. When we want to "translate" a lot of struct like winapi, I prefer to be lasy and test
it. 
I have no "len" of struct. How nice to have it in reflect package :)
2. As You can see in "5 Comment", in C++ WriteFile example, MS dll :(
 - the struct must to be with alignment, sizeof(hdr)==16 
 - and I must send the "len", sizeof(BITMAPFILEHEADER)==14
3. The next problem is to have struct in ByteBuffer, operate by big inherit struct, not
to copy. 
I don't know how to do it without __unaligned.
Realy unsafe errors:
package main
import(
    "unsafe"
    "fmt"
)
type ZZZ struct { a uint16;  b uint32; c uint16;  d uint16; e uint32 }
func main() {
        var z ZZZ = ZZZ{a:1, b:2, c:3, d:4, e:5}
        size := int(unsafe.Sizeof(z))
        fmt.Printf("# unsafe.Sizeof(z): %d\n", size)

        var b = make([]byte, 16)
        b[0]=1 //2
        b[2]=9 // saved in alignment :(
        b[4]=2 //4
        b[8]=3 //2
        b[10]=4 //2
        b[12]=5 //4
        p:=unsafe.Pointer(&b[0])
        var zz *ZZZ
        zz = (*ZZZ)(p)
        fmt.Printf("# zz = %+v\n", zz)
        bb := make([]byte, 12)
        bb[0]=1 //2
//      bb[x]=9 // error, forgot about alignment
        bb[2]=2 //4
        bb[6]=3 //2
        bb[8]=4 //2
        bb[10]=5 //4 
        pp:=unsafe.Pointer(&bb[0])
        var zzz *ZZZ
        zzz = (*ZZZ)(pp)
        fmt.Printf("# zzz = %+v\n", zzz)
} 
Output:
# unsafe.Sizeof(z): 16
# zz = &{a:1 b:2 c:3 d:4 e:5}
# zzz = &{a:1 b:196608 c:4 d:5 e:0}
__________________________________________________________________
So I vote to __unaligned, or wait for Your advice. 
(I know it is posible with reflection, but copy and not so easy :)
ianlancetaylor commented 12 years ago

Comment 12:

Go does not support defining a struct that precisely mirrors an externally defined
memory layout.  If you need to use an externally defined memory layout, and you want to
access it as a Go struct, then in some cases you need to write code to convert the data
between the externally defined representation and the Go struct.
Simply adding "unaligned" or "packed" to Go would address your specific issue but it
would not address the general case of the problem.
alexbrainman commented 12 years ago

Comment 13:

This program shows one way to do it. I do not think it is very complicated. This
program, unlike yours, will also work on any cpu.
package main
import (
    "bytes"
    "encoding/binary"
    "fmt"
    "io"
    "log"
)
type ZZZ struct {
    a uint16
    b uint32
    c uint16
    d uint16
    e uint32
}
var enc = binary.LittleEndian
// WriteTo writes contents of z into io.Writer w.
func (z *ZZZ) WriteTo(w io.Writer) error {
    if e := binary.Write(w, enc, z.a); e != nil {
        return e
    }
    if e := binary.Write(w, enc, z.b); e != nil {
        return e
    }
    if e := binary.Write(w, enc, z.c); e != nil {
        return e
    }
    if e := binary.Write(w, enc, z.d); e != nil {
        return e
    }
    if e := binary.Write(w, enc, z.e); e != nil {
        return e
    }
    return nil
}
// ReadFrom reads contents of z from io.Reader r.
func (z *ZZZ) ReadFrom(r io.Reader) error {
    if e := binary.Read(r, enc, &z.a); e != nil {
        return e
    }
    if e := binary.Read(r, enc, &z.b); e != nil {
        return e
    }
    if e := binary.Read(r, enc, &z.c); e != nil {
        return e
    }
    if e := binary.Read(r, enc, &z.d); e != nil {
        return e
    }
    if e := binary.Read(r, enc, &z.e); e != nil {
        return e
    }
    return nil
}
func main() {
    buf := new(bytes.Buffer)
    z := ZZZ{a: 1, b: 2, c: 3, d: 4, e: 5}
    fmt.Printf("z=%+v\n", z)
    if e := z.WriteTo(buf); e != nil {
        log.Fatal(e)
    }
    fmt.Printf("buf=%+v\n", buf.Bytes())
    var z2 ZZZ
    if e := z2.ReadFrom(buf); e != nil {
        log.Fatal(e)
    }
    fmt.Printf("z2=%+v\n", z2)
}
gopherbot commented 12 years ago

Comment 14 by efekty:

Again it's manual copy. 
My example is not doing the righ work. It only show that I can't use simple (*ZZZ)(p).
One another example is to do a package analizer, concurency.
That is why it looklike: https://github.com/akrennmair/gopcap/blob/master/decode.go
IPPACKET(ETHERNETHAEDER...(IPHEADER...(TCPHEADER...(HTMLHEADER...)))) is the only one
leaf of the tree.
It can be so simple with __unaligned but here is a monster.
Convert(not copy) from struct to []byte is not posible in place of alignment.
I must reinvent the wheel or simply stay with c :( No.
I know Golang struct mast be aligned in general. 
So nice to have addon "unaligned struct" keyword :)
_____________________________________________________________
Is it unposible to implement it in "experimental" of Golang ?
Tell me the "closest" place in code where is it ?
rsc commented 12 years ago

Comment 15:

I don't know how many times we can tell you that Go does not and will not support this.
The issue tracker is not a forum.  If you want to continue this discussion, please do it
on the golang-nuts@ mailing list.
gopherbot commented 12 years ago

Comment 16 by efekty:

Thanks. 
It's enough to know You want support this :)
I don't want to start general discussion. You found bypass for me (9 Comment). 
Sorry too much, too far ..., discussion here. My fault.
Good work.