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

cmd/compile: framepointer + duffzero generates weird (+bad?) code #16013

Closed
randall77 opened this issue Jun 8, 2016 · 5 comments
Closed

cmd/compile: framepointer + duffzero generates weird (+bad?) code #16013

randall77 opened this issue Jun 8, 2016 · 5 comments
Milestone

Comments

@randall77
Copy link
Contributor

func f(i byte) byte {
    var x [256]byte
    return x[i]
}

Without framepointers, the zeroing of x is done with:

    205b:       48 8d 3c 24             lea    (%rsp),%rdi
    205f:       0f 57 c0                xorps  %xmm0,%xmm0
    2062:       e8 7d 60 04 00          callq  480e4 <runtime.duffzero+0xe4>

With framepointers on, however, there's some extra weird code on there:

    206e:       48 8d 3c 24             lea    (%rsp),%rdi
    2072:       0f 57 c0                xorps  %xmm0,%xmm0
    2075:       48 89 6c 24 f0          mov    %rbp,-0x10(%rsp)
    207a:       48 8d 6c 24 f0          lea    -0x10(%rsp),%rbp
    207f:       e8 20 9d 04 00          callq  4bda4 <runtime.duffzero+0xe4>
    2084:       48 8b 6d 00             mov    0x0(%rbp),%rbp

I'm not sure what this code is about or where in the compiler it comes from. I'm investigating. Some sort of pretend framepointer link for duffzero? It doesn't seem necessary, duffzero is a leaf routine.
The fact that it is writing on the wrong side of %rsp scares me.

Also check duffcopy.

@aclements

@randall77 randall77 added this to the Go1.7 milestone Jun 8, 2016
@randall77 randall77 self-assigned this Jun 8, 2016
@randall77
Copy link
Contributor Author

Found it. There is this code (cmd/internal/obj/x86/asm6.go):

                    // Maintain BP around call, since duffcopy/duffzero can't do it
                    // (the call jumps into the middle of the function).
                    // This makes it possible to see call sites for duffcopy/duffzero in
                    // BP-based profiling tools like Linux perf (which is the
                    // whole point of obj.Framepointer_enabled).
                    // MOVQ BP, -16(SP)
                    // LEAQ -16(SP), BP
                    ctxt.AsmBuf.Put(bpduff1)

I'm not sure why this is necessary. Leaf routines have the same issue (they could, but don't, do any BP adjustment) and we don't insert this special code for them.

@aclements
Copy link
Member

Well, it's necessary for the reason the comment says. We need a BP if you want to see who's calling duffcopy/duffzero.

For leaf routines, I thought it was just functions with a zero-sized frame that didn't do BP adjustment, which is a subset of leaf functions, but not all of them. I consider not doing BP adjustment for these a bug. It's just an unfortunate consequence of our rule to avoid adding BPs to sensitive assembly functions. But for these we still could do BP adjustment in the function prologue, but we really can't do BP adjustment in duffcopy/duffzero.

@randall77
Copy link
Contributor Author

Right, it's just frameless leaf functions that don't do BP adjustment.

Do we have to worry about signal handlers? They could clobber values on the wrong side of SP. Or do we always run with a sigaltstack set up?

@ianlancetaylor
Copy link
Contributor

We always run with an alternate signal stack (otherwise receiving the signal would overrun the small goroutine stack).

@randall77
Copy link
Contributor Author

Ok, I'll close this then. Ugly, but probably not buggy.

@golang golang locked and limited conversation to collaborators Jun 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants