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

Jump to bottom

cmd/link: pe .rdata section should not need to be IMAGE_SCN_MEM_EXECUTE #25926

Closed
alexbrainman opened this issue Jun 17, 2018 · 3 comments
Closed

cmd/link: pe .rdata section should not need to be IMAGE_SCN_MEM_EXECUTE #25926

alexbrainman opened this issue Jun 17, 2018 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@alexbrainman
Copy link
Member

CL 115975 split .text section into .text (code) and .rdata (read only data).

I expected .rdata not to need IMAGE_SCN_MEM_EXECUTE flag set (flag that instructs Windows to allow code execution in that section).

Unfortunately, when I run debug/pe tests, TestInternalLinkerDWARF failed with

--- FAIL: TestInternalLinkerDWARF (0.96s)
    file_test.go:348: running test executable failed: exit status 2 Exception 0xc0000005 0x8 0x4d9058 0x4d9058
        PC=0x4d9058

        runtime: unknown pc 0x4d9058
...

I managed to fix test by adding IMAGE_SCN_MEM_EXECUTE to .rdata section. I did not have time to investigate why IMAGE_SCN_MEM_EXECUTE was required, so I just put TODO for later https://go-review.googlesource.com/c/go/+/115975/3/src/cmd/link/internal/ld/pe.go#1346

I had some time to debug this. TestInternalLinkerDWARF builds a.exe executable. I run a.exe in gdb. And I can see that the crash is happening in x_cgo_thread_start. All other functions do have debug information, but this function does not. The asm for x_cgo_thread_start is:

(gdb) disas
Dump of assembler code for function x_cgo_thread_start:
   0x0000000000401330 <+0>:     push   %rbx
   0x0000000000401331 <+1>:     sub    $0x20,%rsp
   0x0000000000401335 <+5>:     mov    %rcx,%rbx
   0x0000000000401338 <+8>:     mov    $0x18,%ecx
=> 0x000000000040133d <+13>:    callq  0x4d9058 <runtime.etypes+24>
   0x0000000000401342 <+18>:    test   %rax,%rax
   0x0000000000401345 <+21>:    je     0x40136a <x_cgo_thread_start+58>
   0x0000000000401347 <+23>:    mov    (%rbx),%rdx
   0x000000000040134a <+26>:    mov    %rax,%rcx
   0x000000000040134d <+29>:    mov    %rdx,(%rax)
   0x0000000000401350 <+32>:    mov    0x8(%rbx),%rdx
   0x0000000000401354 <+36>:    mov    %rdx,0x8(%rax)
   0x0000000000401358 <+40>:    mov    0x10(%rbx),%rdx
   0x000000000040135c <+44>:    mov    %rdx,0x10(%rax)
   0x0000000000401360 <+48>:    add    $0x20,%rsp
   0x0000000000401364 <+52>:    pop    %rbx
   0x0000000000401365 <+53>:    jmpq   0x401430 <_cgo_sys_thread_start>
   0x000000000040136a <+58>:    callq  *0x149cf0(%rip)        # 0x54b060
   0x0000000000401370 <+64>:    mov    $0x2b,%r8d
   0x0000000000401376 <+70>:    mov    $0x1,%edx
   0x000000000040137b <+75>:    lea    0x60(%rax),%r9
   0x000000000040137f <+79>:    lea    0xd4fda(%rip),%rcx        # 0x4d6360 <runtime/cgo(.rdata)>
   0x0000000000401386 <+86>:    callq  0x4d9040 <runtime.etypes>
   0x000000000040138b <+91>:    callq  0x4d9048 <runtime.etypes+8>
   0x0000000000401390 <+96>:    nop

See how call into _beginthread says <runtime.etypes+24> ? It appears _beginthread function end-up in .rdata section.

(gdb) disas
Dump of assembler code for function runtime.etypes:
   0x00000000004d9040 <+0>:     jmpq   *0x54b040
   0x00000000004d9047 <+7>:     nop
   0x00000000004d9048 <+8>:     jmpq   *0x54b058
   0x00000000004d904f <+15>:    nop
   0x00000000004d9050 <+16>:    jmpq   *0x54b050
   0x00000000004d9057 <+23>:    nop
=> 0x00000000004d9058 <+24>:    jmpq   *0x54b038
   0x00000000004d905f <+31>:    nop
   0x00000000004d9060 <+32>:    jmpq   *0x54b048
   0x00000000004d9067 <+39>:    nop
   0x00000000004d9068 <+40>:    add    %al,(%rax)
   0x00000000004d906a <+42>:    add    %al,(%rax)
   0x00000000004d906c <+44>:    add    %al,(%rax)
   0x00000000004d906e <+46>:    add    %al,(%rax)
   0x00000000004d9070 <+48>:    add    %al,(%rax)
   0x00000000004d9072 <+50>:    add    %al,(%rax)
   0x00000000004d9074 <+52>:    add    %al,(%rax)
   0x00000000004d9076 <+54>:    add    %al,(%rax)
   0x00000000004d9078 <+56>:    add    %al,(%rax)
   0x00000000004d907a <+58>:    add    %al,(%rax)
   0x00000000004d907c <+60>:    add    %al,(%rax)
   0x00000000004d907e <+62>:    add    %al,(%rax)
End of assembler dump.
(gdb) si

Program received signal SIGSEGV, Segmentation fault.
0x00000000004d9058 in runtime.etypes ()
(gdb)

It is no surprise that this code cannot execute - it needs IMAGE_SCN_MEM_EXECUTE.

How is _beginthread code

 0x00000000004d9058 <+24>:    jmpq   *0x54b038

end-up in .rdata section? How can we make it go into .text section?

I am hoping that @ianlancetaylor has some ideas about this. :-)

Alex
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 18, 2018
@andybons andybons added this to the Unplanned milestone Jun 18, 2018
@ianlancetaylor
Copy link
Contributor

I would guess that dope in cmd/link/internal/ld/pe.go should use STEXT rather than SELFROSECT. It looks like the contents of the section, as created in windynrelocsym, are instructions, not read-only data.

@alexbrainman
Copy link
Member Author

I would guess that ...

You guessed correctly. I should have guessed it myself. :-)

I think this issue can wait until after go1.11 is released.

Alex

@gopherbot
Copy link

Change https://golang.org/cl/125455 mentions this issue: cmd/link: move .rel symbol from .rdata into .text

@golang golang locked and limited conversation to collaborators Oct 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
4 participants
@andybons @ianlancetaylor @gopherbot @alexbrainman