cmd/link/internal/ld: abstract origin missing local variable

[FiloSottile]

0xe6f2: Subprogram
at=AbstractOrigin val=0x1273d4
at=Lowpc val=0x618bc0
at=Highpc val=0x618c27
at=FrameBase val=0x9c
--- FAIL: TestAbstractOriginSanity (10.00s)
	dwarf_test.go:770: unresolved abstract origin ref in DIE at offset 135569
0xe6f2: Subprogram
at=AbstractOrigin val=0x1273d4
at=Lowpc val=0x618bc0
at=Highpc val=0x618c27
at=FrameBase val=0x9c
--- FAIL: TestAbstractOriginSanityWithLocationLists (11.76s)
	dwarf_test.go:770: unresolved abstract origin ref in DIE at offset 135569
FAIL
FAIL	cmd/link/internal/ld	27.909s

https://storage.googleapis.com/go-build-log/99c11019/linux-amd64_3ca200f0.log

The test has been failing on the BoringCrypto branch since it was introduced in fdecaa8 by @thanm, supposedly due to the BoringSSL syso object.

[thanm]

Let me know if/how I can help.

[FiloSottile]

Traced this to a Go bug unrelated to BoringCrypto thanks to @heschik.

The DIEs refer to this function, crypto/sha1.New from dev.boringcrypto:

func New() hash.Hash {
	if boringEnabled {
		return boringNewSHA1()
	}
	d := new(digest)
	d.Reset()
	return d
}

Here boringEnabled resolves (across packages) to a true constant, so the rest of the function body, including the local variable, gets eliminated as dead code.

It results in this DW_ABRV_FUNCTION_ABSTRACT DIE, with no local variables:

0x00127364:     TAG_subprogram [3] *
                 AT_name( "crypto/sha1.New" )
                 AT_inline( DW_INL_inlined )
                 AT_external( 0x01 )

0x00127377:         NULL

0x00127378:     TAG_subprogram [3] *
                 AT_name( "crypto/sha1.boringNewSHA1" )
                 AT_inline( DW_INL_inlined )
                 AT_external( 0x01 )

0x00127395:         NULL

It's referenced correctly in a few places as a DW_ABRV_INLINED_SUBROUTINE_RANGES:

0x00128aee:         TAG_inlined_subroutine [6] *
                     AT_abstract_origin( {0x0000000000127364}"crypto/sha1.New" )
                     AT_ranges( 0x0006ee90
                         Base address = 0xffffffffffffffff68f2d0
                         End )
                     AT_call_file( "/usr/local/google/home/valsorda/go/src/crypto/tls/cipher_suites.go" )
                     AT_call_line( 133 )

The problem is when it's referenced as a DW_ABRV_FUNCTION_CONCRETE:

0x00151a6f:     TAG_subprogram [4] *
                 AT_abstract_origin( {0x0000000000127364} )
                 AT_low_pc( 0x000000000066bb80 )
                 AT_high_pc( 0x000000000066bbe7 )
                 AT_frame_base( call-frame-cfa )

0x00151a86:         TAG_variable [12]
                     AT_abstract_origin( {0x0000000000127377} )
                     AT_location(  )

0x00151a8c:         TAG_formal_parameter [14]
                     AT_name( "~r0" )
                     AT_variable_parameter( 0x01 )
                     AT_decl_line( 120 )
                     AT_type( {0x000000000001031f} ( hash.Hash ) )
                     AT_location(  )

0x00151a98:         TAG_inlined_subroutine [6] *
                     AT_abstract_origin( {0x0000000000127378} )
                     AT_ranges( 0x00067700
                         Base address = 0xffffffffffffffff66bb80
                         End )
                     AT_call_file( "/usr/local/google/home/valsorda/go/src/crypto/sha1/sha1.go" )
                     AT_call_line( 122 )

[...]

This time you can see it has a local variable, possibly d, which references the non-existing offset 0x0000000000127377, supposedly where the local variable abstract origin was going to be. The test catches this.

The rest of the DIE reflects just the boringNewSHA1 call, so it is in some way aware of the dead code elimination.

Here's a dwarfinl debug snippet:

assembling DWARF inlined routine info for "".New
  0: II:24 ("".boringNewSHA1) V: ( ) C: ( 1 ) R:
    1: II:25 (crypto/internal/boring.NewSHA1) V: ( h ) C: ( 2 ) R: [29,55)
      2: II:26 (crypto/internal/boring.(*sha1Hash).Reset) V: ( h ) C: ( ) R: [55,64)
V0: h CI:0 II:26 IA:1 param
V1: h CI:0 II:25 IA:1 local
V2: d CI:0 II:-1 IA:1 local
V3: ~r0 CI:1 II:-1 IA:0 param
PutAbstractFunc(go.info."".New$abstract)
putAbstractScope(go.info."".New): vars: 0:d
PutConcreteFunc(go.info."".New)
putscope(go.info."".New,0): vars: 0:0:d 1:1:~r0
PutInlinedFunc(caller=go.info."".boringNewSHA1$abstract,callee=go.info."".boringNewSHA1$abstract,abbrev=6)
PutInlinedFunc(caller=go.info.crypto/internal/boring.NewSHA1$abstract,callee=go.info.crypto/internal/boring.NewSHA1$abstract,abbrev=5)
PutInlinedFunc(caller=go.info.crypto/internal/boring.(*sha1Hash).Reset$abstract,callee=go.info.crypto/internal/boring.(*sha1Hash).Reset$abstract,abbrev=5)

The same happens with crypto/sha512.New.

@thanm, assigning this to you, but let me know if you can't work on it, or if you need help reproducing it.

Also, your dwarf-check tool does not catch this because it does not return an error when Next encounters a NULL at the Offset instead of a DIE.

[thanm]

Thanks for the analysis. In theory this scenario is supposed to be handled by the existing design (e.g. dead coding of locals) but clearly it's not working quite right. I will take a closer look tomorrow.

[thanm]

The function of interest in this bug has a variable "d" that gets deleted by dead code elimination; dead variables are (in theory) supposed to be handled cleanly by the DWARF inline generation design, but in this case things are breaking down.

There is a crude dead-code phase (source code in typecheck.go) invoked in gc.Main immediately after type checking; this particular phase will catch things like

  if true {
     ...
  }

however it will not detect things like

  var alwaystrue bool = true
  if x != nil {
     alwaystrue = true
  }
  if alwaystrue {
     return 2
  }
  var iamdead [128]int
  ...

The case above is easily handled by the SSA backend, so 'iamdead' will not survive to the final compiled version of the function. This sort of dead variable doesn't cause any issues for the DWARF code.

The problem here is that in the early deadcode phase, the typecheck.go code eliminates statically unreachable PAUTO nodes from the AST, but it doesn't delete them from the function's "Dcl" list, meaning that the DWARF inlining code still sees them (and emits references to the dead variables).

When export data is generated for a package, however, the export data writer looks at the AST after the deadcode() call invoked by gc.Main(). This means that the version of sha1.New emitted into export data has no locals, which in turn means that some other package that imports "sha1" and invokes New will emit an abstract function for sha1.New that has no locals (since the Dcl list for the inlineable function is reconstructed by the importer).

I think the right way to handle this is to add some code to check for unused PAUTO's (e.g. those that are statically unreachable, like "d" in sha1.New) and not process them in the DWARF code at all. The question is where to do that checking. The typechecker's deadcode phase is a bit too special-purpose for this IMHO; I will look into doing this as part of the inline analysis.

[gopherbot]

Change https://golang.org/cl/114090 mentions this issue: compiler: fix DWARF inline debug issue with dead local vars

[thanm]

Thank you also for reporting the problem with dwarf-check. That issue is fixed here.