Hi,
In data sabato 27 giugno 2015 15:30:00, Mark Wielaard ha scritto:
The only problem is that we cannot guarantee not to break things
since in general elfutils targets the GNU/Linux platform. I really
need to setup a buildbot so that people can provide slaves to
automatically tests their preferred platform doesn't break.
That sounds like a good idea.
In any case, elfutils is part of Debian, and thus it gets built also
on the two current non-Linux ports, GNU/kFreeBSD and GNU/Hurd.
> * Build
>
> With the provided commits, most of sources (including tests) build
> again.
>
> The remaining issue is in src/strings.c, the unconditional usage of
> MAP_POPULATE. which is Linux-only. A local workaround of defining it
> to 0 if not already defined seemed to not cause further issues.
Yeah, it is slightly ugly, but it really is only a hint. So defining
it to zero if it isn't defined aleady seems fine. This is done on the
portable branch. Which I want to get rid of. So lets just do this on
master.
Ok -- commit attached.
> Let's analyze the failing tests:
> | FAIL: run-native-test.sh
> | ========================
> |
> | SRCDIR/tests/allregs: dwfl_module_register_names: no error
>
> allregs relies on the kernel modules handling, which is
> non-functional since there are no "kernel modules" on the Hurd
> (microkernel with userspace servers).
I don't think that is true. It seems run-native-tests.sh tests against
user space with -e (for an executable file test) and -p (for a
running process test), but not against kernel modules (-k or -K).
Ah, I see. Then what I wrote below apply here too, i.e. that no
filenames are currently provided in Hurd's procfs for /proc/$pid/maps
files.
> I get a clearer error message about this with:
>
> diff --git a/tests/allregs.c b/tests/allregs.c
> index 901d4e8..d3d459e 100644
> --- a/tests/allregs.c
> +++ b/tests/allregs.c
> @@ -158,6 +158,8 @@ main (int argc, char **argv)
>
> Dwfl_Module *mod = NULL;
> if (dwfl_getmodules (dwfl, &first_module, &mod, 0) < 0)
> error (EXIT_FAILURE, 0, "dwfl_getmodules: %s", dwfl_errmsg (-1));
> + if (mod == NULL)
> + error (EXIT_FAILURE, 0, "dwfl_getmodules: module not found");
>
> if (remaining == argc)
> {
Better to see first if dwfl_errno () == 0. It it is non-zero we do
want to print the actual error reported.
Do you mean if dwfl_getmodules fails (i.e. in the existing error()
message), or before the check I proposed?
Also, would be an option to skip the test if no modules are available?
Or that is considered a mandatory condition?
> Also, if I comment out the usage of allregs from
run-native-test.sh,
> the test passes.
>
> | FAIL: dwfl-bug-fd-leak
> | ======================
> |
> | (empty)
>
> This test actually crashes on Hurd, as the dwfl_addrmodule() at
> dwfl-bug-fd-leak.c:68 returns null. This is most probably due to our
> procfs emulation not showing yet filenames in /proc/$pid/maps files.
> At least the patch below avoids the segfault of the test, still
> leaving the failure:
>
> diff --git a/tests/dwfl-bug-fd-leak.c b/tests/dwfl-bug-fd-leak.c
> index 170a61a..f992b7a 100644
> --- a/tests/dwfl-bug-fd-leak.c
> +++ b/tests/dwfl-bug-fd-leak.c
> @@ -65,7 +65,10 @@ elfutils_open (pid_t pid, Dwarf_Addr address)
> }
> else
> {
> - Elf *elf = dwfl_module_getelf (dwfl_addrmodule (dwfl, address), &bias);
> + Dwfl_Module *module = dwfl_addrmodule (dwfl, address);
> + if (module == NULL)
> + error (2, 0, "dwfl_addrmodule: no module available for 0x%llx",
address);
> + Elf *elf = dwfl_module_getelf (module, &bias);
> if (elf == NULL)
> error (2, 0, "dwfl_module_getelf: %s", dwfl_errmsg (-1));
> }
That looks like the correct check to me.
OK -- commit attached.
> | FAIL: run-deleted.sh
> | ====================
> |
> | SRCDIR/src/stack: dwfl_linux_proc_attach pid 1491: Function not
>
> implemented
>
> This is because dwfl_linux_proc_attach is implemented only on Linux,
> returning ENOSYS everywhere else. I made the test skipped with the
> following:
>
> diff --git a/tests/backtrace-subr.sh b/tests/backtrace-subr.sh
> index 790b4f4..bb306c0 100644
> --- a/tests/backtrace-subr.sh
> +++ b/tests/backtrace-subr.sh
> @@ -74,6 +74,10 @@ check_unsupported()
>
> echo >&2 $testname: arch not supported
> exit 77
>
> fi
>
> + if grep -q -E ': dwfl_linux_proc_attach pid ([[:digit:]]+):
> Function not implemented$' $err; then
> + echo >&2 $testname: OS not supported
> + exit 77
> + fi
>
> }
>
> check_native_unsupported()
>
> but I'm not sure that would be the correct way (especially that it
> relies on the string representation of ENOSYS).
It isn't really correct to do it that way, but we do already depend on
the string representation of some error messages in some other tests.
Lets just use it here too.
OK -- commit attached, done only in run-deleted.sh itself now (more
logic).
Better would of course be to implement the support. Doesn't the
hurd
support ptrace?
There is a basic implementation, possibly not fully working though.
I'd avoid it for now, maybe in the future...
Thanks,
--
Pino Toscano