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

Add option to save all traces #49

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

marnovandermaas
Copy link
Member

This adds an option to QCVEngine to save all traces instead of just failures. I'm happy to implement any suggestions on how to do this in a cleaner way!

@PeterRugg
Copy link
Collaborator

Interesting. Probably a good idea (presumably for something like checking coverage?) I think this is probably the right way to do it. I'll do detailed review on Monday, but one thought: is there any reason you save out the unannotated test rather than m_traceAB, which should have comments including what the implementations said? I'm a bit fuzzy on all the types involved though, because I haven't touched this for quite a while.

@marnovandermaas
Copy link
Member Author

marnovandermaas commented Jun 7, 2024

Didn't think of trying m_traceAB. The show test looks like:

# Automatically generated test case
#>QCVENGINE_TEST_V2.0
.4byte 0x00000017 # auipc x0, 0
...
# Test end

While showing the m_traceAB looks like:

# Automatically generated test case
Just #>QCVENGINE_TEST_V2.0
((.4byte 0x99c00793 # addi x15, x0, -1636,Just Trap: False, PCWD: 0x0000000080000084, RD: 15, RWD: 0x00000000fffff99c, MA: 0x0000000000000000, MWD: 0x0000000000000000, MWM: 0b00000000, MRD: 0x0000000000000000, MRM: 0b00000000 I: 0x0000000099c00793 PRV_? XL:? (addi x15, x0, -1636)),Just Trap: False, PCWD: 0x0000000080000084, RD: 15, RWD: 0x00000000fffff99c, MA: 0x00000000fffff99c, MWD: 0x0000000000000000, MWM: 0b00000000, MRD: 0x0000000000000000, MRM: 0b00000000 I: 0x0000000099c00793 PRV_? XL:? (addi x15, x0, -1636))
...
((# Test end,Just halt token),Just halt token)

I think I prefer showing just the test because of readability and being able to replay the test back through TestRIG.

@PeterRugg
Copy link
Collaborator

Ah, yes, there might be a way to smooth out the types to get rid of the maybes and make the print nicer, but for sure it will be verbose!

Copy link
Collaborator

@PeterRugg PeterRugg left a comment

Choose a reason for hiding this comment

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

Happy, except for potential usability improvement if it's convenient.

where onDeath test = do putStrLn "Failure rerunning test"
askAndSave sourceFile (show test) Nothing testTrans
onTrace trace = askAndSave sourceFile (showAnnotatedTrace (isNothing m_implB) archDesc trace) (Just trace) testTrans
let checkTrapAndSave sourceFile test = saveOnFail sourceFile test (check_mcause_on_trap :: Test TestResult -> Test TestResult)
let checkResult = if optVerbosity flags > 1 then verboseCheckWithResult else quickCheckWithResult
let checkGen gen remainingTests =
checkResult (Args Nothing remainingTests 1 (testLen flags) (optVerbosity flags > 0) (if optShrink flags then 1000 else 0))
(prop implA m_implB alive (checkTrapAndSave Nothing) archDesc (timeoutDelay flags) (optVerbosity flags) (optIgnoreAsserts flags) (optStrict flags) gen)
(prop implA m_implB alive (checkTrapAndSave Nothing) archDesc (timeoutDelay flags) (optVerbosity flags) (if (optSaveAll flags) then (saveDir flags) else Nothing) (optIgnoreAsserts flags) (optStrict flags) gen)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Slight issue: users might be surprised if they passed "save-all", forgot to pass a directory, and lose all the tests. I wonder if it's convenient to give an error early on in that case? We could also have separate options "failures-dir" and "tests-dir" that the user can set separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, locally I had put that check into TestRIG run script. I might merge this PR and then see if I can get the failure and test directories separated.

@marnovandermaas marnovandermaas merged commit da85ac6 into CTSRD-CHERI:master Jun 14, 2024
2 checks passed
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.

2 participants