cpython icon indicating copy to clipboard operation
cpython copied to clipboard

gh-95973: Add a new --with-dsymutil option to link debug information in macOS

Open pablogsal opened this issue 3 years ago • 15 comments

  • Issue: gh-95973

pablogsal avatar Aug 14 '22 01:08 pablogsal

Note to self: add -object_path_lto if --lto is used. (DONE)

pablogsal avatar Aug 14 '22 01:08 pablogsal

Note to self: add -object_path_lto if --lto is used.

The easy solution is to add this option unconditionally. It should be harmless for non-lto builds. That said, I haven't checked if the linker will complain/warn if the option is used with a non-lto build.

ronaldoussoren avatar Aug 15 '22 07:08 ronaldoussoren

The easy solution is to add this option unconditionally. It should be harmless for non-lto builds. That said, I haven't checked if the linker will complain/warn if the option is used with a non-lto build.

It doesn't as long as you also pass --lto. The most difficult part was generate unique names per linkage target. I have opted to place a templated expansion so the makefile inserts the rule name. The actual name of the file is not important as long as is unique per linked object but this helps identify what files belong to what linked objects.

pablogsal avatar Aug 15 '22 09:08 pablogsal

@ronaldoussoren If you have some time available, could you review and approve the PR?

pablogsal avatar Aug 15 '22 10:08 pablogsal

Ah, we are also missing installing the dLYB files when running make install.

pablogsal avatar Aug 15 '22 11:08 pablogsal

Ah, we are also missing installing the dLYB files when running make install.

Ok I fixed this, but it still doesn't handle --enable-shared or framework installations. I don't know how the later works so maybe we can do that in a separate PR

pablogsal avatar Aug 15 '22 12:08 pablogsal

Ok I fixed this, but it still doesn't handle --enable-shared or framework installations. I don't know how the later works so maybe we can do that in a separate PR

I am handling --enable-shared now in 281513b

pablogsal avatar Aug 15 '22 12:08 pablogsal

Ok I fixed this, but it still doesn't handle --enable-shared or framework installations. I don't know how the later works so maybe we can do that in a separate PR

I am handling --enable-shared now in 281513b

That should be enough to handle a framework installation as well.

The PR looks fine now, although I haven't tested yet if this is enough to see debug information when using an installed copy of python.

ronaldoussoren avatar Aug 15 '22 13:08 ronaldoussoren

Ok I fixed this, but it still doesn't handle --enable-shared or framework installations. I don't know how the later works so maybe we can do that in a separate PR

I am handling --enable-shared now in 281513b

That should be enough to handle a framework installation as well.

The PR looks fine now, although I haven't tested yet if this is enough to see debug information when using an installed copy of python.

Awesome. Give it a go and if everything looks good we can land it 👍

pablogsal avatar Aug 15 '22 15:08 pablogsal

Here is a backtrace from a installed Python with a --enable-shared:

libpython3.12.dylib was compiled with optimization - stepping may behave oddly; variables may not be available.
frame #1: 0x0000000100824dc8 libpython3.12.dylib`_PyEvalFramePushAndInit(tstate=0x0000000100accc88, func=0x0000000102c83ba0, locals=0x0000000000000000, args=0x000000016fdfc490, argcount=1, kwnames=0x0000000000000000) at ceval.c:6408:24 [opt]
   6405	    _PyFrame_InitializeSpecials(frame, func, locals, code);
   6406	    PyObject **localsarray = &frame->localsplus[0];
   6407	    for (int i = 0; i < code->co_nlocalsplus; i++) {
-> 6408	        localsarray[i] = NULL;
   6409	    }
   6410	    if (initialize_locals(tstate, func, localsarray, args, argcount, kwnames)) {
   6411	        assert(frame->owner != FRAME_OWNED_BY_GENERATOR);
(lldb)
frame #2: 0x000000010081905c libpython3.12.dylib`_PyEval_Vector(tstate=0x0000000100accc88, func=<unavailable>, locals=<unavailable>, args=<unavailable>, argcount=<unavailable>, kwnames=<unavailable>) at ceval.c:6465:34 [opt]
   6462	            Py_INCREF(args[i+argcount]);
   6463	        }
   6464	    }
-> 6465	    _PyInterpreterFrame *frame = _PyEvalFramePushAndInit(
   6466	        tstate, func, locals, args, argcount, kwnames);
   6467	    if (frame == NULL) {
   6468	        return NULL;
(lldb)
frame #3: 0x000000010073139c libpython3.12.dylib`PyObject_CallOneArg [inlined] _PyObject_VectorcallTstate(tstate=0x0000000100accc88, callable=0x0000000102c83ba0, nargsf=9223372036854775809, kwnames=0x0000000000000000) at pycore_call.h:92:11 [opt]
   89  	        Py_ssize_t nargs = PyVectorcall_NARGS(nargsf);
   90  	        return _PyObject_MakeTpCall(tstate, callable, args, nargs, kwnames);
   91  	    }
-> 92  	    res = func(callable, args, nargsf, kwnames);
   93  	    return _Py_CheckFunctionResult(tstate, callable, res, NULL);
   94  	}
   95
(lldb)
frame #4: 0x0000000100731370 libpython3.12.dylib`PyObject_CallOneArg(func=0x0000000102c83ba0, arg=0x00000001029f2910) at call.c:378:12 [opt]
   375 	    args[0] = arg;
   376 	    PyThreadState *tstate = _PyThreadState_GET();
   377 	    size_t nargsf = 1 | PY_VECTORCALL_ARGUMENTS_OFFSET;
-> 378 	    return _PyObject_VectorcallTstate(tstate, func, args, nargsf, NULL);
   379 	}
   380
   381

pablogsal avatar Aug 15 '22 16:08 pablogsal

The latest version of the PR doesn't work for me with a framework install, I get an error during installation due to the invocation of dsymutil in the altbininstall target.

I've configured with ../configure '--enable-framework' '--with-pydebug' '--with-dsymutil' --with-framework-name=PythonDebug. This uses an alternate framework name to avoid affecting the primary framework and makes it easier to clean up the results of this experiment.

End of the regular build looks ok:

/usr/bin/dsymutil python.exe
/usr/bin/dsymutil PythonDebug.framework/Versions/3.12/PythonDebug
/usr/bin/dsymutil Modules/array.cpython-312d-darwin.so
/usr/bin/dsymutil Modules/_asyncio.cpython-312d-darwin.so
/usr/bin/dsymutil Modules/_bisect.cpython-312d-darwin.so
/usr/bin/dsymutil Modules/_contextvars.cpython-312d-darwin.so
...
/usr/bin/dsymutil Modules/xxlimited.cpython-312d-darwin.so
/usr/bin/dsymutil Modules/xxlimited_35.cpython-312d-darwin.so

But installing fails with:

if test -d "PythonDebug.framework/Versions/3.12/PythonDebug.dSYM"; then \
		echo /usr/bin/dsymutil /Library/Frameworks/PythonDebug.framework/Versions/3.12/lib/PythonDebug.framework/Versions/3.12/PythonDebug; \
		/usr/bin/dsymutil /Library/Frameworks/PythonDebug.framework/Versions/3.12/lib/PythonDebug.framework/Versions/3.12/PythonDebug; \
	fi
/usr/bin/dsymutil /Library/Frameworks/PythonDebug.framework/Versions/3.12/lib/PythonDebug.framework/Versions/3.12/PythonDebug
error: cannot parse the debug map for '/Library/Frameworks/PythonDebug.framework/Versions/3.12/lib/PythonDebug.framework/Versions/3.12/PythonDebug': No such file or directory
make: *** [altbininstall] Error 1

The error is expected here because /Library/Frameworks/PythonDebug.framework/Versions/3.12/lib/PythonDebug.framework/Versions/3.12/PythonDebug does not exist, not the two PythonDebug.framework entries in this path.

The solution is to change LIBDIR to PYTHONFRAMEWORKPREFIX in the last command in that block. That is, to:

        if test -d "$(LDLIBRARY).dSYM"; then \
                echo $(DSYMUTIL_PATH) $(DESTDIR)$(PYTHONFRAMEWORKPREFIX)/$(INSTSONAME); \
                $(DSYMUTIL_PATH) $(DESTDIR)$(PYTHONFRAMEWORKPREFIX)/$(INSTSONAME); \
        fi

Although that likely breaks a regular shared installation...

That appears to be enough, I get this backtrace after moving the source directory aside:

(lldb) thread backtrace
* thread #2, queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000100986490 PythonDebug`PyList_New(size=0) at listobject.c:153:9 [opt]
    frame #1: 0x0000000100aa3ec0 PythonDebug`_PyGC_Init(interp=0x0000000100d256e0) at gcmodule.c:164:24 [opt]
    frame #2: 0x0000000100a759b8 PythonDebug`pycore_interp_init(tstate=0x0000000100d40d20) at pylifecycle.c:834:14 [opt]
    frame #3: 0x0000000100a755ec PythonDebug`pyinit_config(runtime=<unavailable>, tstate_p=0x000000016fdff208, config=0x000000016fdff028) at pylifecycle.c:900:14 [opt]
    frame #4: 0x0000000100a74440 PythonDebug`pyinit_core(runtime=<unavailable>, src_config=0x000000016fdff2a0, tstate_p=0x000000016fdff208) at pylifecycle.c:1063:18 [opt]
    frame #5: 0x0000000100a742d0 PythonDebug`Py_InitializeFromConfig(config=0x000000016fdff2a0) at pylifecycle.c:1248:14 [opt]
    frame #6: 0x0000000100aa3df0 PythonDebug`pymain_init(args=0x000000016fdff4f0) at main.c:67:14 [opt]
    frame #7: 0x0000000100aa2e6c PythonDebug`pymain_main(args=<unavailable>) at main.c:710:23 [opt]
    frame #8: 0x0000000100aa2ed0 PythonDebug`Py_BytesMain(argc=<unavailable>, argv=<unavailable>) at main.c:743:12 [opt]
    frame #9: 0x0000000100003fa4 PythonDebug`main(argc=<unavailable>, argv=<unavailable>) at python.c:15:12 [opt]
    frame #10: 0x000000010001108c dyld`start + 520

And do get a source listing when I keep the source directory as-is, but remove the build directory.

ronaldoussoren avatar Aug 15 '22 20:08 ronaldoussoren

Ah, I think we need to make that conditional to the framework vs --enable-shared installation

pablogsal avatar Aug 15 '22 20:08 pablogsal

@ronaldoussoren can you check if commit https://github.com/python/cpython/pull/95974/commits/f544cfad9449eccdd209970ca0c2b59373c54246 does the trick?

pablogsal avatar Aug 15 '22 21:08 pablogsal

I'll review and test this in the next couple of days.

ned-deily avatar Aug 16 '22 02:08 ned-deily

@ronaldoussoren can you check if commit f544cfa does the trick?

This version works for me.

One minor issue: The makefile in Mac doesn't do the dsymutil dance yet. That only affects the stub executable though, and that only execs the real interpreter. It is unlikely that anyone will end up debugging that binary.

One thing I haven't looked into yet, and could be looked at later: Debugging the x86_64 fork of a Universal2 build of the framework doesn't work for me (error from LLDB and SIGABRT from the launched executable). That feels like a different issue though, esp. given that my development laptop is using an Xcode 14 beta.

ronaldoussoren avatar Aug 16 '22 08:08 ronaldoussoren

I plan to land this at the end of the week if possible to avoid merge conflicts. If you need more time to review, please indicate this in the comments :)

pablogsal avatar Aug 23 '22 09:08 pablogsal

I plan to land this at the end of the week

On it

ned-deily avatar Aug 24 '22 22:08 ned-deily

Status check is done, and it's a success ✅ .

miss-islington avatar Aug 27 '22 00:08 miss-islington