Bug 23903 - Uses linux specifc mremap
Summary: Uses linux specifc mremap
Status: RESOLVED DUPLICATE of bug 27337
Alias: None
Product: elfutils
Classification: Unclassified
Component: libelf (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Mark Wielaard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-20 20:28 UTC by Kurt Roeckx
Modified: 2022-07-01 21:25 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed: 2019-08-09 00:00:00


Attachments
patch to allow compile on systems without mremap (334 bytes, patch)
2019-08-03 20:17 UTC, Jean Michault
Details | Diff
patch to allow compile on systems without mremap (492 bytes, patch)
2019-08-04 16:38 UTC, Jean Michault
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kurt Roeckx 2018-11-20 20:28:38 UTC
In Debian's kfreebsd ports, I'm getting:
/usr/bin/ld: libelf_pic.a(elf_update.os): in function `write_file':
./libelf/elf_update.c:109: undefined reference to `mremap'
Comment 1 Mark Wielaard 2018-11-21 12:30:31 UTC
That is a weird error. It means you could compile fine as if mremap was available, but only during linking it is suddenly discovered that there is no mremap?

mremap is needed for supporting ELF_C_RDWR_MMAP with ELF files that grow. See for example the tests/run-copyadd-sections.sh and tests/run-copymany-sections.sh testcases (and the explanation in the second why we don't use ELF_C_RDWR_MMAP). mremap is used because it is the only way to extend an mmap without moving it.

Even on GNU/Linux mremap might fail, in which case writing out an ELF_C_RDWR_MMAPed ELF file might just not work (it normally works when extending the file a little, but not when multiple pages are needed).
Comment 2 Kurt Roeckx 2018-11-21 19:21:15 UTC
Note that Debian's kfreebsd port uses glibc. Maybe they have mremap() in the header even when it's not actually in the library?
Comment 3 Kurt Roeckx 2018-11-21 19:24:37 UTC
On Hurd I get:
/usr/bin/ld: libelf_pic.a(elf32_updatefile.os): in function `__elf32_updatemmap':
./libelf/elf32_updatefile.c:462: warning: msync is not implemented and will always fail
/usr/bin/ld: libelf_pic.a(elf_update.os): in function `write_file':
./libelf/elf_update.c:109: undefined reference to `mremap'
Comment 4 Mark Wielaard 2018-11-23 09:11:44 UTC
Having an msync that doesn't write out changes to a file is bad, but you might get away with it in most cases (the changes should be written to disk when munmap is called).

You do need a mremap function, even if it always fails.
Comment 5 Kurt Roeckx 2018-11-23 17:43:14 UTC
Since I don't have it, it will make this a Linux only package. This at least means that currently glib2.0 and rpm will also not be able to get build on non-Linux platforms. This is really a Linux specific system call.
Comment 6 Mark Wielaard 2018-11-23 20:31:11 UTC
It is fine for elfutils to have GNU/Linux specific code since that is the primary target. But if other platforms provide needed functionality in a different way then patches are obviously more than welcome. In the case of mremap you could even get away with something that always fails. Which would probably work just fine for Hurd, but for kfreebsd you really need to figure out first why it seems to provide a mremap function but then fails while linking. Otherwise it will be hard to know when/why to provide an alternative.
Comment 7 Kurt Roeckx 2018-11-23 21:29:36 UTC
I've filed a bug in Debian about mremap() being in the headers, so that should hopefully get fixed. But that will not fix the problem, it will either now just generate a warning that it doesn't have a prototype, or give a compilation error.

It seems to me that we want to detect in configure that mremap() exists, and if it doesn't disable the use of mmap.
Comment 8 Jean Michault 2019-08-03 20:17:14 UTC
Created attachment 11932 [details]
patch to allow compile on systems without mremap

I suggest the patch attached.
This makes it possible to compile elfutils on nonlinux systems, but generates an error if one tries to use features that need mremap.
Comment 9 Mark Wielaard 2019-08-03 20:56:40 UTC
(In reply to Jean Michault from comment #8)
> Created attachment 11932 [details]
> patch to allow compile on systems without mremap
> 
> I suggest the patch attached.
> This makes it possible to compile elfutils on nonlinux systems, but
> generates an error if one tries to use features that need mremap.

Thanks. Some questions about the proposed patch:
- What defines HAVE_MMAP?
  I don't believe we currently have a configure check for it, should we?
- The guard checks for HAVE_MMAP, but the code calls mremap.
  Should we have a HAVE_MREMAP guard and configure check instead?
- On which systems did you test and what were the make check results?
Comment 10 Jean Michault 2019-08-04 06:30:36 UTC
(In reply to Mark Wielaard from comment #9)
> (In reply to Jean Michault from comment #8)
> > Created attachment 11932 [details]
> > patch to allow compile on systems without mremap
> > 
> > I suggest the patch attached.
> > This makes it possible to compile elfutils on nonlinux systems, but
> > generates an error if one tries to use features that need mremap.
> 
> Thanks. Some questions about the proposed patch:
> - What defines HAVE_MMAP?
>   I don't believe we currently have a configure check for it, should we?
> - The guard checks for HAVE_MMAP, but the code calls mremap.
>   Should we have a HAVE_MREMAP guard and configure check instead?
> - On which systems did you test and what were the make check results?

You're right, i was a little too fast, you can have mmap without having mremap, so you have to test mremap. And we need to add the mremap test in configure.ac. I will work again.
Comment 11 Jean Michault 2019-08-04 16:38:21 UTC
Created attachment 11933 [details]
patch to allow compile on systems without mremap
Comment 12 Jean Michault 2019-08-04 16:47:43 UTC
(In reply to Mark Wielaard from comment #9)
> (In reply to Jean Michault from comment #8)
> > Created attachment 11932 [details]
> > patch to allow compile on systems without mremap
> > 
> > I suggest the patch attached.
> > This makes it possible to compile elfutils on nonlinux systems, but
> > generates an error if one tries to use features that need mremap.
> 
> Thanks. Some questions about the proposed patch:
> - What defines HAVE_MMAP?
>   I don't believe we currently have a configure check for it, should we?
> - The guard checks for HAVE_MMAP, but the code calls mremap.
>   Should we have a HAVE_MREMAP guard and configure check instead?
> - On which systems did you test and what were the make check results?

I propose another patch that checks mremap.
I tested on a debian system GNU / Hurd 2019. The compilation is going well but some tests fail:

============================================================================
Testsuite summary for elfutils 0.176
============================================================================
# TOTAL: 205
# PASS:  170
# SKIP:  23
# XFAIL: 0
# FAIL:  12
# XPASS: 0
# ERROR: 0

failing tests are :
FAIL: run-readelf-loc.sh
FAIL: run-readelf-ranges.sh
FAIL: run-bug1-test.sh
FAIL: run-varlocs.sh
FAIL: run-allfcts-multi.sh
FAIL: elfstrtab
FAIL: emptyfile
FAIL: run-get-units-split.sh
FAIL: run-attr-integrate-skel.sh
FAIL: run-all-dwarf-ranges.sh
FAIL: run-unit-info.sh
FAIL: run-copyadd-sections.sh
Comment 13 Mark Wielaard 2019-08-09 21:19:53 UTC
(In reply to Jean Michault from comment #12)
> I propose another patch that checks mremap.

That looks perfectly reasonable. Thanks.

It isn't really necessary for such small patches, but would you mind taking a look at our CONTRIBUTING file and let me know if you are OK with your Signed-off-by for this patch?
https://sourceware.org/git/?p=elfutils.git;a=blob_plain;f=CONTRIBUTING;hb=HEAD

> I tested on a debian system GNU / Hurd 2019. The compilation is going well
> but some tests fail:
> 
> ============================================================================
> Testsuite summary for elfutils 0.176
> ============================================================================
> # TOTAL: 205
> # PASS:  170
> # SKIP:  23
> # XFAIL: 0
> # FAIL:  12
> # XPASS: 0
> # ERROR: 0
> 
> failing tests are :
> FAIL: run-readelf-loc.sh
> FAIL: run-readelf-ranges.sh
> FAIL: run-bug1-test.sh
> FAIL: run-varlocs.sh
> FAIL: run-allfcts-multi.sh
> FAIL: elfstrtab
> FAIL: emptyfile
> FAIL: run-get-units-split.sh
> FAIL: run-attr-integrate-skel.sh
> FAIL: run-all-dwarf-ranges.sh
> FAIL: run-unit-info.sh
> FAIL: run-copyadd-sections.sh

I would expect run-copyadd-sections to fail (because it explicitly tests this code path and will fail if mremap fails). But some of the others are surprising. Would you mind attaching the tests/test-suite.log file so we can inspect how exactly these tests fail?
Comment 14 Mark Wielaard 2019-08-26 14:29:08 UTC
Hi Jean Michault,

I am still interested getting this patch in. Could you look at comment #13 to see if the CONTRIBUTING text makes sense and if you could provide the tests/test-suite.log file to analyze the results a bit better.

Thanks,

Mark
Comment 15 Mark Wielaard 2022-07-01 21:25:17 UTC
This was resolved for the similar bug #27337

*** This bug has been marked as a duplicate of bug 27337 ***