All commit authors signed the Contributor License Agreement.

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

On success, these functions return the address associated with
symbol. On failure, they return NULL; the cause of the error can
be diagnosed usingdlerror(3).

This is direct quotation from the man page you linked.
Why you think this is correct?

dlsym(), a return value of NULL does not necessarily indicate an error [1].

@efimov-mikhail

Indeed, I failed to mention that at some point the man page contradicts itself, and I believe that the part you quoted is something the maintainer forgot to remove.

[edit: added this part]

I say this because further up, in the DESCRIPTION, the man page states:

In unusual cases (see NOTES) the value of the symbol couldactually be NULL.  Therefore, a NULL return from dlsym() need notindicate an error.  The correct way to distinguish an error froma symbol whose value is NULL is to call dlerror(3) to clear anyold error conditions, then call dlsym(), and then call dlerror(3)again, saving its return value into a variable, and check whetherthis saved value is not NULL.

[end of edit]

Additionally, further down, in the NOTES segment, the man page states:

There are several scenarios when the address of a global symbolis NULL.  For example, a symbol can be placed at zero address bythe linker, via a linker script or with --defsym command-lineoption.  Undefined weak symbols also have NULL value.  Finally,the symbol value may be the result of a GNU indirect function(IFUNC) resolver function that returns NULL as the resolvedvalue.  In the latter case, dlsym() also returns NULL withouterror.  However, in the former two cases, the behavior of GNUdynamic linker is inconsistent: relocation processing succeedsand the symbol can be observed to have NULL value, but dlsym()fails and dlerror() indicates a lookup error.

Understood. IMO, it would be better if we can make an actual reproducible test case, and put this case into regular testing procedure.

See reproducer here:#126554 (comment)

@ZeroIntensity

1. I understand that I can just click "Commit" on your proposed changes, and then we can squash before merging
2. With some grepping I found out that we probably also need to change

   cpython/Modules/_ctypes/callproc.c

   Line 1626 inb19d12f

   ptr=dlsym((void*)handle,name);

   
   using the same approach.
3. Regarding the test case, I'm thinking something similar tohttps://github.com/python/cpython/blob/b19d12f44735583e339ee6ee560588fcf22633b4/Lib/test/test_ctypes/test_find.py, where we will:
   1. Write a fixed string (the content offoo.c from the reproducer),
   2. Compile (as does thetest_find testcase) into a shared lib
   3. Load, and access thefoo object.

(2) is done, pushing a commit. I also added a comment that explicitly states why we calldlerror() beforedlsym().

From a project maintaner's POV, if we have 3 instances of this phenomenon, do we add a comment disclaimer at every instance? (This is the approach I took, but I'm unsure what the best / most common practice is.)

Proceeding with the test case (3), will let you know when ready.

• I understand that I can just click "Commit" on your proposed changes, and then we can squash before merging

Yup. In fact, that's the preferred workflow for us. Force pushing onto one commit is very much discouraged.

2. With some grepping I found out that we probably also need to change

That looks like a problem too, yeah. Thanks!

I'll create a separate issue if I can come up with a repro for the locale issue. (I'll CC you on it, if you want to author that PR as well.)

Added a commit with the test case undertest_ctypes/test_dlerror. PTAL.

Running on the PR branch, it passes :)

On HEAD of main, it explodes (segfault).

Do we have any way of handling this gracefully (maybe running a subprocess with the interpreter?), or is it the correct way to go?

Do we have any way of handling this gracefully (maybe running a subprocess with the interpreter?)

Could you please explain, what exactly are you speaking about?
Gracefully handling what?

If we want to be pedantic, we must check all 3 code paths that invokedlsym().

In this case, we only check one (PyCFuncPtr_FromDll).

On one hand, since our amendment is the same across all 3 funcs, we should theoretically be OK.

Do we have any way of handling this gracefully (maybe running a subprocess with the interpreter?)

Could you please explain, what exactly are you speaking about? Gracefully handling what?

@efimov-mikhail

If I run the test on the main branch, the Python interpreter gets a segmentation fault.

What I'm saying is that in a scenario where someone runs all tests, when the./python -m test process encounters this test, it will exit with a segmentation fault and terminate the testing procedure. It won't mark the test as failing. It will explode.

Maybe I have misunderstood something regarding the philosophy of testing.

Run the test in a subprocess and check the return code.

I'm not sure if the ASan failures are related.

🤖 New build scheduled with the buildbot fleet by@ZeroIntensity for commitab22349 🤖

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

Let's try the build bots again and see if it holds up now.

Buildbots are all good, the one failure is a problem with the DRC API that got merged the other day, which is unrelated. I'll submit a (separate) patch to fix that. I think we're good to merge!

Thanks@grgalex for the PR, and@encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

Sorry,@grgalex and@encukou, I could not cleanly backport this to3.12 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker 8717f792f7cc343599dc60daf80630cceb66145b 3.12

GH-126861 is a backport of this pull request to the3.13 branch.

@grgalex, you've finished it! Congrats with your first contribution to CPython!

Thanks for the guidance@efimov-mikhail,@ZeroIntensity,@encukou.

Looking forward to collaborating again in the future!

3.13 backports were done but not 3.12;@encukou Do you want the 3.12 backport in the end or can we close the issue with only 3.13 and 3.14 being affected?

AsI said, I'm fine not backporting. If anyone disagrees, open the PR :)

GH-127764 is a backport of this pull request to the3.12 branch.