Page MenuHomeFreeBSD
Differential D54303

libc/amd64: fix stpncpy.S again
Needs ReviewPublic
Actions

Authored by fuz on Thu, Dec 18, 11:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 19, 3:15 PM
Unknown Object (File)
Fri, Dec 19, 6:57 AM
Unknown Object (File)
Fri, Dec 19, 6:56 AM
Unknown Object (File)
Fri, Dec 19, 6:25 AM
Unknown Object (File)
Fri, Dec 19, 5:52 AM
Unknown Object (File)
Thu, Dec 18, 11:51 PM
Subscribers
imp

Details

Reviewers
cy
des
markj
getz
Summary

The previous fix introduced a regression on machines without the BMI1
instruction set extension. The TZCNT instruction used in this function
behaves different on old machines when the source operand is zero, but
the code was originally designed to never trigger this case. The bug
fix caused this case to be possible, leading to a regression on
sufficiently old hardware.

Fix the code by messing with things such that the source operand is
never zero.

PR: 291720
Fixes: 66eb78377bf109af1d9e25626bf254b4369436ec

Test Plan

passes unit test. The behaviour on older machines was
manually checked by replacing TZCNT with BSF in the affected code
path. Please check again before I land this.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 69391
Build 66274: arc lint + arc unit

Event Timeline

fuz created this revision.Thu, Dec 18, 11:42 PM
Herald added a subscriber: imp. · View Herald TranscriptThu, Dec 18, 11:42 PM
fuz requested review of this revision.Thu, Dec 18, 11:42 PM
Harbormaster completed remote builds in B69391: Diff 168382.Thu, Dec 18, 11:42 PM
markj added a comment.Fri, Dec 19, 1:51 PM

Approved. If this doesn't land soon, say in the next day or two, we should revert the original change until it does.

fuz added a comment.Fri, Dec 19, 2:50 PM
In D54303#1241050, @markj wrote:

Approved. If this doesn't land soon, say in the next day or two, we should revert the original change until it does.

A change to completely disable the SIMD code until this is fixed (line 39) had already been committed when the regression was discovered, so the tree is currently in working order.

Revision Contents
Changeset List

PathSize
lib/
libc/
amd64/
string/
9 lines

Diff 168382

View Options

lib/libc/amd64/string/stpncpy.S

Loading...