Project

General

Profile

Bug #5291

x86 {high,low}bit rely on undefined behavior

Added by Josef Sipek over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
kernel
Start date:
2014-11-06
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

Intel defines bsf & bsr instructions as:

If the content of the source operand is 0, the content of the destination operand is undefined.

All the implementation in the gate assume that if the source is zero, the destination operand is unmodified. This needs to be fixed to avoid difficult to diagnose issues on future hardware in case this assumption doesn't hold.

http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/asm/bitmap.h#39
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/ia32/ml/i86_subr.s#2800
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/ia32/ml/i86_subr.s#2830
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/amd64/ml/amd64.il#66
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/ia32/ml/ia32.il#68
http://src.illumos.org/source/xref/illumos-gate/usr/src/uts/intel/ia32/ml/ia32.il#77


Related issues

Related to illumos gate - Bug #5288: x86 bitmap inline asm functions duplicate codeClosed2014-11-05

Actions

History

#1

Updated by Josef Sipek over 5 years ago

http://31bits.net/illumos/cr/5291-x86-highbit-lowbit-rely-on-undefined-behavior/

I think it makes sense to ditch the studio inline implementation. Given that studio's inline asm support is pretty rudimentary (unlike gcc, it still follows the ABI calling conventions instead of "templating"), the fact that function calls (on x86) these days aren't as expensive as they once were, and that we don't really care about studio that much anymore, I think that it's reasonable to just nuke them.

Sadly, I don't think there's a way in inline asm to specify the zero flag in [er]flags as a constraint - hence the setz instructions.

#2

Updated by Josef Sipek over 5 years ago

  • % Done changed from 0 to 80
#3

Updated by Electric Monk over 5 years ago

  • Status changed from New to Closed
  • % Done changed from 80 to 100

git commit fdb8cf8c1b80da286f448f5e748b65f9115d7043

commit  fdb8cf8c1b80da286f448f5e748b65f9115d7043
Author: Josef 'Jeff' Sipek <josef.sipek@nexenta.com>
Date:   2014-11-24T18:25:18.000Z

    5291 x86 {high,low}bit rely on undefined behavior
    Reviewed by: Keith Wesolowski <keith.wesolowski@joyent.com>
    Approved by: Dan McDonald <danmcd@omniti.com>

Also available in: Atom PDF