Page MenuHomeFreeBSD
Differential D16525

Some fixes for compat32.
ClosedPublic
Actions

Authored by kib on Jul 31 2018, 10:15 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jan 7, 12:54 AM
Unknown Object (File)
Sat, Jan 4, 6:42 PM
Unknown Object (File)
Tue, Dec 17, 10:30 AM
Unknown Object (File)
Tue, Dec 17, 3:24 AM
Unknown Object (File)
Nov 24 2024, 9:38 PM
Unknown Object (File)
Nov 24 2024, 12:12 AM
Unknown Object (File)
Nov 21 2024, 9:32 AM
Unknown Object (File)
Nov 19 2024, 3:25 PM
View All 87 Files
Subscribers
imp

Details

Reviewers
asomers
ngie
Commits
rS336987: For compat32, emulate the same wraparound check as occurs on the real
rS336980: Provide compat32 shims for sched_rr_get_interval(2).
Summary
  1. Fix sched_rr_get_interval(2) for compat32, timespec requires translation.

PR: 230175

  1. Fix mprotect_err test to not assume specific address space layout.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kib created this revision.Jul 31 2018, 10:15 AM
Herald added a reviewer: ngie. · View Herald TranscriptJul 31 2018, 10:15 AM
Herald added a subscriber: imp. · View Herald Transcript
Harbormaster completed remote builds in B18456: Diff 46065.Jul 31 2018, 10:15 AM
asomers added a comment.Jul 31 2018, 12:49 PM

Are you going to regenerate sys/compat/freebsd32 stuff too?

kib added a comment.Jul 31 2018, 12:58 PM
In D16525#350820, @asomers wrote:

Are you going to regenerate sys/compat/freebsd32 stuff too?

As a follow-up commit, yes. It is irrelevant to the review.

asomers added a comment.Jul 31 2018, 12:59 PM

Also, I notice three other syscalls in compat/freebsd32 that use plain timespec instead of timespec32. Do they need fixing too? I haven't tested them. They are freebsd32_sigtimedwait, freebsd32_futimens, and freebsd32_utimensat.

asomers added a comment.Jul 31 2018, 1:13 PM

The mprotect_err test is now failing on amd64. The assertion at line 167 fails. I haven't tried it in 32-bit mode yet; I need to rebuild my 32-bit world first.

kib added a comment.Jul 31 2018, 2:07 PM
In D16525#350843, @asomers wrote:

The mprotect_err test is now failing on amd64. The assertion at line 167 fails. I haven't tried it in 32-bit mode yet; I need to rebuild my 32-bit world first.

I see what is going on, finally. Can you explain to me how to rebuild mprotect_test without rebuilding the whole world, even with -DNO_CLEAN ? For normal components, I can make buildenv, then cd <dir> && make.

kib added a comment.Jul 31 2018, 2:08 PM
In D16525#350840, @asomers wrote:

Also, I notice three other syscalls in compat/freebsd32 that use plain timespec instead of timespec32. Do they need fixing too? I haven't tested them. They are freebsd32_sigtimedwait, freebsd32_futimens, and freebsd32_utimensat.

They use struct timespec in the declaration, but correctly copy in timespec32 and convert to timespec as needed. I will fix the cosmetics in freebsd32/syscalls.master later.

kib updated this revision to Diff 46071.Jul 31 2018, 2:23 PM

This should correctly detect the address wraparound on compat32 ABIs.

Harbormaster completed remote builds in B18457: Diff 46071.Jul 31 2018, 2:23 PM
asomers added a comment.Jul 31 2018, 3:47 PM
In D16525#350848, @kib wrote:
In D16525#350843, @asomers wrote:

The mprotect_err test is now failing on amd64. The assertion at line 167 fails. I haven't tried it in 32-bit mode yet; I need to rebuild my 32-bit world first.

I see what is going on, finally. Can you explain to me how to rebuild mprotect_test without rebuilding the whole world, even with -DNO_CLEAN ? For normal components, I can make buildenv, then cd <dir> && make.

I don't even bother with "make buildenv". I just cd to the directory and "make && make install". Remember, you can't execute the test from the source directory. You must install it into /usr/tests/lib/libc/sys.

asomers accepted this revision.Jul 31 2018, 4:07 PM

LGTM. Don't forget to reference both PR 230175 and PR 230162 in your commit message. And don't forget to commit the autogenerated files too.

This revision is now accepted and ready to land.Jul 31 2018, 4:07 PM
Closed by commit rS336980: Provide compat32 shims for sched_rr_get_interval(2). (authored by kib). · Explain WhyJul 31 2018, 5:15 PM
This revision was automatically updated to reflect the committed changes.
kib reopened this revision.Jul 31 2018, 5:17 PM
kib updated this revision to Diff 46082.Jul 31 2018, 5:48 PM

Better fix for mprotect(2) wraparound check. It does not change the more delicate behavior for 64bits,

This revision was not accepted when it landed; it landed in state Needs Review.Jul 31 2018, 6:01 PM
Closed by commit rS336987: For compat32, emulate the same wraparound check as occurs on the real (authored by kib). · Explain Why
This revision was automatically updated to reflect the committed changes.
asomers added a comment.Jul 31 2018, 6:04 PM

Updating a Phabricator revision without including all of the files that have ever been part of that revision makes the history confusing, because Phabricator thinks you want to revert your changes to the files that you didn't include. You can't fix it now, but please try not to do it again.

Revision Contents
Changeset List

PathSize
head/
sys/
vm/
6 lines

Diff 46083

View Options

head/sys/vm/vm_mmap.c

Loading...