Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace criterion with our test runner on x86 #448

Merged
merged 9 commits into from
Oct 22, 2024
Merged

Replace criterion with our test runner on x86 #448

merged 9 commits into from
Oct 22, 2024

Conversation

ayrtonm
Copy link
Contributor

@ayrtonm ayrtonm commented Oct 15, 2024 •
edited
Loading

This PR also moves the test_fault_handler.h functionality into the test runner. Closes #439

@ayrtonm ayrtonm changed the title Replace criterion with our test runner on x86 Replace criterion with our test runner on x86 (WIP) Oct 15, 2024
@ayrtonm
Copy link
Contributor Author

ayrtonm commented Oct 17, 2024

CI is failing because the (existing) setbuf(stdout, NULL) is forcing compartment pkey != 1 to flush stdout which is actually located in the main binary's .bss. Nothing seemed out of the ordinary in glibc's definition for the streams so I'm not sure which flags/what aspect of symbol versioning is causing it to be in .bss. I'll just get rid of the setbuf which removes the symbol though that's not solving the underlying problem.

@ayrtonm
Copy link
Contributor Author

ayrtonm commented Oct 17, 2024

We should probably also use sigaction rather than signal to install the handler.

@ayrtonm ayrtonm mentioned this pull request Oct 17, 2024
Open
@ayrtonm ayrtonm force-pushed the am/remove_criterion branch from 0a41a32 to 2904db0 Compare October 17, 2024 17:40
@ayrtonm ayrtonm changed the title Replace criterion with our test runner on x86 (WIP) Replace criterion with our test runner on x86 Oct 17, 2024
@ayrtonm ayrtonm force-pushed the am/remove_criterion branch from 2904db0 to 83a08cc Compare October 17, 2024 17:41
@ayrtonm ayrtonm requested a review from rinon October 17, 2024 17:55
rinon
rinon reviewed Oct 17, 2024
View reviewed changes
misc/test_runner/include/ia2_test_runner.h Outdated Show resolved Hide resolved
tools/rewriter/SourceRewriter.cpp Outdated Show resolved Hide resolved
@ayrtonm ayrtonm force-pushed the am/remove_criterion branch from 017fe8e to 0e52190 Compare October 22, 2024 15:58
@ayrtonm
Remove external/b63
820aef1
@ayrtonm
Replace criterion with test runner on x86
2b6cfb5
@ayrtonm
Move test_fault_handler.h into test_runner.c
7079d5b
@ayrtonm
Remove needless macro defines and add missing headers
fe54d5f
@ayrtonm
Conditionally define Test macro to avoid spurious rewriter warnings
68c2cdd 
The spurious warnings are caused because we don't (and should not) rewrite the
test runner source file. See issue #439 for context.
@ayrtonm
Disable stdio buffering in test runner and replace signal(2) with sig…
0d81a89 
…action

First part is a workaround for issue #450
@ayrtonm
Update comments and remove constructor for installing signal handler
ce1d84e 
Since the test runner is its own .c we can just call sigaction from main instead
of from a global constructor to avoid needless complexity.
@ayrtonm
Remove fn decl with naked attribute on ARM in test_runner.c
565626e 
__attribute__((naked)) is not supported on ARM and triggers a compiler error so
this commit moves around functions in test_runner.c to avoid declaring
handle_segfault with it.
@ayrtonm
Update new test to use ia2_test_runner.h
01beb78
@ayrtonm ayrtonm force-pushed the am/remove_criterion branch from 43d0867 to 01beb78 Compare October 22, 2024 16:10
@ayrtonm ayrtonm merged commit 94f890b into main Oct 22, 2024
34 checks passed
@ayrtonm ayrtonm deleted the am/remove_criterion branch October 22, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rinon rinon rinon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace use of prebuilt criterion with our test runner
2 participants
@ayrtonm @rinon