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.

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

Replace criterion with our test runner on x86 #448

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ayrtonm
Copy link
Contributor

@ayrtonm ayrtonm commented Oct 15, 2024

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 changed the title Replace criterion with our test runner on x86 (WIP) Replace criterion with our test runner on x86 Oct 17, 2024
Comment on lines +46 to +51
* signal handlers. This file must be included exactly once from a source file
* in the main binary with IA2_DEFINE_TEST_HANDLER defined by the preprocessor.
* This will define the functions and variables used by the test handler, ensure
* it is initialized before main and provide access to the LOG and
* CHECK_VIOLATION macros. Other files which need CHECK_VIOLATION or LOG may
* include the header without defining IA2_DEFINE_TEST_HANDLER. Using
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still true? Do we need to define IA2_DEFINE_TEST_HANDLER?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right, I forgot to update the comments

Comment on lines +617 to +620
auto annotation = fn_ptr_expr->getDecl()->getAttr<clang::AnnotateAttr>();
if (annotation && annotation->getAnnotation() == SKIP_WRAP_ATTR) {
return;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for this change? Just wanted to make sure it was related and not accidental.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, but it's not obvious why so I should've update the documentation in ia2.h. The attribute is added to the function declaration in the Test macro and this branch makes it so that expressions that refer to that dec (e.g. the RHS in .test = fake_criterion_##suite##_##name,) are not rewritten. In the case of Test it wasn't being rewritten since it's expanded from a macro, but the rewriter was printing out warnings about needing to manually rewrite that spot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would've like to have IA2_{BEGIN,END}_NO_WRAP apply to only things between those macros but I don't think we can annotate expressions so I don't see an obvious way to do this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace use of prebuilt criterion with our test runner
2 participants