Include debug symbols in the release for the lib #1271

krk · 2025-04-29T10:05:12Z

Motivation and context

It is useful to have the exact symbols corresponding to a release, for debugging purposes.

How has this been tested?

make
objdump -h build/lib/libasyncProfiler.so
objdump -h build/lib/libasyncProfiler.so.debug

Only libasyncProfiler.so.debug has the .debug* sections and libasyncProfiler.so retains symbols.

Sample sizes:

599K libasyncProfiler.so
4.0M libasyncProfiler.so.debug

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

krk · 2025-04-29T10:16:35Z

Unrelated failure:

FAIL [1/71] AllocTests.alloc took 11.187 s
java.util.concurrent.TimeoutException: Child process has not exited
	at one.profiler.test.TestProcess.waitForExit(TestProcess.java:297)
	at one.profiler.test.TestProcess.profile(TestProcess.java:320)
	at one.profiler.test.TestProcess.profile(TestProcess.java:302)
	at test.alloc.AllocTests.alloc(AllocTests.java:23)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at one.profiler.test.Runner.run(Runner.java:1[28](https://github.com/async-profiler/async-profiler/actions/runs/14728737409/job/41337688736?pr=1271#step:8:29))
	at one.profiler.test.Runner.main(Runner.java:188)

apangin · 2025-04-29T11:55:01Z

Makefile

-CXXFLAGS=-O3 -fno-exceptions -fno-omit-frame-pointer -fvisibility=hidden -std=c++11 $(CXXFLAGS_EXTRA)
 CPPFLAGS=
+CXXFLAGS=-O3 -fno-exceptions -fno-omit-frame-pointer -fvisibility=hidden -std=c++11 $(CXXFLAGS_EXTRA)
+LIB_PROFILER_CXXFLAGS=$(CXXFLAGS)


We already have many variables with flags, can we do without another one?
Can't we just add an option to CXXFLAGS conditionally?

apangin · 2025-04-29T11:56:26Z

Makefile

+endif
+
+ifeq ($(OS_TAG),linux)
+	$(STRIP) --only-keep-debug $@ -o $@.debug


Does .debug file get in the release package?

yes, e.g. https://github.com/async-profiler/async-profiler/actions/runs/14728842155/artifacts/3028736017

It should not. What's the purpose of having a separate .debug file then? The idea was to keep release package small, but provide debuginfo in a separate package.

I thought the idea was to keep the .so binary small, will provide in a separate package.

apangin · 2025-04-29T18:23:06Z

Makefile

+endif
+
+ifeq ($(OS_TAG),linux)
+	$(STRIP) --only-keep-debug $@ -o $@.debug


It should not. What's the purpose of having a separate .debug file then? The idea was to keep release package small, but provide debuginfo in a separate package.

apangin · 2025-04-29T18:25:27Z

Makefile

 endif

+ifeq ($(OS_TAG),linux)
+  CXXFLAGS += -ggdb


There is already ifeq ($(OS),Darwin) branch above that sets CXXFLAGS depending on a platform, should we move it there?

apangin · 2025-04-30T23:34:17Z

Makefile

+	$(STRIP) --only-keep-debug $@ -o build/sym/$(@F).debug
+	$(STRIP) -g $@
+	$(OBJCOPY) --add-gnu-debuglink=build/sym/$(@F).debug $@
+endif


Is it the right place to extract debuginfo? I think, for development purposes, one binary with debug symbols is more convenient; we just do redundant steps here. Should we strip binaries at the packaging step instead?

apangin · 2025-04-30T23:39:52Z

.github/workflows/test-and-publish-nightly.yml

          esac
-          echo "archive=$(basename $(find . -type f -iname "async-profiler-*" ))" >> $GITHUB_OUTPUT
+          echo "archive=$(find . -type f -name "async-profiler-*" -not -name "*-sym*" -exec basename {} \; || echo "")" >> $GITHUB_OUTPUT
+          echo "sym_archive=$(find . -type f -name "async-profiler-*-sym*" -exec basename {} \; | awk '{print; exit} END{if(NR==0) print "IGNORE"}')" >> $GITHUB_OUTPUT


No files were found with the provided path: IGNORE.

Isn't there a less tricky way to skip macOS?

Github actions step did not accept empty string as a valid value.

Will skip with an "is macos" check.

apangin · 2025-04-30T23:40:58Z

Makefile

+SYM_PACKAGE_NAME=$(PACKAGE_NAME)-sym
+SYM_PACKAGE_DIR=$(PACKAGE_DIR)-sym


Is it a common naming convention? I saw -debug or -debuginfo, but not -sym.

Moving to -debug.

apangin · 2025-05-01T12:37:05Z

.github/workflows/test-and-publish-nightly.yml

            macos*)
              brew install gcovr
              make COMMIT_TAG=$HASH FAT_BINARY=true release coverage -j
+              echo "debug_archive=UNSUPPORTED" >> $GITHUB_OUTPUT


Can it be a condition in the "Upload debug info" step instead?

Upload debug info fails if debug_archive is not set or is set to an empty string - or I don't understand the question.

My question is whether we can skip upload step altogether on macOS instead of uploading a dummy archive? Doesn't if: work for steps?

apangin · 2025-05-01T12:37:31Z

Makefile

 .PHONY: all jar release build-test test clean coverage clean-coverage build-test-java build-test-cpp build-test-libs build-test-bins test-cpp test-java check-md format-md

-all: build/bin build/lib build/$(LIB_PROFILER) build/$(ASPROF) jar build/$(JFRCONV) build/$(ASPROF_HEADER)
+all: build/bin build/lib build/debug build/$(LIB_PROFILER) build/$(ASPROF) jar build/$(JFRCONV) build/$(ASPROF_HEADER)


Is build/debug still used?

Yes, this creates the folder via

build/%: mkdir -p $@

Right, this is my question. make creates an empty build/debug directory, and I'm not sure why.

apangin · 2025-05-01T12:39:02Z

Makefile


-$(PACKAGE_DIR): all LICENSE README.md
+$(PACKAGE_DIR): all LICENSE README.md build/$(LIB_PROFILER_DEBUG)
+	rm -rf $@ || :


Why || :? rm -rf does not fail in case of missing file.

Good call, removing.

apangin · 2025-05-01T14:08:08Z

Makefile

+# To make building debug symbols idempotent, a rebuild of $(LIB_PROFILER) is required.
+# Remove stamp file to ensures $(LIB_PROFILER) rule re-runs if needed after this rule modifies it (strip -g).
+	rm -f build/$(LIB_STAMP)


I don't fully understand this complication. Updating build/$(LIB_PROFILER) in place doesn't seem a good idea, can we only update it in a package dir?

If it helps to keep things simpler, we can eliminate separate rules for $(DEBUG_PACKAGE_DIR) and $(LIB_PROFILER_DEBUG) and always build debug package under $(PACKAGE_NAME).tar.gz rule.

…profiler#1246)

…ler#1276)

krk · 2025-05-06T17:15:27Z

Unrelated test failure:

 FAIL [1/74] AllocTests.alloc took 7.224 s
java.lang.AssertionError
	at test.alloc.AllocTests.alloc(AllocTests.java:24)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
INFO: Running AllocTests.allocTotal...
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at one.profiler.test.Runner.run(Runner.java:128)
	at one.profiler.test.Runner.main(Runner.java:188)

Include debug symbols in the release for the lib

4aaae99

krk force-pushed the dbgsymlib branch from b9a1826 to 4aaae99 Compare April 29, 2025 10:11

empty

14d7dd5

apangin reviewed Apr 29, 2025

View reviewed changes

Use CXXFLAGS instead

00e8a69

apangin reviewed Apr 29, 2025

View reviewed changes

address comments

4b8baa1

apangin reviewed Apr 30, 2025

View reviewed changes

krk added 2 commits May 1, 2025 10:28

Merge branch 'master' into dbgsymlib

fdd9ba5

address comments

44b762d

apangin reviewed May 1, 2025

View reviewed changes

krk and others added 6 commits May 6, 2025 16:56

address comments

855eef0

Build and run tests on amazonlinux:2023 and amazonlinux:2 (async-…

08ca286

…profiler#1246)

address comments

ab28e18

Redirect only stderr from taskset in integration tests (async-profi…

0a761f3

…ler#1276)

Merge branch 'master' into dbgsymlib

ed0654f

empty

12b6aff

apangin merged commit b3907b4 into async-profiler:master May 6, 2025
30 of 32 checks passed

fandreuz mentioned this pull request May 15, 2025

[GHA] Print stack trace from core dumps #1295

Closed

		SYM_PACKAGE_NAME=$(PACKAGE_NAME)-sym
		SYM_PACKAGE_DIR=$(PACKAGE_DIR)-sym

Include debug symbols in the release for the lib #1271

Include debug symbols in the release for the lib #1271

Uh oh!

Conversation

krk commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation and context

How has this been tested?

Uh oh!

krk commented Apr 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apangin May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krk May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krk commented May 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krk commented Apr 29, 2025 •

edited

Loading

apangin May 1, 2025 •

edited

Loading

krk May 1, 2025 •

edited

Loading