Page MenuHomeFreeBSD
Differential D33078

Basic signal tests: Can we deliver a signal?
ClosedPublic
Actions

Authored by imp on Nov 22 2021, 4:34 PM.
Tags
None
Referenced Files
F157552721: D33078.id98838.diff
Fri, May 22, 6:15 PM
F157552704: D33078.id98838.diff
Fri, May 22, 6:15 PM
Unknown Object (File)
Fri, May 22, 7:20 AM
Unknown Object (File)
Sun, May 17, 8:06 PM
Unknown Object (File)
Sun, May 17, 12:51 PM
Unknown Object (File)
Sun, May 17, 8:37 AM
Unknown Object (File)
Sun, May 17, 3:00 AM
Unknown Object (File)
Sun, May 17, 12:35 AM
View All Files
Subscribers
None

Details

Reviewers
cognet
kevans
Commits
rG359f5cf29ff1: Basic signal tests: Can we deliver a signal?
rGab44780f2c79: Basic signal tests: Can we deliver a signal?
rGafc5ab870d86: Basic signal tests: Can we deliver a signal?
Summary

Basic signal tests that tests can we deliver a signal via raise() and
can we deliver one via SIGALARM asynchronously.

In addition, tests whether or not on ARM T32 (Thumb) code can interrupt
A32 (normal) and vice versa.

While this test is aimed at ensuring basic qemu signals are working,
it's good to have in the base.

Sponsored by: Netflix

Test Plan

Tested on x86 and passes.
Haven't tested the armv7 stuff yet.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 42917
Build 39805: arc lint + arc unit

Event Timeline

imp requested review of this revision.Nov 22 2021, 4:34 PM
imp created this revision.
Harbormaster completed remote builds in B42917: Diff 98838.Nov 22 2021, 4:34 PM
imp edited the test plan for this revision. (Show Details)Nov 22 2021, 4:41 PM
imp added reviewers: cognet, kevans.Nov 22 2021, 4:57 PM
kevans added inline comments.Nov 22 2021, 5:23 PM
tests/sys/kern/basic_signal.c
4

Nice

13

+volatile here and on the ARM ones

78

I think this and the below will need alternative GCC versions; the GCC documentation seems to indicate that arch= is specifically for -march options, and the GCC equivalents to this specifically are just target("arm") and target("thumb").

I wonder if these are worth macros in cdefs.h, but I really can't come up with a good name for those... __thumb is maybe not terrible, but __arm probably is. __a32 and __t32 maybe, but that feels a bit obscure.

cognet added inline comments.Nov 22 2021, 5:45 PM
tests/sys/kern/basic_signal.c
78

I think clang now support the GNU syntax, so maybe using it would be enough ?

https://reviews.llvm.org/D33721

imp added inline comments.Nov 22 2021, 9:43 PM
tests/sys/kern/basic_signal.c
78

They build and appear to work on both. I'm not sure I want to invent something for FreeBSD for an architecture that may be the long glide path after peaking that currently only has use in a test.

imp updated this revision to Diff 98852.Nov 22 2021, 9:46 PM
imp edited the test plan for this revision. (Show Details)

+ volatile.

Harbormaster completed remote builds in B42921: Diff 98852.Nov 22 2021, 9:46 PM
imp added inline comments.Nov 22 2021, 10:56 PM
tests/sys/kern/basic_signal.c
78

Hmmm, there may be a clang issue... Rebuilding to make sure.

imp updated this revision to Diff 98858.Nov 22 2021, 11:28 PM

Update for clang

Harbormaster completed remote builds in B42926: Diff 98858.Nov 22 2021, 11:28 PM
imp marked 2 inline comments as done.Nov 22 2021, 11:31 PM

Updated with the 'marker' things. Didn't see that we had this in the rest of the tree, so I thought I'd limit them to this file...

tests/sys/kern/basic_signal.c
78

As you see, I needed to update since the above code review doesn't seem to be in the clang tree yet. Or at least the latest tree needs them...

imp added a comment.Nov 22 2021, 11:32 PM

... The only other place that we'd need them is when we do T32 / A32 thread tests...

cognet added inline comments.Nov 23 2021, 12:20 AM
tests/sys/kern/basic_signal.c
83

Shouldn't that be just target("arm") and target("thumb") for gcc ? :)

imp updated this revision to Diff 98861.Nov 23 2021, 3:45 AM

Turns out cognet was right. target("arm") not target("arch=arm") is the
form. And it seems to work for both clang and arm.

Harbormaster completed remote builds in B42929: Diff 98861.Nov 23 2021, 3:46 AM
imp updated this revision to Diff 98862.Nov 23 2021, 3:58 AM

better comments. remind the reader that thumb function addresses have the low
bit set.

Harbormaster completed remote builds in B42930: Diff 98862.Nov 23 2021, 3:58 AM
imp marked 2 inline comments as done.Nov 23 2021, 8:40 PM

I'm going to go ahead and commit this: it passes in my env for amd64, aarch64 and armv7 (running in a chroot on aarch64).

tests/sys/kern/basic_signal.c
78

My last comment is wrong: this code was wrong in arch=arm. target(arm) and target(thumb) just work for both gcc and clang.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 23 2021, 8:42 PM
Closed by commit rGafc5ab870d86: Basic signal tests: Can we deliver a signal? (authored by imp). · Explain Why
This revision was automatically updated to reflect the committed changes.
imp added a commit: rGafc5ab870d86: Basic signal tests: Can we deliver a signal?.
imp added a commit: rGab44780f2c79: Basic signal tests: Can we deliver a signal?.Dec 6 2021, 3:55 PM
imp added a commit: rG359f5cf29ff1: Basic signal tests: Can we deliver a signal?.

Revision Contents
Changeset List

PathSize
tests/
sys/
kern/
1 line
156 lines

Diff 98838

View Options

tests/sys/kern/Makefile

Loading...
View Options

tests/sys/kern/basic_signal.c

Loading...