Musl support #2605

gnurizen · 2024-03-07T22:50:41Z

Add support for pulling unwind info from debug link that is available
locally, if debug link can't be found dwarf unwinding will fail. Future work
may have the agent to pull debuginfo from parca server and allow
dwarf unwinding w/o debug link files available locally.

If the debug link file doesn't match the CRC report an error and don't
use it.

Fix finder so that it doesn't return debuglink matches pointing back at
the exe itself, this just leads to spurious crc failures.

kakkoyun · 2024-03-09T11:39:43Z

pkg/objectfile/object_file.go

@@ -30,7 +30,7 @@ import (
 // ObjectFile represents an executable or library file.
 // It handles the lifetime of the underlying file descriptor.
 type ObjectFile struct {
-	p *Pool
+	Pool *Pool


Why do we export this?

Ignore, WIP temporary hack.

kakkoyun · 2024-03-09T11:40:55Z

pkg/stack/unwind/unwind_table.go

@@ -200,37 +203,84 @@ func (ptb *UnwindTableBuilder) PrintTable(writer io.Writer, file *objectfile.Obj
 }

 func ReadFDEs(file *objectfile.ObjectFile) (frame.FrameDescriptionEntries, elf.Machine, error) {
+	// Try the normal path, if it fails try the alternatives and go with them if they


It worth checking our debuginfo package. Especially the Find. I believe we can reuse some code pieces.

Thanks for the tip! I especially want to nail the CRC checking before landing this.

gnurizen · 2024-05-20T15:04:44Z

@umanwizard can you review this? Thanks!

umanwizard · 2024-05-30T18:34:50Z

bpf/unwinders/native.bpf.c

    FIND_UNWIND_CHUNK_NOT_FOUND = 5,
+    FIND_UNWIND_CHUNK_NOT_FOUND_FOR_PC = 6,


Can you leave a comment explaining what the difference is between these?

umanwizard · 2024-05-30T18:36:17Z

bpf/unwinders/native.bpf.c

+    LOG("[error] could not find chunk for adjusted ip=0x%llx, mapping idx %d, mapping exe id 0x%llx", adjusted_pc, index,
+        proc_info->mappings[index].executable_id);


Please make sure this still verifies on x86 5.4. IIRC this was exactly the logging I had to remove in order to get it to work.

It seems to work on ubuntu 20.04 LTS (5.14 kernel). Are you sure you weren't just fixing printfs with more than 3 args?

umanwizard · 2024-05-30T18:37:17Z

bpf/unwinders/native.bpf.c

@@ -1402,7 +1411,7 @@ int entrypoint(struct bpf_perf_event_data *ctx) {
        if (!is_debug_enabled_for_thread(per_process_id)) {
            bump_unwind_total_filter_misses();
            BUMP_UNWIND_FAILED_COUNT(per_process_id, missed_filter);
-            LOG("[debug] pid %u didn't match filter, ignoring.", per_process_id);
+            // LOG("[debug] pid %u didn't match filter, ignoring.", per_process_id);


Why is this commented out now?

Too verbose

umanwizard · 2024-05-30T18:45:09Z

pkg/stack/unwind/compact_unwind_table.go

+		// We use pc=0 as a sentinel so we can't have that, however some debug
+		// files have FDE's with offset 0, usually empty or tiny but not always.
+		// There's probably a better way to filter these but this works, we
+		// panic over in maps.setUnwindTableForMapping if any start at 0.
+		// An example is the alpine "ld-musl-x86_64.so.1.debug".


Can you say more about this / give an example / post some relevant output of objdump -WF and nm ? What function in musl does that fde correspond to? Wouldn't that mean the process is expecting to be able to map things a the zero page?

I tried to repro it and couldn't, I wonder if it was from parsing .debug_frame wrong. We'll see what more testing shows.

umanwizard · 2024-05-30T18:50:35Z

pkg/stack/unwind/unwind_table.go

+	// and external debuginfo's.
+	var debugLinkFDEs frame.FrameDescriptionEntries
+	if uc.finder != nil {
+		debugPath, err := uc.finder.Find(context.TODO(), root, file)


why context.TODO

Well we should have a legit context here but we don't, this is a bread crumb to remember to wire in a real context.

umanwizard · 2024-05-30T18:51:55Z

pkg/stack/unwind/unwind_table.go

+		if err != nil {
+			level.Debug(uc.logger).Log("msg", "no .debug file found for ", "exe", exe, "err", err)
+		} else if debugPath != "" {


Can we check specifically that this is an expected type of error, so we can avoid swallowing possibly real errors?

umanwizard · 2024-05-30T18:52:27Z

pkg/stack/unwind/unwind_table.go

-		secName = ".debug_frame"
+			if err != nil {
+				if isUnexpected(err) {
+					level.Warn(uc.logger).Log("msg", "error reading FDEs from .ebug_frame section in debuglink", "err", err, "exe", exe, "debuglink", debugPath)


Suggested change

level.Warn(uc.logger).Log("msg", "error reading FDEs from .ebug_frame section in debuglink", "err", err, "exe", exe, "debuglink", debugPath)

level.Warn(uc.logger).Log("msg", "error reading FDEs from .debug_frame section in debuglink", "err", err, "exe", exe, "debuglink", debugPath)

Nice catch!

umanwizard · 2024-05-31T04:10:00Z

I still don’t have approval rights, but it looks good to me overall, so merge at your discretion!

Add support for pulling symbols from debug link that is available locally, if debug link can't be found unwinding may fail. Future work may have the agent to pull debuginfo from parca server and allow dwarf unwinding w/o debug link files available locally. If the debug link file doesn't match the CRC report an error and don't use it. Fix finder so that it doesn't return debuglink matches pointing back at the exe itself, this just leads to spurious crc failures. Don't use -v so we only get logs on test failures report a successful sample when we miss on dwarf symbol Fix arm64/musl gilstate addressing Skip alpine 3.3 on arm64 better integration test helper

…g_frame wrong

gnurizen requested a review from a team as a code owner March 7, 2024 22:50

gnurizen force-pushed the musl-debug-test branch 2 times, most recently from d2caf14 to 36030d4 Compare March 7, 2024 23:00

gnurizen marked this pull request as draft March 7, 2024 23:00

gnurizen force-pushed the musl-debug-test branch 3 times, most recently from ac62dcd to 355909b Compare March 8, 2024 18:07

kakkoyun reviewed Mar 9, 2024

View reviewed changes

gnurizen force-pushed the musl-debug-test branch 4 times, most recently from cb1f1bb to 996430a Compare March 13, 2024 06:19

gnurizen force-pushed the musl-debug-test branch from 996430a to 09a0791 Compare March 28, 2024 22:51

gnurizen changed the title ~~[WIP] Musl support~~ Musl support Mar 29, 2024

gnurizen force-pushed the musl-debug-test branch 5 times, most recently from df30637 to 48c6449 Compare March 30, 2024 11:36

gnurizen force-pushed the musl-debug-test branch from 48c6449 to e1127bc Compare April 9, 2024 22:34

gnurizen force-pushed the musl-debug-test branch 3 times, most recently from 82cccd3 to 2748eb0 Compare April 18, 2024 11:18

gnurizen marked this pull request as ready for review April 18, 2024 11:32

gnurizen force-pushed the musl-debug-test branch 2 times, most recently from fbc4815 to 4b9ea4f Compare April 22, 2024 14:30

gnurizen force-pushed the musl-debug-test branch 4 times, most recently from 159894e to a298bcc Compare May 14, 2024 17:34

gnurizen force-pushed the musl-debug-test branch from de86bac to 02af69c Compare May 14, 2024 21:20

gnurizen force-pushed the musl-debug-test branch 5 times, most recently from 9802b36 to bf59f6f Compare May 20, 2024 20:37

brancz approved these changes May 21, 2024

View reviewed changes

gnurizen force-pushed the musl-debug-test branch 3 times, most recently from 7c639ec to 06fbe08 Compare May 23, 2024 20:25

gnurizen force-pushed the musl-debug-test branch 5 times, most recently from 01b606a to 175fe98 Compare May 30, 2024 17:22

gnurizen marked this pull request as draft May 30, 2024 17:35

gnurizen marked this pull request as ready for review May 30, 2024 17:43

umanwizard reviewed May 30, 2024

View reviewed changes

gnurizen added 7 commits May 31, 2024 07:32

Don't fail build for arm64 java tests, ie make them like amd64

1986b4c

Add explanation for gilLibCDelta

ee90237

remove this and see if it matters, might have been from parsing .debu…

0872563

…g_frame wrong

debug expected errors, warn on others

ec817f5

Fix typo

6d4ed82

improve commentary

9d17349

gnurizen force-pushed the musl-debug-test branch from 42b017a to 9d17349 Compare May 31, 2024 11:35

gnurizen enabled auto-merge (squash) May 31, 2024 11:37

gnurizen merged commit 63f7cc9 into parca-dev:main May 31, 2024
11 of 12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Musl support #2605

Musl support #2605

gnurizen commented Mar 7, 2024 •

edited

Loading

kakkoyun Mar 9, 2024

gnurizen Mar 13, 2024

kakkoyun Mar 9, 2024

gnurizen Mar 13, 2024

gnurizen commented May 20, 2024

umanwizard May 30, 2024

gnurizen May 31, 2024

umanwizard May 30, 2024

gnurizen May 30, 2024

gnurizen May 31, 2024

umanwizard May 30, 2024

gnurizen May 30, 2024

umanwizard May 30, 2024

gnurizen May 31, 2024

umanwizard May 30, 2024

gnurizen May 31, 2024

umanwizard May 30, 2024

gnurizen May 31, 2024

umanwizard May 30, 2024

gnurizen May 31, 2024

umanwizard commented May 31, 2024

		FIND_UNWIND_CHUNK_NOT_FOUND = 5,
		FIND_UNWIND_CHUNK_NOT_FOUND_FOR_PC = 6,

		LOG("[error] could not find chunk for adjusted ip=0x%llx, mapping idx %d, mapping exe id 0x%llx", adjusted_pc, index,
		proc_info->mappings[index].executable_id);

	level.Warn(uc.logger).Log("msg", "error reading FDEs from .ebug_frame section in debuglink", "err", err, "exe", exe, "debuglink", debugPath)
	level.Warn(uc.logger).Log("msg", "error reading FDEs from .debug_frame section in debuglink", "err", err, "exe", exe, "debuglink", debugPath)

Musl support #2605

Musl support #2605

Conversation

gnurizen commented Mar 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnurizen commented May 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umanwizard commented May 31, 2024

gnurizen commented Mar 7, 2024 •

edited

Loading