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

breakpoints inside multi-line var declaration are unsafe #83

Closed
ivarg opened this issue Mar 11, 2015 · 13 comments
Closed

breakpoints inside multi-line var declaration are unsafe #83

ivarg opened this issue Mar 11, 2015 · 13 comments
Labels
Milestone

Comments

@ivarg
Copy link
Contributor

ivarg commented Mar 11, 2015

It is possible to set a breakpoint in a multi-line variable declaration, at which point the variables have not yet been initialized. Calling info locals at that point will render undefined behavior and may result in a panic.

@derekparker
Copy link
Member

Sure enough, thanks for reporting this.

@epipho would you have any interest in looking into this? You've taken an interest in the variable evaluation side of things and had some great commits. If not, I'll start digging into it.

The following snippet will reproduce this:

func main() {
        foo := `
i 
am a
multiline
string
`
        fmt.Println(foo)
}

Setting a breakpoint right at foo := and trying to print foo will crash.

@epipho
Copy link
Contributor

epipho commented Mar 12, 2015

I have run into this before.

The bad news is I am not sure what we can do about it as I don't think there is any information in the binary.

Instead of a location expression for the variable location I would expect a location list (http://www.dwarfstd.org/doc/dwarf-2.0.0.pdf section 2.4.6) that would give a set of instruction ranges that the variable is valid for but the binary does not contain a .debug_loc section.

The same problem exists for code that shadows variables

func main() {
   f := "foo"
   {
     f := "bar"
     fmt.Println(f)
   }
   fmt.Println(f)
}

This actually generates two variables, and there does not appear to be any way to disambiguate them.

<2><40>: Abbrev Number: 4 (DW_TAG_variable)
    <41>   DW_AT_name        : f
    <47>   DW_AT_location    : 5 byte block: 9c 11 a8 7f 22     (DW_OP_call_frame_cfa; DW_OP_consts: -88; DW_OP_plus)
    <4d>   DW_AT_type        : <0x2939f>
 <2><55>: Abbrev Number: 4 (DW_TAG_variable)
    <56>   DW_AT_name        : f
    <5c>   DW_AT_location    : 5 byte block: 9c 11 b8 7f 22     (DW_OP_call_frame_cfa; DW_OP_consts: -72; DW_OP_plus)
    <62>   DW_AT_type        : <0x2939f>

Thoughts?

@derekparker
Copy link
Member

@epipho that's unfortunate. I don't see a clear fix other than patching the linker(s) to emit the .debug_loc section, or at the very least submitting an issue to the Go project to include that information.

@epipho
Copy link
Contributor

epipho commented Mar 12, 2015

This weekend I will dig through the go issue tracker to see if this has been brought up. I wonder what the complexity of the change would be to add .debug_log to the linker.

@derekparker
Copy link
Member

@epipho not sure, but I know there's a lot going on with the compiler / linker rewrites. I'll likely post something on golang-dev about this and another Dwarf tag I'd like the linker to emit in the .debug_info section.

A cursory glance through the issue tracker didn't reveal any existing issues surrounding this, but if you dive deeper and find anything definitely let me know.

@epipho
Copy link
Contributor

epipho commented Mar 12, 2015

Which additional tags are you hoping for? For our purposes it might be nice
to have DW_TAG_lexical_block set so we can play nicer with scoping / autos.

On Thu, Mar 12, 2015 at 2:51 PM, Derek Parker notifications@github.com
wrote:

@epipho https://github.com/epipho not sure, but I know there's a lot
going on with the compiler / linker rewrites. I'll likely post something on
golang-dev about this and another Dwarf tag I'd like the linker to emit in
the .debug_info section.

A cursory glance through the issue tracker didn't reveal any existing
issues surrounding this, but if you dive deeper and find anything
definitely let me know.


Reply to this email directly or view it on GitHub
https://github.com/derekparker/delve/issues/83#issuecomment-78565836.

@ivarg
Copy link
Contributor Author

ivarg commented Mar 12, 2015

I don't know if this is related, but after continued testing I get erratic behavior similar to what I reported above, but also on completely valid source lines. For example, breaking in the following function at fmt.Println(a1,a2) and inspecting locals will list only uninitialized a1, i.e. a1 with seemingly random size, len, and content.

func foo() {
    var (
        a1 = []int{1}
        a2 = 2
    )

    //println("")
    fmt.Println(a1, a2)
}

However, by uncommenting the println("") statement, locals will list expected values for both a1 and a2.

@ivarg
Copy link
Contributor Author

ivarg commented Mar 12, 2015

Btw, I get identical behavior in the above example on both darwin and linux (vbox image).

@derekparker
Copy link
Member

@epipho DW_OP_form_tls_address would be nice for getting at the G(oroutine) in thread local storage, since m->procid can be unreliable and it can be difficult to map a scheduler M to a real OS thread.

@ivarg that's interesting... I'll try and play around with that example and check out the asm / debug info it generates, may be clues there. Not sure why a1 would be uninitialized in that example without that erroneous println call.

@ivarg
Copy link
Contributor Author

ivarg commented Mar 12, 2015

Turns out I forgot to build with optimizations disabled. Building with -gcflags "-N" makes this second behavior go away. Sorry for that. Original issue is still valid though.

@derekparker
Copy link
Member

@ivarg no worries, that makes sense. I'll still look into it a bit to see how feasible it would be for Delve to handle programs built without -N.

FWIW dlv run builds with -gcflags="-N -l". Are you building manually and pointing Delve to the binary?

@ivarg
Copy link
Contributor Author

ivarg commented Mar 12, 2015

Yes, when testing I just go install both dlv and my test programs.

@derekparker derekparker added this to the 1.0 milestone Apr 9, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 5, 2015
The new contents of api.Variable are documented in
service/api/types.go. Additionally evaluating an ambiguous variable
name will yeld the values of all variables that name (see issues

Implements go-delve#243, Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 5, 2015
The new contents of api.Variable are documented in
service/api/types.go. Additionally evaluating an ambiguous variable
name will yeld the values of all variables that name (see issues

Implements go-delve#243, Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 5, 2015
The new contents of api.Variable are documented in
service/api/types.go. Additionally evaluating an ambiguous variable
name will yeld the values of all variables that name (see issues
 go-delve#186, go-delve#106, go-delve#83)

Implements go-delve#243, Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 6, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 7, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 10, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 13, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 14, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 15, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 18, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 18, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 18, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 18, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 19, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Oct 29, 2015
aarzilli added a commit to aarzilli/delve that referenced this issue Feb 15, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Feb 17, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Feb 18, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Feb 23, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Feb 29, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 8, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 9, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 15, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 22, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 29, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83
aarzilli added a commit to aarzilli/delve that referenced this issue May 10, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83

# Conflicts:
#	proc/eval.go
#	proc/proc_test.go
aarzilli added a commit to aarzilli/delve that referenced this issue Jun 11, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83

# Conflicts:
#	proc/eval.go
#	proc/proc_test.go
aarzilli added a commit to aarzilli/delve that referenced this issue Jul 3, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83

# Conflicts:
#	proc/eval.go
#	proc/proc_test.go
aarzilli added a commit to aarzilli/delve that referenced this issue Jul 8, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83

# Conflicts:
#	proc/eval.go
#	proc/proc_test.go
aarzilli added a commit to aarzilli/delve that referenced this issue Jul 12, 2016
Evaluating an ambiguous variable results in an empty variable being
returned with all the possible definitions as children.
To disambiguate between the definitions of a variable use 0(varname),
1(varname), etc

Workaround for go-delve#186, go-delve#106, go-delve#83

# Conflicts:
#	proc/eval.go
#	proc/proc_test.go
aarzilli added a commit to aarzilli/delve that referenced this issue Nov 23, 2017
aarzilli added a commit to aarzilli/delve that referenced this issue Nov 28, 2017
aarzilli added a commit to aarzilli/delve that referenced this issue Dec 8, 2017
aarzilli added a commit to aarzilli/delve that referenced this issue Dec 8, 2017
@aarzilli
Copy link
Member

aarzilli commented Mar 3, 2018

As far as I can tell this is not unsafe anymore.

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

No branches or pull requests

4 participants