Steel Bank Common Lisp

The x86-64 disassembler sometimes prints the REX prefix as BYTE #X48

Reported by Lutz Euler on 2012-12-02
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Low
Unassigned

Bug Description

Executing the following code

(let ((sb-c::*compiler-trace-output* *standard-output*))
  (compile nil `(lambda (x)
                   (declare (type word x) (optimize speed))
                   (values (floor x -8)))))

outputs as disassembly

; BD: 48892B MOV [RBX], RBP
; C0: 48 BYTE #X48
; C1: 8BEB MOV RBP, RBX

where one would expect

; BD: 48892B MOV [RBX], RBP
; C0: 488BEB MOV RBP, RBX

Lutz Euler (lutz-euler) wrote :

Bisected to

65bdee4ba534e82c352cff3eec16473daaf285dd is the first bad commit
commit 65bdee4ba534e82c352cff3eec16473daaf285dd
Author: Lutz Euler <email address hidden>
Date: Wed Dec 14 18:11:53 2011 +0100

    Improve handling of x86[-64] prefix instructions in the disassembler.

Lutz Euler (lutz-euler) wrote :

With a function definition

(defun f (x)
  (declare (type word x) (optimize speed))
  (values (floor x -8)))

both (DISASSEMBLE 'F) or (SB-DISASSEM::DISASSEMBLE-CODE-COMPONENT #'F) show the correct disassembly.
Replace F here with the (COMPILE NIL ...) expression from the bug description and both calls again show the correct disassembly.

So it may be that the bug occurs only when using *COMPILER-TRACE-OUTPUT*.

Lutz Euler (lutz-euler) on 2012-12-06
Changed in sbcl:
status: New → In Progress
assignee: nobody → Lutz Euler (lutz-euler)
importance: Undecided → Low
Lutz Euler (lutz-euler) wrote :

I attach a patch that fixes the issue.

I currently don't want to commit it because I am not yet decided whether the disassembler code could not be cleaned up in this area as described below which would allow for a simpler fix, too.

The cleanup would be to simplify disassemble-assem-segment, assem-segment-to-disassem-segments and add-block-segments by simply copying the complete assem-segment into one continuous vector and disassembling that. This would allow to drop all the machinery that now is in place to find instruction boundaries because add-block-segments seems or maybe even needs to assume that fillers could be anywhere inside instructions.

I don't currently understand whether this simplification would break something or might be undesirable.

Changed in sbcl:
status: In Progress → Confirmed
assignee: Lutz Euler (lutz-euler) → nobody
Lutz Euler (lutz-euler) wrote :

The attached patch implements the version with the simplified
disassembler as hinted at in comment #3.

I don't want to commit that currently, either, as there is an old
KLUDGE comment at ON-SEGMENTS-CONTENTS-VECTORLY
saying that it might be possible to move the segment compaction
to FINALIZE-SEGMENT.

I want to look into that first as it would supersede this patch.

Lutz Euler (lutz-euler) on 2013-05-08
Changed in sbcl:
assignee: nobody → Lutz Euler (lutz-euler)
status: Confirmed → In Progress
Lutz Euler (lutz-euler) wrote :

The attached patch implements a version that fixes the KLUDGE mentioned in comment #4. The segment compaction is now done in FINALIZE-SEGMENT. This allows for the same simplifications in the disassembler as done in the patch in comment #4.

Lutz Euler (lutz-euler) wrote :

I just committed the patch from #5 as 51bc001b7a98af096af782a672389e51004af068.

Changed in sbcl:
status: In Progress → Fix Committed
assignee: Lutz Euler (lutz-euler) → nobody
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers