Details

Reviewers

arichardson
jhb
manu
jrtc27
jmg
jhibbits
bdragon

Commits

rS368354: Add CFI start/end proc directives to arm64, i386, and ppc

Summary

Follow-up to r353959 and r368070: do the same for other architectures.

arm32 already seems to use its own .fnstart/.fnend directives, which
appear to be ARM-specific variants of the same thing. Likewise, MIPS
uses .frame directives.

Test Plan

Tinderbox running.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

cem created this revision.Nov 26 2020, 6:27 PM

Herald added a reviewer: manu. · View Herald TranscriptNov 26 2020, 6:27 PM

Herald added subscribers: jhibbits, emaste, andrew, imp. · View Herald Transcript

cem requested review of this revision.Nov 26 2020, 6:27 PM

Harbormaster completed remote builds in B35057: Diff 80034.Nov 26 2020, 6:27 PM

cem mentioned this in D27177: Add .cfi_{start,end}proc for RISC-V assembly functions.Nov 26 2020, 6:29 PM

jrtc27 requested changes to this revision.Nov 26 2020, 6:35 PM

jrtc27 added a subscriber: jrtc27.

jrtc27 added inline comments.

sys/powerpc/include/asm.h
112 ↗	(On Diff #80034)	This is the descriptor; you need it after `DOT_LABEL(name)`.
163 ↗	(On Diff #80034)	`END_SIZE(name)` whilst you're here?

This revision now requires changes to proceed.Nov 26 2020, 6:35 PM

cem added inline comments.Nov 26 2020, 7:07 PM

sys/powerpc/include/asm.h
112 ↗	(On Diff #80034)	Thanks, will fix.
163 ↗	(On Diff #80034)	Sorry, what about END_SIZE?

Move cfi_startproc in ELFv1 prologue after DOT_LABEL

Harbormaster completed remote builds in B35058: Diff 80035.Nov 26 2020, 7:08 PM

jrtc27 added inline comments.Nov 26 2020, 7:10 PM

sys/powerpc/include/asm.h
163 ↗	(On Diff #80034)	Currently there's no `.size` directive for powerpc32, unlike powerpc64. There's an END_SIZE macro available for that, though it looks like it's assuming powerpc64 without checking so you might want to just do a normal `.size`.

cem added inline comments.Nov 26 2020, 7:40 PM

sys/powerpc/include/asm.h
163 ↗	(On Diff #80034)	Sure, I'll add it. Thanks.

Add a lot of missing END directives. Tinderbox passes on arm64 and i386.
PowerPC mostly passes, except for all the PS3 HV calls which still need END
directives (and there are hundreds of these).

Herald added a reviewer: jmg. · View Herald TranscriptNov 26 2020, 9:11 PM

Harbormaster completed remote builds in B35063: Diff 80041.Nov 26 2020, 9:11 PM

cem added reviewers: jhibbits, bdragon.Nov 26 2020, 9:12 PM

cem marked 2 inline comments as done.

Add remaining missing END/ASEND directives in powerpc. Now tinderbox is clean.

Harbormaster completed remote builds in B35066: Diff 80044.Nov 26 2020, 10:34 PM

Fix ENTRY consumers in userspace (syscalls, MD assembly functions)

Harbormaster completed remote builds in B35135: Diff 80174.Dec 1 2020, 8:03 AM

Are there any outstanding objections? (I think I've addressed everything raised so far, but please correct me if I am mistaken.) Is there anyone who hasn't reviewed yet, but will have time to review in the next few days? Thanks.

Looks good to me. Obviously there might be some non-default build configurations that are missing end() calls, but if so they been be fixed later.

Probably best to wait for at least one more review though.

This revision was not accepted when it landed; it landed in state Needs Review.Dec 5 2020, 12:34 AM

Closed by commit rS368354: Add CFI start/end proc directives to arm64, i386, and ppc (authored by cem). · Explain Why

This revision was automatically updated to reflect the committed changes.

cem added a commit: rS368354: Add CFI start/end proc directives to arm64, i386, and ppc.

I'm fine with it on readthrough, I just hadn't gotten around to the actual tests. If it broke anything in powerpc land, it should be easy to fix. Thanks for tidying up the procedure annotations!

In D27387#614192, @bdragon wrote:

I'm fine with it on readthrough, I just hadn't gotten around to the actual tests. If it broke anything in powerpc land, it should be easy to fix. Thanks for tidying up the procedure annotations!

Thanks! It compiles (tinderbox, anyway) and shouldn't affect code generation, so I'm cautiously optimistic nothing broke. Please let me know if it broke something and I can help fix it :-).

Add CFI start/end proc directives to arm64, i386, and ppcClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 80347

head/lib/libc/powerpc/SYS.h

head/lib/libc/powerpc64/SYS.h

head/lib/libc/powerpc64/sys/cerror.S

head/lib/libthr/arch/i386/i386/_umtx_op_err.S

head/lib/msun/i387/e_logf.S

head/lib/msun/i387/e_remainderl.S

head/lib/msun/i387/e_sqrtl.S

head/lib/msun/i387/s_llrintl.S

head/lib/msun/i387/s_logbl.S

head/lib/msun/i387/s_lrintl.S

head/lib/msun/i387/s_remquol.S

head/lib/msun/i387/s_rintl.S

head/libexec/rtld-elf/aarch64/rtld_start.S

head/libexec/rtld-elf/powerpc/rtld_start.S

head/libexec/rtld-elf/powerpc64/rtld_start.S

head/stand/libsa/powerpc/_setjmp.S

head/stand/powerpc/kboot/host_syscall.S

head/stand/powerpc/uboot/start.S

head/sys/arm64/include/asm.h

head/sys/arm64/linux/linux_locore.asm

head/sys/arm64/linux/linux_support.s

head/sys/crypto/des/arch/i386/des_enc.S

head/sys/i386/bios/smapi_bios.S

head/sys/i386/include/asm.h

head/sys/powerpc/aim/locore.S

head/sys/powerpc/aim/locore64.S

head/sys/powerpc/aim/trap_subr64.S

head/sys/powerpc/booke/locore.S

head/sys/powerpc/booke/trap_subr.S

head/sys/powerpc/include/asm.h

head/sys/powerpc/mambo/mambocall.S

head/sys/powerpc/ofw/ofwcall32.S

head/sys/powerpc/ofw/ofwcall64.S

head/sys/powerpc/powernv/opalcall.S

head/sys/powerpc/powerpc/cpu_subr64.S

head/sys/powerpc/powerpc/setjmp.S

head/sys/powerpc/powerpc/support.S

head/sys/powerpc/powerpc/swtch32.S

head/sys/powerpc/powerpc/swtch64.S

head/sys/powerpc/ps3/ps3-hvcall.S

head/sys/powerpc/pseries/phyp-hvcall.S

Add CFI start/end proc directives to arm64, i386, and ppc
ClosedPublic
Actions

Revision Contents
Changeset List