Page MenuHomeFreeBSD

Update devel/elfutils to 0.174
ClosedPublic

Authored by cem on Nov 4 2018, 3:48 AM.

Details

Summary

Rebase one Makefile patch and patch new use of mremap with munmap+mmap.

PR 232932

Test Plan

Something fails in stage-qa on my system, probably due to screwy pkg vs openssl
version after the bump. I need to update world.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cem created this revision.Nov 4 2018, 3:48 AM
gerald accepted this revision.Nov 4 2018, 10:54 AM

This looks good to me, thanks (with the disclaimer that I'm not exactly an expert on the mmap stuff).

I checked the man page for mrep on an openSUSE system and found

CONFORMING TO
    This call is Linux-specific, and should not be used in programs intended to be portable.

so indeed the looks like a (portability) bug. Are you going to report this upstream (possibly with your patch) or would you like me to do so?

This revision is now accepted and ready to land.Nov 4 2018, 10:54 AM
linimon retitled this revision from Update elfutils to 0.174 to Update devel/elfutils to 0.174.Nov 4 2018, 5:32 PM
cem added a comment.Nov 4 2018, 6:21 PM

This looks good to me, thanks (with the disclaimer that I'm not exactly an expert on the mmap stuff).

Thanks! I'm not either. Just doing my best off reading the manual pages. I have some concerns with my approach as taken but don't like anything else much better.

  1. If munmap() fails, we shouldn't destroy the mapping pointer and state on the object? OTOH, I don't know why munmap would fail.
  2. Does anything really care if we actually stay at the same address? That's what mremap(2) promises, as long as the MREMAP_MAYMOVE flag is not included. And elfutils' use does not include MREMAP_MAYMOVE. If it did, I'd just remove MAP_FIXED.
  3. Maybe doing something clever with guard pages could avoid unmapping the existing mapping in the case where extending the range was not possible due to another mapping. I did not take this approach because (a) I don't understand guard pages very well; (b) I'm not sure guard pages are available prior to 12.0; and (c) getting this right would be more complicated and more effort. I would guess typical libeu applications are not address-space constrained and would likely be able to extend an existing mapping. The authors seem to assume that is true on Linux, anyway.

I checked the man page for mrep on an openSUSE system and found

CONFORMING TO
    This call is Linux-specific, and should not be used in programs intended to be portable.

so indeed the looks like a (portability) bug. Are you going to report this upstream (possibly with your patch) or would you like me to do so?

My impression is that elfutils is by Linux folks for Linux exclusively. They don't mind that we port it, but I don't think they care much about off-Linux portability. If you would like to send the report upstream, please go ahead. But I am not very optimistic. There is a bunch of other Linux-specific functionality we port around if you check out the rest of the patches in files/. In particular, patch-lib_eu-config.h. (Maybe mremap should have been added as a static inline there instead.)

Thanks for reviewing!

cem edited subscribers, added: kib; removed: mat.Nov 7 2018, 4:17 AM
This revision was automatically updated to reflect the committed changes.