Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@ncopa

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

What's the status on this? As seen by the mention, this is required to make some stuff work on Musl systems.

Although I do want to see this resolved, I don't believe the current implementation is correct.

It is some time since I worked on this, but I think my conclusion was that there is not really any way to implement this correctly (at least not in a portable way), so this is only an attempt to get it working on current known use cases.

Right, and that is reasonable for a downstream patch, but I am not too comfortable having it in the upstream. I generally think it is better to have something not working, which forces users to work around it in a way that works for their use-case, than to have something with broken behavior.

Anyway, regardless of if I think this is a good idea to have or not, as I said above, the new search mechanism you are introducing is not limited to musl. So, if this was ever to be merged, it should at least limit this search routine to musl. But I am not the maintainer, I am just giving my opinion.

Anyway, regardless of if I think this is a good idea to have or not, as I said above, the new search mechanism you are introducing is not limited to musl. So, if this was ever to be merged, it should at least limit this search routine to musl. But I am not the maintainer, I am just giving my opinion.

I find that unfair. Current _findSoname_ldconfig search routine is not limited to GNU libc, but comment says that GNU libc is assumed. Why should we limit search routing for musl when we don't do it for GNU libc?

That said, I started to look at how to fix the issues you have raised. I believe the {name}.so claim is bogus, but can add that to the search list if I can get a specific real world test case.

Finding the correct $ARCH for /etc/ld-musl-$ARCH.path is a more tricky problem, but can be solved with by finding the PT_INTERP using dl_iterate_phdr (thanks to @nsz-arm for this):

#include <elf.h>
#include <link.h>
#include <stdio.h>

static int cb(struct dl_phdr_info *info, size_t size, void *data)
{
	const char **ps = data;
	const char *base = (const char *)info->dlpi_addr;
	const ElfW(Phdr) *ph = info->dlpi_phdr;
	int phn = info->dlpi_phnum;

	for(int i=0; i < phn; i++) {
		if (ph[i].p_type == PT_INTERP) {
			*ps = base + ph[i].p_vaddr;
			return 1;
		}
	}
	return 0;
}

const char *get_interp()
{
	const char *s = 0;
	int r = dl_iterate_phdr(cb, &s);
	return r == 1 ? s : 0;
}

int main()
{
	printf("%s\n", get_interp());
}

it will print the interpreter (eg /lib/ld-musl-$ARCH.so.1). I think this can also be used to pick the correct /lib/libc.musl-$ARCH.so.*, which current implementation ('.musl-*.so.[0-9]*') may pick wrong.

However, using dl_iterate_phrd requires some C glue and configure.ac tests, since its not available on all platforms, and I'm not sure how to implement this C glue.

BTW, in the current implementation, running a 32 bit python binary on a 64 bit host will find 64 bit libraries:

$ docker run --rm -it ubuntu sh -c "dpkg --add-architecture i386 && apt-get update -y && apt-get install -y python3:i386 && python3 -c 'import platform; print(platform.architecture())' && python3 -c \"import ctypes.util; print(ctypes.util.find_library('systemd'))\" | xargs find /usr -name
...
('32bit', 'ELF')
/usr/lib/x86_64-linux-gnu/libsystemd.so.0

So I'm thinking that making things simple by parsing all /etc/ld-musl-*.path files might be good enough. What do you think?

I find that unfair. Current _findSoname_ldconfig search routine is not limited to GNU libc, but comment says that GNU libc is assumed. Why should we limit search routing for musl when we don't do it for GNU libc?

I don't know, why should we improve things?

I will refrain from commenting further. I have made my suggestions, but they were just that. It is up to the maintainers to make the call. This code gives me anxiety and I would not merge it as is. Everybody is happy until things break and you are the one who has to fix them 🙃 Anyway, not my place.

Perhaps you should write to the musl mailing list asking for advice on how to approach this.
https://musl.libc.org/support.html

Perhaps you should write to the musl mailing list asking for advice on how to approach this.
https://musl.libc.org/support.html

musl list won't help much.

this is a bug python dig for itself: the exposed api has semantics that is impossible to implement on a posix system including glibc and musl. i've seen this fail on glibc too and it will cause more trouble as python is used in more custom environments. so it's up to the python community to figure out what they want find_library to mean.

the dynamic linker search path logic is not a public api and can change (musl or glibc may introduce new mechanisms to configure it) and there is no guarantee that a library exists in the filesystem (which is e.g. the case for libpthread.so in musl: dlopen gives a handle to it but there is no libpthread.so in the filesystem or similar can happen with kernel mapped shared libraries like the vdso if they get exposed via dlopen or already loaded but then deleted files).

@FFY00 Thank you for taking time to review and comment. I understand that the code gives you anxiety. The whole file gives me anxiety 😄

I think the get_interp approach might be doable by adding it to Modules/_ctypes/callproc.c. This may also serve as a way to detect musl, since it seems to be tricky to detect musl. (no __MUSL__ c macro, by design, no detection of *-*-musl triples in configure.ac)

I did get help from musl devs on IRC who came up with the get_interp idea.

I'd like to see this fixed upstream, but I don't believe this PR or any of the variants proposed in the comments above are correct. Can someone explain to me what the purpose of find_library is? I don't feel like I can give meaningful input on how it should work without knowing its purpose.

OK, to summarize what I've found/been-told (please feel free to correct any of this if it's not right) the purpose is mainly (only?) getting a string to pass to CDLL (dlopen). For that purpose, find_library should really be nothing but a string transformation s/.*/lib&.so/. But this fails to work if the distro does not provide unversioned .so symlinks except in the dev packages. Of course this exposes a big flaw in the API: Python programs using find_library have no way to request a specific major version, but the FFI linkage is based on assumption of a particular library version's ABI. So really, programs already need to be skipping the faux-portability of find_library and instead doing CDLL("libfoo.so.X") for the known version X whose ABI they hard-coded. So AFAICT, any fix here is just going to be a band-aid on a fundamentally flawed interface. And basically the reason there's no interface (outside Python) for taking a library base name and producing the full filename or pathname is that you have to know this version for the operation to be meaningful.

I have updated the patch to only use the fallback when the interpreter contains the string "ld-musl-", and will also honor the correct /etc/ld-musl-$ARCH.path.

As @richfelker mentions, there is no meaningful way to ever use find_library, but since the function is here, and is used in the wild, I think it makes sense to emulate the (questionable) behavior to unbreak real life applications.

I think we could have a cleaner solution/workaround if we solve https://bugs.python.org/issue43112 first, which would allow us to detect/set musl at compile time.

I rebased and did git push --force to my branch but it seems like github broke.

Updated commit is found: ncopa@cec86ae

Python project requests that PRs don’t use rebase to avoid the bad github UX: https://devguide.python.org/pullrequest/

Python project requests that PRs don’t use rebase to avoid the bad github UX: https://devguide.python.org/pullrequest/

Oh, the PR is squashed when its merged... Sorry.

@merwok I rebased it again because there was some issues with github yesterday causing the rebase turn into a merge instead. I think should be fixed now and we do no more rebases. Sorry again.

I changed two excepts to be clause specific, not only the one you pointed out.

Thanks!

I don’t have much to add. Reading the whole logic gives me multiple OMG WHY moments, but assuming “make find_library work on musl like how it does on glibc” is what we want to do (which, from the discussion above, is?), the C and Python code to achieve it seems correct to me.

I don’t have much to add. Reading the whole logic gives me multiple OMG WHY moments, but assuming “make find_library work on musl like how it does on glibc” is what we want to do (which, from the discussion above, is?), the C and Python code to achieve it seems correct to me.

TBH I'd prefer to have the entire find_library removed, since I don't think there are any valid use cases for it, but I don't think we can.

It should give same result as on a glibc based system. The only difference I am aware of is that a 32 bit python on a 64 bit multiarch glibc system may give you the 64 bit library - which of course is wrong. On a musl system this wouldn't happen (I think).

Thank you for reviewing it and I hope it didn't give you any nightmares 😄

@ncopa Is there any chance you could rebase this? Thank you!

I can rebase. Can I do it with autoconf 2.71 or do I need set up an environment with autoconf 2.69?

The autoconf version should not change, and to make this convenient @tiran made an autoconf docker image that can be used.

Reminder that Python PR process requests no force pushes 🙂

Reminder that Python PR process requests no force pushes slightly_smiling_face

Sorry, forgot that.