Page MenuHomeFreeBSD
Differential D20970

Adds signal number format to kern.corefile
ClosedPublic
Actions

Authored by leimy2k_gmail.com on Jul 16 2019, 3:02 PM.
Tags
None
Referenced Files
F114934565: D20970.id59810.diff
Fri, Apr 18, 6:05 PM
F114904645: D20970.id.diff
Fri, Apr 18, 8:54 AM
Unknown Object (File)
Mar 6 2025, 9:11 PM
Unknown Object (File)
Jan 18 2025, 10:02 PM
Unknown Object (File)
Jan 1 2025, 5:44 AM
Unknown Object (File)
Nov 26 2024, 9:27 AM
Unknown Object (File)
Nov 23 2024, 11:46 PM
Unknown Object (File)
Nov 4 2024, 8:39 PM
View All 55 Files
Subscribers
imp

Details

Reviewers
vangyzen
markj
rwatson
Group Reviewers
manpages
Commits
rS350043: Adds signal number format to kern.corefile
Summary

Add format capability to core file names to include signal
that generated the core. This can help various validation workflows
where all cores should not be considered equally (SIGQUIT is often
intentional and not an error unlike SIGSEGV or SIGBUS)

Submitted By: David Leimbach (leimy2k@gmail.com)

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25370
Build 24017: arc lint + arc unit

Event Timeline

leimy2k_gmail.com created this revision.Jul 16 2019, 3:02 PM
Harbormaster completed remote builds in B25367: Diff 59808.Jul 16 2019, 3:02 PM
leimy2k_gmail.com added a reviewer: vangyzen.Jul 16 2019, 3:04 PM

I realize procstat can list out the signal from a core file using ELF hints, but some workflows where I work use compressed corefiles, and the work to decompress them vs scanning the filename is obviously a lot more.

leimy2k_gmail.com added a comment.Jul 16 2019, 3:08 PM

Did some basic testing with the following code.

#include <sys/types.h>
#include <signal.h>
#include <unistd.h>

int main () {
    kill(getpid(), 3);
    return 0;
}

And used:

sudo sysctl kern.corefile="%N_sig%S.core"

Ended up with

-rw-------   1 leimy  leimy  8966144 Jul 11 17:45 a.out_sig3.core
leimy2k_gmail.com added reviewers: markj, rwatson.Jul 16 2019, 3:10 PM
markj accepted this revision.Jul 16 2019, 3:15 PM
markj added inline comments.
sys/kern/kern_sig.c
3491

This case should be sorted before 'U'.

This revision is now accepted and ready to land.Jul 16 2019, 3:15 PM
vangyzen accepted this revision.Jul 16 2019, 3:18 PM
vangyzen added inline comments.
sys/kern/kern_sig.c
3491

In the man page, too.

leimy2k_gmail.com updated this revision to Diff 59810.Jul 16 2019, 3:20 PM

Fix the sort order

This revision now requires review to proceed.Jul 16 2019, 3:20 PM
Harbormaster completed remote builds in B25368: Diff 59810.Jul 16 2019, 3:20 PM
leimy2k_gmail.com updated this revision to Diff 59811.Jul 16 2019, 3:23 PM
leimy2k_gmail.com marked an inline comment as done.

Make man page sort order match code sort order.

Harbormaster completed remote builds in B25370: Diff 59811.Jul 16 2019, 3:23 PM
leimy2k_gmail.com marked an inline comment as done.Jul 16 2019, 3:24 PM

Fixed up the sort order per review feedback.

vangyzen accepted this revision.Jul 16 2019, 3:25 PM
This revision is now accepted and ready to land.Jul 16 2019, 3:25 PM
Closed by commit rS350043: Adds signal number format to kern.corefile (authored by vangyzen). · Explain WhyJul 16 2019, 3:51 PM
This revision was automatically updated to reflect the committed changes.
vangyzen added a commit: rS350043: Adds signal number format to kern.corefile.
Herald added a reviewer: manpages. · View Herald TranscriptJul 16 2019, 3:51 PM
Herald added a subscriber: imp. · View Herald Transcript

Revision Contents
Changeset List

PathSize
share/
man/
man5/
2 lines
sys/
kern/
7 lines

Diff 59811

View Options

share/man/man5/core.5

Show All 28 Lines
LDRSRC= ${BOOTSRC}/common LDRSRC= ${BOOTSRC}/common
LIBLUASRC= ${BOOTSRC}/liblua LIBLUASRC= ${BOOTSRC}/liblua
LUASRC= ${SRCTOP}/contrib/lua/src LUASRC= ${SRCTOP}/contrib/lua/src
SASRC= ${BOOTSRC}/libsa SASRC= ${BOOTSRC}/libsa
SYSDIR= ${SRCTOP}/sys SYSDIR= ${SRCTOP}/sys
UBOOTSRC= ${BOOTSRC}/uboot UBOOTSRC= ${BOOTSRC}/uboot
ZFSSRC= ${SASRC}/zfs ZFSSRC= ${SASRC}/zfs
LIBCSRC= ${SRCTOP}/lib/libc LIBCSRC= ${SRCTOP}/lib/libc
WORLDTMP= ${OBJTOP}/tmp
BOOTOBJ= ${OBJTOP}/stand BOOTOBJ= ${OBJTOP}/stand
# BINDIR is where we install # BINDIR is where we install
BINDIR?= /boot BINDIR?= /boot
# LUAPATH is where we search for and install lua scripts. # LUAPATH is where we search for and install lua scripts.
LUAPATH?= /boot/lua LUAPATH?= /boot/lua
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines
.endif .endif
# Machine specific flags for all builds here # Machine specific flags for all builds here
# All PowerPC builds are 32 bit. We have no 64-bit loaders on powerpc # All PowerPC builds are 32 bit. We have no 64-bit loaders on powerpc
# or powerpc64. # or powerpc64.
.if ${MACHINE_ARCH} == "powerpc64" .if ${MACHINE_ARCH} == "powerpc64"
CFLAGS+= -m32 -mcpu=powerpc CFLAGS+= -m32 -mcpu=powerpc
# Use same linker used to link 32 bit binaries
.if "${COMPILER_TYPE}" == "clang" && "${LINKER_TYPE}" == "lld"
bdragonUnsubmitted
Done
Inline Actions

I'm seeing build failures at cleandir stage. I guess LINKER_TYPE isn't being defined at that point. So ${LINKER_TYPE} would need to be wrapped in quotes, or for LINKER_TYPE to be defined at all stages.

bdragon: I'm seeing build failures at cleandir stage. I guess LINKER_TYPE isn't being defined at that…
CFLAGS+= -fuse-ld=${LIB32LD}
impUnsubmitted
Done
Inline Actions

how does LIB32LD get defined? That's 100% opaque to me

imp: how does LIB32LD get defined? That's 100% opaque to me
alfredoAuthorUnsubmitted
Done
Inline Actions

It's defined by in Makefile.libcompat.inc1 and passed to 'stand' by Makefile.inc1 through CROSSENV variable.

alfredo: It's defined by in Makefile.libcompat.inc1 and passed to 'stand' by Makefile.inc1 through…
impUnsubmitted
Not Done
Inline Actions

This will fail when compiled stand alone outside of buildworld.

imp: This will fail when compiled stand alone outside of buildworld.
luporlUnsubmitted
Done
Inline Actions

LIB32LD will be passed to stand either through buildworld or by first performing a 'make buildenv' before changing to stand and trying to build it.

But when trying to build stand without calling buildenv first, LIB32LD will be undefined.
In this case I chose to report an error, informing that LIB32LD must be set when stand is built with clang on powerpc64.

Is that ok? If not, what would be the alternative? Put the LIB32LD detection code in a separate make file that could be included by stand/defs.mk?

luporl: LIB32LD will be passed to stand either through buildworld or by first performing a 'make…
impUnsubmitted
Not Done
Inline Actions

No. I'd say that's not OK. It's almost never OK to have .error statements based on things not being defined. there's too many use cases to know you covered them all.

imp: No. I'd say that's not OK. It's almost never OK to have .error statements based on things not…
impUnsubmitted
Done
Inline Actions

LIB32LD still isn't defined in a file that gets included in the build.
Defining it all in bsd.compat.mk seems fundamentally wrong to me. that's not really what that's for.
If you want to use those things in the rest of the build, you'd need to define it bsd.cpu.mk (which bsd.compat.mk has access to).

imp: LIB32LD still isn't defined in a file that gets included in the build. Defining it all in bsd.
.endif
.endif .endif
# For amd64, there's a bit of mixed bag. Some of the tree (i386, lib*32) is # For amd64, there's a bit of mixed bag. Some of the tree (i386, lib*32) is
# build 32-bit and some 64-bit (lib*, efi). Centralize all the 32-bit magic here # build 32-bit and some 64-bit (lib*, efi). Centralize all the 32-bit magic here
# and activate it when DO32 is explicitly defined to be 1. # and activate it when DO32 is explicitly defined to be 1.
.if ${MACHINE_ARCH} == "amd64" && ${DO32:U0} == 1 .if ${MACHINE_ARCH} == "amd64" && ${DO32:U0} == 1
CFLAGS+= -m32 CFLAGS+= -m32
# LD_FLAGS is passed directly to ${LD}, not via ${CC}: # LD_FLAGS is passed directly to ${LD}, not via ${CC}:
▲ Show 20 Lines • Show All 128 Lines • Show Last 20 Lines
View Options

sys/kern/kern_sig.c

Show All 28 Lines
LDRSRC= ${BOOTSRC}/common LDRSRC= ${BOOTSRC}/common
LIBLUASRC= ${BOOTSRC}/liblua LIBLUASRC= ${BOOTSRC}/liblua
LUASRC= ${SRCTOP}/contrib/lua/src LUASRC= ${SRCTOP}/contrib/lua/src
SASRC= ${BOOTSRC}/libsa SASRC= ${BOOTSRC}/libsa
SYSDIR= ${SRCTOP}/sys SYSDIR= ${SRCTOP}/sys
UBOOTSRC= ${BOOTSRC}/uboot UBOOTSRC= ${BOOTSRC}/uboot
ZFSSRC= ${SASRC}/zfs ZFSSRC= ${SASRC}/zfs
LIBCSRC= ${SRCTOP}/lib/libc LIBCSRC= ${SRCTOP}/lib/libc
WORLDTMP= ${OBJTOP}/tmp
BOOTOBJ= ${OBJTOP}/stand BOOTOBJ= ${OBJTOP}/stand
# BINDIR is where we install # BINDIR is where we install
BINDIR?= /boot BINDIR?= /boot
# LUAPATH is where we search for and install lua scripts. # LUAPATH is where we search for and install lua scripts.
LUAPATH?= /boot/lua LUAPATH?= /boot/lua
▲ Show 20 Lines • Show All 48 Lines • ▼ Show 20 Lines
.endif .endif
# Machine specific flags for all builds here # Machine specific flags for all builds here
# All PowerPC builds are 32 bit. We have no 64-bit loaders on powerpc # All PowerPC builds are 32 bit. We have no 64-bit loaders on powerpc
# or powerpc64. # or powerpc64.
.if ${MACHINE_ARCH} == "powerpc64" .if ${MACHINE_ARCH} == "powerpc64"
CFLAGS+= -m32 -mcpu=powerpc CFLAGS+= -m32 -mcpu=powerpc
# Use same linker used to link 32 bit binaries
.if "${COMPILER_TYPE}" == "clang" && "${LINKER_TYPE}" == "lld"
bdragonUnsubmitted
Done
Inline Actions

I'm seeing build failures at cleandir stage. I guess LINKER_TYPE isn't being defined at that point. So ${LINKER_TYPE} would need to be wrapped in quotes, or for LINKER_TYPE to be defined at all stages.

bdragon: I'm seeing build failures at cleandir stage. I guess LINKER_TYPE isn't being defined at that…
CFLAGS+= -fuse-ld=${LIB32LD}
impUnsubmitted
Done
Inline Actions

how does LIB32LD get defined? That's 100% opaque to me

imp: how does LIB32LD get defined? That's 100% opaque to me
alfredoAuthorUnsubmitted
Done
Inline Actions

It's defined by in Makefile.libcompat.inc1 and passed to 'stand' by Makefile.inc1 through CROSSENV variable.

alfredo: It's defined by in Makefile.libcompat.inc1 and passed to 'stand' by Makefile.inc1 through…
impUnsubmitted
Not Done
Inline Actions

This will fail when compiled stand alone outside of buildworld.

imp: This will fail when compiled stand alone outside of buildworld.
luporlUnsubmitted
Done
Inline Actions

LIB32LD will be passed to stand either through buildworld or by first performing a 'make buildenv' before changing to stand and trying to build it.

But when trying to build stand without calling buildenv first, LIB32LD will be undefined.
In this case I chose to report an error, informing that LIB32LD must be set when stand is built with clang on powerpc64.

Is that ok? If not, what would be the alternative? Put the LIB32LD detection code in a separate make file that could be included by stand/defs.mk?

luporl: LIB32LD will be passed to stand either through buildworld or by first performing a 'make…
impUnsubmitted
Not Done
Inline Actions

No. I'd say that's not OK. It's almost never OK to have .error statements based on things not being defined. there's too many use cases to know you covered them all.

imp: No. I'd say that's not OK. It's almost never OK to have .error statements based on things not…
impUnsubmitted
Done
Inline Actions

LIB32LD still isn't defined in a file that gets included in the build.
Defining it all in bsd.compat.mk seems fundamentally wrong to me. that's not really what that's for.
If you want to use those things in the rest of the build, you'd need to define it bsd.cpu.mk (which bsd.compat.mk has access to).

imp: LIB32LD still isn't defined in a file that gets included in the build. Defining it all in bsd.
.endif
.endif .endif
# For amd64, there's a bit of mixed bag. Some of the tree (i386, lib*32) is # For amd64, there's a bit of mixed bag. Some of the tree (i386, lib*32) is
# build 32-bit and some 64-bit (lib*, efi). Centralize all the 32-bit magic here # build 32-bit and some 64-bit (lib*, efi). Centralize all the 32-bit magic here
# and activate it when DO32 is explicitly defined to be 1. # and activate it when DO32 is explicitly defined to be 1.
.if ${MACHINE_ARCH} == "amd64" && ${DO32:U0} == 1 .if ${MACHINE_ARCH} == "amd64" && ${DO32:U0} == 1
CFLAGS+= -m32 CFLAGS+= -m32
# LD_FLAGS is passed directly to ${LD}, not via ${CC}: # LD_FLAGS is passed directly to ${LD}, not via ${CC}:
▲ Show 20 Lines • Show All 128 Lines • Show Last 20 Lines