Page MenuHomeFreeBSD

csu: Add a stop indicator to _start to satisfy unwinders on aarch64
ClosedPublic

Authored by dchagin on Jun 20 2023, 12:21 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, May 18, 8:11 PM
Unknown Object (File)
Sat, May 18, 8:11 PM
Unknown Object (File)
Sat, May 18, 8:11 PM
Unknown Object (File)
Sat, May 18, 8:05 PM
Unknown Object (File)
Fri, May 17, 5:57 PM
Unknown Object (File)
Fri, May 17, 5:57 PM
Unknown Object (File)
Fri, May 17, 5:57 PM
Unknown Object (File)
Fri, May 17, 5:57 PM

Details

Summary

The right unwinding stop indicator should be CFI-undefined PC.
https://dwarfstd.org/doc/Dwarf3.pdf - page 118:
If a Return Address register is defined in the virtual unwind table,
and its rule is undefined (for example, by DW_CFA_undefined), then
there is no return address and no call address, and the virtual
unwind of stack activations is complete.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dchagin retitled this revision from csu: Add an annotated _start to satisfy unwinders to csu: Add an annotated _start to satisfy unwinders on x86_64.Jun 20 2023, 12:32 PM
dchagin added a reviewer: kib.
lib/csu/amd64/crt1_s.S
34 ↗(On Diff #123489)

%rbp id already cleared by kernel; callq breaks stack alignment.

dchagin edited the summary of this revision. (Show Details)

tack alignment is fixed, merged x86 and arm64 changes into one commit
due o -fno-asynchronous-unwind-tables removal requirments

dchagin retitled this revision from csu: Add an annotated _start to satisfy unwinders on x86_64 to csu: Add a stop indicator to _start to satisfy unwinders on x86 and arm64.Jun 26 2023, 11:52 AM
dchagin edited the summary of this revision. (Show Details)
dchagin added a reviewer: dim.
dchagin added inline comments.
lib/csu/amd64/crt1_s.S
34 ↗(On Diff #123489)

after rtld it is not 0

lib/csu/aarch64/crt1_c.c
35

why have this. It's useless here since there's no _start here.

lib/csu/aarch64/crt1_c.c
35

the email makes this sound confusing: Why have _start declared here at all. Seems like a nop since _start isn't referenced in this file.

lib/csu/aarch64/crt1_c.c
35

just due to attributes, _start is not referenced anywhere since it is entry point

Hmm this might also allow us to revert rG5866c369e4fd917c0d456f0f10b92ee354b82279, which was added to work around https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256864 . As far as I remember, this only affected aarch64 though, and that is tricky for me to test.

In D40623#927474, @dim wrote:

Hmm this might also allow us to revert rG5866c369e4fd917c0d456f0f10b92ee354b82279, which was added to work around https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=256864 . As far as I remember, this only affected aarch64 though, and that is tricky for me to test.

for aarch64 unwinding through signal frame is not implemented, it would be nice to add something like https://reviews.llvm.org/rGc82deed6764cbc63966374baf9721331901ca958#change-9Mv5koDaVLbn

lib/csu/amd64/crt1_s.S
32 ↗(On Diff #123756)

Perhaps add a comment that the only reason for _start existence is to issue .cfi_undefined.

That said, how hard is to rewrite _start1 in asm?

lib/csu/amd64/crt1_s.S
32 ↗(On Diff #123756)

https://reviews.freebsd.org/D40780

however I don't know what is GCRT and how test under it

dchagin retitled this revision from csu: Add a stop indicator to _start to satisfy unwinders on x86 and arm64 to csu: Add a stop indicator to _start to satisfy unwinders on aarch64.Jun 30 2023, 8:24 AM
dchagin edited the summary of this revision. (Show Details)
dchagin removed reviewers: kib, dim.
dchagin edited the summary of this revision. (Show Details)

Fix how address of a main is resolved, follow p 7.3.1 of sysv ABI for Aarch64

ugh, fix GCRT case, gprof fine

lib/csu/aarch64/crt1_s.S
50

If you're adding support for the large code model why didn't you use the following sequence? We know the code length will be under 1MiB so it is safe to use.

ldr x0, .L0
.L0
.xword var

Thank you, mostly due to I doubts as Im new to it, so I used this approach, fixed now.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 7 2023, 4:56 PM
This revision was automatically updated to reflect the committed changes.

Is there a reason you didn't add __asm__ volatile(".cfi_undefined x30"); to the start of the C function? In my testing with CheriBSD this appears to work in the same way (libunwind tests fail before, succeed after).

Is there a reason you didn't add __asm__ volatile(".cfi_undefined x30"); to the start of the C function? In my testing with CheriBSD this appears to work in the same way (libunwind tests fail before, succeed after).

For context, here is the change I just submitted to CheriBSD: https://github.com/CTSRD-CHERI/cheribsd/pull/1859

Is there a reason you didn't add __asm__ volatile(".cfi_undefined x30"); to the start of the C function? In my testing with CheriBSD this appears to work in the same way (libunwind tests fail before, succeed after).

libunwind version? AFAIR, I fixed this in the libunwind but 8.1 libunwind version has not been released yet, seems

Is there a reason you didn't add __asm__ volatile(".cfi_undefined x30"); to the start of the C function? In my testing with CheriBSD this appears to work in the same way (libunwind tests fail before, succeed after).

stock aarch64 main, fresh libunwind:

make check-TESTS
PASS: test-proc-info
PASS: test-static-link
PASS: test-strerror
PASS: Gtest-bt
PASS: Ltest-bt
PASS: Gtest-init
PASS: Ltest-init
PASS: Gtest-concurrent
PASS: Ltest-concurrent
PASS: Gtest-trace
PASS: Ltest-trace
PASS: Ltest-mem-validate
PASS: test-async-sig
PASS: test-flush-cache
PASS: test-init-remote
PASS: test-mem
PASS: test-reg-state
PASS: Ltest-varargs
PASS: Ltest-nomalloc
PASS: Ltest-nocalloc
PASS: Lrs-race
PASS: Ltest-init-local-signal
PASS: Gtest-exc
PASS: Ltest-exc
PASS: Gtest-resume-sig
PASS: Ltest-resume-sig
PASS: Gtest-resume-sig-rt
PASS: Ltest-resume-sig-rt
PASS: test-ptrace
PASS: test-setjmp
Illegal instruction (core dumped)
XFAIL: Garm64-test-sve-signal
Illegal instruction (core dumped)
XFAIL: Larm64-test-sve-signal
PASS: aarch64-test-plt
FAIL: run-check-namespace
PASS: run-ptrace-mapper

PASS: run-ptrace-misc

Testsuite summary for libunwind 1.8-pre

  1. TOTAL: 36
  2. PASS: 33
  3. SKIP: 0
  4. XFAIL: 2
  5. FAIL: 1
  6. XPASS: 0
  7. ERROR: 0