Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fmt: formats struct pointer unexpectedly and/or the go doc of package fmt is ambigious #27768

Open
leixure opened this issue Sep 20, 2018 · 10 comments

Comments

@leixure
Copy link

@leixure leixure commented Sep 20, 2018

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.11 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"

What did you do?

https://play.golang.org/p/YLjGTapBvsg

What did you expect to see?

go doc fmt

Pointer:

%p base 16 notation, with leading 0x
The %b, %d, %o, %x and %X verbs also work with pointers,
formatting the value exactly as if it were an integer.

According to the above statement, I am expecting %d formats a pointer which points to address 0x8000 as 32768.

What did you see instead?

%d formats the pointer to a struct as &{<space-separated-field-values>}, and all the fields are formatted with %d.

This behavior also applies to %b, %o, %x and %X.

Basically, for a pointer p who points to a struct, fmt.Printf("%[bdoxX]", p) is equivalent to fmt.Printf("&%[bdoxX]", *p). That is counterintuitive, because I'd always expect to get the address of the pointer. Just like a pointer p that points to an integer or string, fmt.Printf always formats the address of p, instead of its pointed value. The inconsistency here between a struct pointer and the aforementioned pointer type feels odd.

If people really need a struct pointer be formatted the way those flags currently do, they can always get the same result with fmt.Printf("&%[bdoxX]", *p); but with the current implementation, to get the decimal format of a struct pointer pointed address, one has to call fmt.Printf("%d", reflect.ValueOf(p).Pointer()) or fmt.Printf("%d", strconv.ParseUint(fmt.Sprintf("%p", p), 0, 0)). Both are insane.

I am confronted with this issue because my go program needs to exchange data with a C program, which uses the addresses of some go struct instances as the keys in a C hash map. My case may be uncommon, but I believe there could be a lot of other cases where people really just need a different format of the struct pointer pointed address.

@martisch
Copy link
Contributor

@martisch martisch commented Sep 20, 2018

I do not think we should change the current way of formatting as it has been documented and otherwise I do not see how nested structures with pointers can be printed anymore if pointers are always printed as numbers.

fmt applies this rule first:

For compound objects, the elements are printed using these rules, recursively, laid out like this:
pointer to above: &{}, &[], &map[]

fmt does not follow pointers within the struct to not fall into infinite loops chasing pointers.

fmt.Printf("%d", unsafe.Pointer(p)) also for works for your use case.

To not need to specify the conversion in calls to fmt the Point type can implement the Formatter interface:
func (p Point) Format(f fmt.State, c rune) { fmt.Fprintf(f, "{%d %d %d}", p.X, p.Y, reflect.ValueOf(p.Ref).Pointer()) }

cc @robpike

@leixure
Copy link
Author

@leixure leixure commented Sep 20, 2018

The I would argue "%d" should get &{1 2 &{3 4 0}} instead of &{1 2 272670928}

@martisch
Copy link
Contributor

@martisch martisch commented Sep 20, 2018

It very well could print &{1 2 &{3 4 0}} per documentation, however the implementation does not perform any cycle detection so therefore bails out at some point. This might be better clarified in the documentation.

@martisch martisch changed the title Package fmt formats struct pointer unexpectedly and/or the go doc of package fmt is ambigious fmt: formats struct pointer unexpectedly and/or the go doc of package fmt is ambigious Sep 20, 2018
@bcmills bcmills added this to the Unplanned milestone Sep 22, 2018
@bcmills
Copy link
Member

@bcmills bcmills commented Sep 22, 2018

I am confronted with this issue because my go program needs to exchange data with a C program, which uses the addresses of some go struct instances as the keys in a C hash map.

Note that the addresses of Go structs may change over time, and are therefore not suitable as keys in a long-lived C hash map.
(See https://golang.org/cmd/cgo/#hdr-Passing_pointers.)

@leixure
Copy link
Author

@leixure leixure commented Sep 22, 2018

I am confronted with this issue because my go program needs to exchange data with a C program, which uses the addresses of some go struct instances as the keys in a C hash map.

Note that the addresses of Go structs may change over time, and are therefore not suitable as keys in a long-lived C hash map.
(See https://golang.org/cmd/cgo/#hdr-Passing_pointers.)

This might be off the topic of this issue, but hopefully you don't mind.

Yeah, I noticed the "do not save a go pointer in C" statement when reading the cgo doc, but still confused. Is it just "generally speaking but there're some special cases" or "it's true as it's part of the language specification and even if it seems to work it's still wrong and definitely may break in some future version"?

For example, If I know a go struct instance persists during the time its address is used as the key of a C hash map, is it still unsafe/wrong to use it that way? Because the memory address of that go struct instance could change during its lifetime?

type Foo struct {
    a,b int
}

type FooMap map[*Foo]int

var foo1, foo2 Foo
var m = FooMap{
    &foo1: 1
    &foo2: 2
}

func valueOf(foo *Foo) {
    return m[foo]
}

Consider the useless code above, if m is never changed after its initialized, can I assume valueOf(&foo1) is 1 and valueOf(&foo2) is 2? Or they may return 0 under some circumstances because the addresses of foo1 and/or foo2 are changed?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 24, 2018

@leixure Your Go example is guaranteed to work; if the Go pointers move the GC will update the map, one way or another. What is not guaranteed to work is building a hash table using Go pointers in C. I think the rules are clear: C code may not keep a copy of a Go pointer. That is true whether you pass the pointer or whether you print it using %p and interpret it on the C side.

(It happens to be the case that the current Go 1.11 implementation does not move pointers, and so holding on to Go pointers in C will work in some cases. But that could break, in various different ways, in any future Go release.)

@cznic
Copy link
Contributor

@cznic cznic commented Sep 24, 2018

It happens to be the case that the current Go 1.11 implementation does not move pointers, ...

I thought Go stacks are movable for quite some time and can contain pointers. Am I mistaken?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 24, 2018

@cznic You are correct, of course, but stack pointers are never passed to C code, precisely because they might move during execution of the C code (if the C code calls back into the Go code).

But you're quite right that if escape analysis got good enough then passing a pointer to fmt.Printf might not cause it to escape, in which case passing that number to C code would not be sufficient to prevent the pointer value from moving while the C code executes. Thanks for the thought.

@bcmills
Copy link
Member

@bcmills bcmills commented Sep 24, 2018

Goroutine stacks are are movable and can contain pointers.
However, pointers passed to C currently cannot be allocated on the stack (#27538).

@cznic
Copy link
Contributor

@cznic cznic commented Sep 24, 2018

Thanks to both for the clarification. My use case elsewhere works hard to avoid this problem, so I'm glad it's not effort wasted. (Handling Go pointers in code translated from C, but without the compiler's support CGo can enjoy)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.