Page MenuHomeFreeBSD
Differential D15535

Indicate the brk/sbrk are deprecated and not portable.
ClosedPublic
Actions

Authored by brooks on May 23 2018, 3:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 2, 10:22 PM
Unknown Object (File)
Fri, Sep 27, 8:24 PM
Unknown Object (File)
Fri, Sep 27, 3:15 PM
Unknown Object (File)
Mon, Sep 23, 4:23 PM
Unknown Object (File)
Mon, Sep 16, 1:19 AM
Unknown Object (File)
Sun, Sep 15, 1:24 AM
Unknown Object (File)
Wed, Sep 11, 4:55 AM
Unknown Object (File)
Sep 9 2024, 2:29 AM
View All 67 Files
Subscribers
imp
saper_saper.info

Details

Reviewers
emaste
andrew
Group Reviewers
manpages
Commits
rS334440: MFC r292268, r334176
rS334439: MFC r334176:
rS334176: Indicate the brk/sbrk are deprecated and not portable.
Summary

Include the history of arm64 and riscv shipping without brk/sbrk.

Mention that sbrk(0) produces unreliable results.

(Per request in D6464).

Diff Detail

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

Event Timeline

brooks created this revision.May 23 2018, 3:55 PM
Harbormaster completed remote builds in B16784: Diff 42883.May 23 2018, 3:55 PM
brooks added a reviewer: manpages.May 23 2018, 3:56 PM
saper_saper.info added a subscriber: saper_saper.info.May 23 2018, 8:24 PM
saper_saper.info added inline comments.
lib/libc/sys/brk.2
180 ↗(On Diff #42883)

typo: "reflect"

brooks updated this revision to Diff 42894.May 23 2018, 8:27 PM
  • Fix typo.
Harbormaster completed remote builds in B16790: Diff 42894.May 23 2018, 8:27 PM
brooks marked an inline comment as done.May 23 2018, 8:28 PM

Thanks for the review @saper_saper.info

saper_saper.info added a comment.May 23 2018, 8:31 PM

I have one question. From what I see in the code for brk() and sbrk() got removed from the C library, but the system call sys_obreak still exists, correct?

emaste accepted this revision.May 23 2018, 8:41 PM
This revision is now accepted and ready to land.May 23 2018, 8:41 PM
saper_saper.info added a comment.May 23 2018, 8:41 PM

Are there any architecture-specific reasons for removal? (Something that riscv and aarch64 cannot do but amd64 and i386 can)?

Maybe we should also hint at mmap(2) as a replacement for larger than zero sbrk(2).

emaste added a comment.May 23 2018, 8:44 PM

Are there any architecture-specific reasons for removal?

Just a lack of backwards compatibility requirements (i.e., there are no existing FreeBSD binaries which make use of sbrk).

brooks added a comment.May 23 2018, 8:46 PM
In D15535#328078, @saper_saper.info wrote:

I have one question. From what I see in the code for brk() and sbrk() got removed from the C library, but the system call sys_obreak still exists, correct?

Hmm, that's a bug. We should remove it (or at least make it ENOSYS).

brooks added a comment.May 23 2018, 8:49 PM
In D15535#328082, @emaste wrote:

Are there any architecture-specific reasons for removal?

Just a lack of backwards compatibility requirements (i.e., there are no existing FreeBSD binaries which make use of sbrk).

Also, sbrk() is incompatible with (or at least a very poor fit with) a number of memory protection schemes. Among other things it defeats ASLR.

saper_saper.info added a comment.May 23 2018, 8:53 PM
In D15535#328087, @brooks wrote:
In D15535#328078, @saper_saper.info wrote:

I have one question. From what I see in the code for brk() and sbrk() got removed from the C library, but the system call sys_obreak still exists, correct?

Hmm, that's a bug. We should remove it (or at least make it ENOSYS).

I haven't tried to call it in assembly myself, it's just my reading of D6464: Remove sbrk on arm64 and some random grepping around the code, so don't take my word.

So basically we are just taking the opportunity of the new architecture not to have to provide backward compatibility for it later?

brooks updated this revision to Diff 42898.May 23 2018, 9:15 PM
  • Explictitly refer to mmap.
  • Refer to mmap in the right section.
This revision now requires review to proceed.May 23 2018, 9:15 PM
Harbormaster completed remote builds in B16792: Diff 42898.May 23 2018, 9:15 PM
emaste added inline comments.May 23 2018, 9:30 PM
lib/libc/sys/brk.2
56 ↗(On Diff #42898)

section 3?

brooks added inline comments.May 23 2018, 9:57 PM
lib/libc/sys/brk.2
56 ↗(On Diff #42898)

grr, fixed one and added another. Will fix shortly.

brooks updated this revision to Diff 42940.May 24 2018, 5:21 PM
  • mmap(2) not mmap(3).
Harbormaster completed remote builds in B16815: Diff 42940.May 24 2018, 5:21 PM
saper_saper.info added inline comments.May 24 2018, 5:41 PM
lib/libc/sys/brk.2
159 ↗(On Diff #42940)
$ groff -Tps -mdoc t.man > t.ps
mdoc warning: .Fx: Unknown FreeBSD version `11' (#159)

Ugh

saper_saper.info added a comment.May 24 2018, 5:58 PM

mdoc(7) nitpicking :(

lib/libc/sys/brk.2
159 ↗(On Diff #42940)

Just use

.Fx 11.0
168 ↗(On Diff #42940)

From the point of view of readability .Fn does not seem to mix well with .Xr in formatted text.

t01.png (164×824 px, 2 KB)
t02.png (257×547 px, 4 KB)

brooks updated this revision to Diff 42947.May 24 2018, 6:20 PM
brooks marked 5 inline comments as done.
  • Use "Fx 11.0" rather than "Fx 11"
Harbormaster completed remote builds in B16822: Diff 42947.May 24 2018, 6:20 PM
brooks added inline comments.May 24 2018, 6:20 PM
lib/libc/sys/brk.2
168 ↗(On Diff #42940)

Way beyond the scope of this change and too much effort for an interface people should have long since abandoned.

emaste accepted this revision.May 24 2018, 6:25 PM
This revision is now accepted and ready to land.May 24 2018, 6:25 PM
Closed by commit rS334176: Indicate the brk/sbrk are deprecated and not portable. (authored by brooks). · Explain WhyMay 24 2018, 6:33 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptMay 24 2018, 6:33 PM
saper_saper.info added a comment.May 24 2018, 8:10 PM

Great, thanks!

Revision Contents
Changeset List

PathSize
head/
lib/
libc/
sys/
17 lines

Diff 42948

View Options

head/lib/libc/sys/brk.2

Loading...