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/asm: index out of range (4) #12657

Closed
dvyukov opened this issue Sep 17, 2015 · 4 comments
Closed

cmd/asm: index out of range (4) #12657

dvyukov opened this issue Sep 17, 2015 · 4 comments

Comments

@dvyukov
Copy link
Member

dvyukov commented Sep 17, 2015

cmd/asm crashes on the following program:

TEXT T(SB),$0
CALL (SP)(FP)
panic: runtime error: index out of range

goroutine 1 [running]:
cmd/internal/obj/x86.asmandsz(0xc820104000, 0xc820120140, 0xc8201201a8, 0x2, 0x0, 0x1)
    src/cmd/internal/obj/x86/asm6.go:2523 +0xf37
cmd/internal/obj/x86.doasm(0xc820104000, 0xc820120140)
    src/cmd/internal/obj/x86/asm6.go:3222 +0x902d
cmd/internal/obj/x86.asmins(0xc820104000, 0xc820120140)
    src/cmd/internal/obj/x86/asm6.go:4308 +0x15bb
cmd/internal/obj/x86.span6(0xc820104000, 0xc82011c000)
    src/cmd/internal/obj/x86/asm6.go:1709 +0x916
cmd/internal/obj.Writeobjdirect(0xc820104000, 0xc820112020)
    src/cmd/internal/obj/objfile.go:297 +0x35b
main.main()
    src/cmd/asm/main.go:65 +0xd9b

go version devel +5512ac2 Wed Sep 16 17:56:14 2015 +0000 linux/amd64

@robpike robpike assigned rsc and unassigned robpike Sep 17, 2015
@ianlancetaylor ianlancetaylor added this to the Go1.6 milestone Sep 17, 2015
@rsc rsc modified the milestones: Unplanned, Go1.6 Dec 5, 2015
@quasilyte
Copy link
Contributor

Reproduced with 46c5856
go version devel +46c5856 Mon Dec 25 00:07:22 2017 +0000 linux/amd64.

More generally, any of pseudo registers will cause runtime panic
if it is used as scaled index in memory operand.
This includes FP, SB and PC, but not SP (at least for x86).

Should (Base)(PseudoReg*Scale) result in error like
"illegal addressing mode for FP/SB/PC"?

If we are going for error, should this check be x86-specific,
or should ARM code fail to compile too?
Currently, it accepts things like MOVW (R20)(PC*2), R0.

Another note: empty scaling will result in asmidx: bad address for x86.
Here is more complete error reproducer:

// Included p.String() output (note that pseudo registers are not recognized).
TEXT asmfunc(SB), $0
        CALL (AX)(PC*1) // CALL	(AX)(R???-4*1) | runtime error: index out of range
        CALL (AX)(SB*1) // CALL	(AX)(R???-2*1) | runtime error: index out of range
        CALL (AX)(FP*1) // CALL	(AX)(R???-1*1) | runtime error: index out of range
        RET

@quasilyte
Copy link
Contributor

If patch like this is applied:

diff --git a/src/cmd/asm/internal/asm/parse.go b/src/cmd/asm/internal/asm/parse.go
index 1d5d073..a278de0 100644
--- a/src/cmd/asm/internal/asm/parse.go
+++ b/src/cmd/asm/internal/asm/parse.go
@@ -790,6 +790,10 @@ func (p *Parser) registerIndirect(a *obj.Addr, prefix rune) {
                if r2 != 0 {
                        p.errorf("unimplemented two-register form")
                }
+               if p.arch.InFamily(sys.I386, sys.AMD64) && r1 < 0 {
+                       // Reject (R)(Pseudo*scale) forms for x86.
+                       p.errorf("illegal addressing mode for pseudo-register")
+               }
                a.Index = r1
                if scale == 0 && p.arch.Family == sys.ARM64 {
                        // scale is 1 by default for ARM64

Attempt to compile example above will result in:

GOARCH=386 ./bin/go tool asm foo.s
foo.s:2: illegal addressing mode for pseudo-register
foo.s:3: illegal addressing mode for pseudo-register
foo.s:4: illegal addressing mode for pseudo-register
asm: assembly of foo.s failed

@gopherbot
Copy link

Change https://golang.org/cl/85419 mentions this issue: cmd/asm: disallow (Reg)(Pseudo*scale) indirect for x86

@gopherbot
Copy link

Change https://golang.org/cl/107835 mentions this issue: cmd/internal/obj/x86: disallow PC/FP/SB scaled index

@golang golang locked and limited conversation to collaborators Apr 20, 2019
@rsc rsc removed their assignment Jun 23, 2022
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

6 participants