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: redundant checks for nil pointers when calling an inlined method #11483

Closed
jacobsa opened this issue Jun 30, 2015 · 3 comments
Closed

Comments

@jacobsa
Copy link
Contributor

jacobsa commented Jun 30, 2015

(This is an issue about less than ideal generated code. I don't know if the project accepts such issues?)

In real programs, I've observed some redundant checks for nil pointers that I think the compiler should be able to optimize away. They seem to have to do with inlined methods.

Here is a reduced test case that panics due to a nil pointer dereference:

package main

import "bytes"

type Foo struct {
  buf bytes.Buffer
} 

func (f *Foo) size() uint32 {
  return uint32(f.buf.Len())
}

func main() {
  var f *Foo
  f.size()
} 

The segfault looks like this in GDB (with Intel syntax):

Dump of assembler code for function main.main:
   0x0000000000400c00 <+0>:     xor    eax,eax
   0x0000000000400c02 <+2>:     cmp    rax,0x0
   0x0000000000400c06 <+6>:     je     0x400c16 <main.main+22>
   0x0000000000400c08 <+8>:     mov    rbp,QWORD PTR [rax+0x8]
   0x0000000000400c0c <+12>:    mov    rbx,QWORD PTR [rax+0x18]
   0x0000000000400c10 <+16>:    sub    rbp,rbx
   0x0000000000400c13 <+19>:    mov    ebx,ebp
   0x0000000000400c15 <+21>:    ret    
=> 0x0000000000400c16 <+22>:    mov    DWORD PTR [rax],eax
   0x0000000000400c18 <+24>:    jmp    0x400c08 <main.main+8>

The code checks f for being nil, and intentionally segfaults if so. But I believe it doesn't need to: the compiler knows the minimum page size on the architecture, and knows that Foo is less than a page long. Therefore if f is nil, then f+0x8 will be on the same unmapped page as 0x0, and should segfault just the same as the write to 0x0 that the generated code uses. In other words: the branch need not exist, because the instruction at +8 will already segfault if f is nil.

Indeed when a member of Foo is instead read directly, this fact appears to be used:

package main

type Foo struct {
  i uint32
  j uint32
} 

func (f *Foo) size() uint32 {
  return f.j
}

func main() {
  var f *Foo
  f.size()
} 
Dump of assembler code for function main.main:
   0x0000000000400c00 <+0>:     xor    eax,eax
=> 0x0000000000400c02 <+2>:     mov    ebp,DWORD PTR [rax+0x4]
   0x0000000000400c05 <+5>:     ret    

Go version:

% go version
go version devel +3b7841b Mon Jun 29 01:49:05 2015 +0000 linux/amd64
@adg
Copy link
Contributor

adg commented Jun 30, 2015

Sure, we accept these kinds of issues.

@adg adg changed the title Redundant checks for nil pointers when calling an inlined method cmd/compile: redundant checks for nil pointers when calling an inlined method Jun 30, 2015
@josharian
Copy link
Contributor

The work-in-progress SSA backend may help here.

@josharian josharian self-assigned this Jun 30, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Jul 11, 2015
@rsc rsc modified the milestones: Unplanned, Go1.6 Nov 4, 2015
@randall77
Copy link
Contributor

This is fixed with the new SSA backend.

@golang golang locked and limited conversation to collaborators Oct 7, 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

7 participants