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

The test for the package “./pkg/terminal” fails when using -asan or -msan. #2919

Closed
zhangfannie opened this issue Mar 8, 2022 · 6 comments · Fixed by #2922
Closed

The test for the package “./pkg/terminal” fails when using -asan or -msan. #2919

zhangfannie opened this issue Mar 8, 2022 · 6 comments · Fixed by #2922

Comments

@zhangfannie
Copy link

Please answer the following before submitting your issue:

Note: Please include any substantial examples (debug session output,
stacktraces, etc) as linked gists.

  1. What version of Delve are you using (dlv version)?
    git clone https://github.com/go-delve/delve.git

  2. What version of Go are you using? (go version)?
    go version devel go1.19-d9d55724bd Mon Mar 7 23:46:55 2022 +0000 linux/arm64

  3. What operating system and processor architecture are you using?
    linux/arm64

  4. What did you do?
    <1>. download the delve souce code.
    <2>. cd ./pkg/terminal
    <3>. go test -asan -c
    <4>. ./terminal.test

  5. What did you expect to see?
    pass

  6. What did you see instead?
    It reports the error as below.
    fatal error: checkptr: pointer arithmetic computed bad pointer value.
    goroutine 560 [running]:
    runtime.throw({0xd3190a?, 0x204000c98900?})
    /home/fanzha02/work/go_project/gotest/src/runtime/panic.go:992 +0x50 fp=0x204000364e90 sp=0x204000364e60 pc=0x438590
    runtime.checkptrArithmetic(0xe53b68?, {0x0?, 0x204000364f48?, 0x2?})
    /home/fanzha02/work/go_project/gotest/src/runtime/checkptr.go:52 +0xc0 fp=0x204000364ec0 sp=0x204000364e90 pc=0x409c80
    github.com/go-delve/delve/pkg/proc/native.processVmRead(0x204001042018?, 0x10, {0x204001042018?, 0x8, 0x204000364fc8?})
    /home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/native/ptrace_linux_64bit.go:19 +0xc8 fp=0x204000364f70 sp=0x204000364ec0 pc=0x8f6ae8
    github.com/go-delve/delve/pkg/proc/native.(*nativeThread).ReadMemory(0x2040006ca790, {0x204001042018, 0x8, 0x8}, 0x10)
    /home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/native/threads_linux.go:112 +0xc4 fp=0x204000365020 sp=0x204000364f70 pc=0x8fa514
    github.com/go-delve/delve/pkg/proc.readUintRaw({0xe55340, 0x2040006ca790}, 0x204000365128?, 0x8)
    /home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/variables.go:1694 +0x68 fp=0x204000365080 sp=0x204000365020 pc=0x878898
    github.com/go-delve/delve/pkg/proc.getGVariable({0xe5bb80, 0x2040006ca790})
    /home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/variables.go:445 +0x23c fp=0x204000365170 sp=0x204000365080 pc=0x86c53c

@zhangfannie
Copy link
Author

The cause is when using -asan or -msan option, the compiler will set -d=checkptr=1 implicitly, which will instrument runtime.CheckPtrArithmetic for conversions involving unsafe.Pointer.
So the code unsafe.Pointer(addr) in line 19 of the file delve/pkg/proc/native/ptrace_linux_64bit.go is intrumented runtime.checkptrArithmetic for addr pointer, the addr is calucated by regs.TLS() + thread.BinInfo().GStructOffset() in line 445 of the file delve/pkg/proc/variables.go.

I print out the value of addr which causes the test to fail when its value is 16.

I am not familiar with the logical of these codes. can someone help with a look?Thank you.

@aarzilli
Copy link
Member

aarzilli commented Mar 9, 2022

Why is the error reported for pkg/proc/native/ptrace_linux_64bit.go:19 which is:

	p_remote := uintptr(unsafe.Pointer(&remote_iov))

and not for line 17 which contains the unsafe.Pointer conversion of addr:

	remote_iov := sys.Iovec{Base: (*byte)(unsafe.Pointer(addr)), Len: len_iov}

Anyway, this is reading memory from the target process, the value of remote_iov.Base is always wrong for delve's process, except accidentally. Would using a struct like this:

type remoteSysIovec struct {
    base uintptr
    len uint64
}

for remote_iov be better?

@zhangfannie
Copy link
Author

zhangfannie commented Mar 10, 2022

@aarzilli Sorry, I added some logs to the source code that made the line numbers inconsistent. Below is the correct stack trace. And the analysis I mentioned above is correct. Thank you.

fatal error: checkptr: pointer arithmetic computed bad pointer value

goroutine 626 [running]:
runtime.throw({0xd31749?, 0x40fa00?})
/home/fanzha02/work/go_project/gotest/src/runtime/panic.go:992 +0x50 fp=0x204000362f10 sp=0x204000362ee0 pc=0x438590
runtime.checkptrArithmetic(0x3?, {0x0?, 0xffff8e9556e0?, 0x20?})
/home/fanzha02/work/go_project/gotest/src/runtime/checkptr.go:52 +0xc0 fp=0x204000362f40 sp=0x204000362f10 pc=0x409c80
github.com/go-delve/delve/pkg/proc/native.processVmRead(0x204000bfe138?, 0x8?, {0x204000bfe138?, 0x8, 0x0?})
/home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/native/ptrace_linux_64bit.go:17 +0x58 fp=0x204000362fd0 sp=0x204000362f40 pc=0x8f6948
github.com/go-delve/delve/pkg/proc/native.(*nativeThread).ReadMemory(0x204000397ad0, {0x204000bfe138, 0x8, 0x8}, 0x10)
/home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/native/threads_linux.go:112 +0xc4 fp=0x204000363080 sp=0x204000362fd0 pc=0x8fa374
github.com/go-delve/delve/pkg/proc.readUintRaw({0xe550a0, 0x204000397ad0}, 0x204000363168?, 0x8)
/home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/variables.go:1690 +0x68 fp=0x2040003630e0 sp=0x204000363080 pc=0x878768
github.com/go-delve/delve/pkg/proc.getGVariable({0xe5b8e0, 0x204000397ad0})
/home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/variables.go:441 +0x108 fp=0x204000363170 sp=0x2040003630e0 pc=0x86c408
github.com/go-delve/delve/pkg/proc.GetG({0xe5b8e0, 0x204000397ad0})
/home/fanzha02/sharefolder/asanprojecttest/delve/pkg/proc/variables.go:253 +0x4f0 fp=0x2040003631f0 sp=0x204000363170 pc=0x86b100
github.com/go-delve/delve/service/api.ConvertThread({0xe5b8e0, 0x204000397ad0})
/home/fanzha02/sharefolder/asanprojecttest/delve/service/api/conversions.go:125 +0x26c fp=0x2040003632c0 sp=0x2040003631f0 pc=0x88ba3c
github.com/go-delve/delve/service/debugger.(*Debugger).state(0x204000846e10, 0x0)
/home/fanzha02/sharefolder/asanprojecttest/delve/service/debugger/debugger.go:610 +0x764 fp=0x204000363580 sp=0x2040003632c0 pc=0x901944
github.com/go-delve/delve/service/debugger.(*Debugger).State(0x204000846e10, 0x1)
/home/fanzha02/sharefolder/asanprojecttest/delve/service/debugger/debugger.go:582 +0x3dc fp=0x204000363600 sp=0x204000363580 pc=0x90110c
github.com/go-delve/delve/service/rpc2.(*RPCServer).State(0x2040008618e0, {0x0?}, {0xffffb5975e48, 0x204001826000})
/home/fanzha02/sharefolder/asanprojecttest/delve/service/rpc2/server.go:114 +0x60 fp=0x204000363640 sp=0x204000363600 pc=0xa9e630
runtime.call32(0x204000ed2640, 0x204000992990, 0x0, 0x0, 0x0, 0x20, 0x204000363ca0)
/home/fanzha02/work/go_project/gotest/src/runtime/asm_arm64.s:508 +0x7c fp=0x204000363670 sp=0x204000363640 pc=0x469db

@aarzilli
Copy link
Member

Ok. Should we change our use of sys.Iovec to the remoteSysIovec that uses uintptr for the Base field?

@zhangfannie
Copy link
Author

@aarzilli I'm not a developer of the delve project. But based on the stack trace, I'm more curious why regs.TLS() + thread.BinInfo().GStructOffset() calculates a bad pointer value, 16? Thank you.

@aarzilli
Copy link
Member

Probably because the go runtime hasn't fully finished initializing yet.

aarzilli added a commit to aarzilli/delve that referenced this issue Mar 11, 2022
Replaces sys.Iovec with a similar struct that uses uintptr instead of
*byte for the base field when referring to addresses of the target
process, so that we do not generate invalid pointers.

Fixes go-delve#2919
aarzilli added a commit to aarzilli/delve that referenced this issue Mar 11, 2022
Replaces sys.Iovec with a similar struct that uses uintptr instead of
*byte for the base field when referring to addresses of the target
process, so that we do not generate invalid pointers.

Fixes go-delve#2919
derekparker pushed a commit that referenced this issue Mar 15, 2022
…2922)

Replaces sys.Iovec with a similar struct that uses uintptr instead of
*byte for the base field when referring to addresses of the target
process, so that we do not generate invalid pointers.

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

Successfully merging a pull request may close this issue.

2 participants