Skip to content

Conversation

@cjappl
Copy link
Collaborator

@cjappl cjappl commented Jul 24, 2024

Highly recommend reviewing this commit by commit, introduces each change in each top level directory (llvm, then clang, then compiler-rt), and each commit is dependent on the previous commit.

This seems to be equivalently functional to our existing approach. I have compiled test programs and it appears as if nothing has changed. I haven't seen any situations where these calls are optimized out (and I've added unit tests to confirm that they do not, on all optimization levels)

Again highly recommend reviewing commit by commit (Files changed->Select "Changes from all commits" and work in the order of top to bottom, LLVM->clang->compiler-rt). This is likely how we would submit them in review order.

1. [LLVM] Introduce LLVM Pass Manager pass for RealtimeSanitizer and new llvm::nonblocking attribute

The most substantial change to our existing code.

A PassManager Pass is discussed more here: https://llvm.org/docs/WritingAnLLVMNewPMPass.html which breaks it down very simply.

The result of this is a unit test in check-llvm that ensures that when our pass runs, the __rtsan_realtime_e[nter/xit] calls are inserted (llvm/test/Instrumentation/RealtimeSanitizer/rtsan.ll)

There is reasonable precedent for this, it seems like many of the sanitizers do some work in the pass stage:

> fd Sanitizer.cpp
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
llvm/lib/Transforms/Instrumentation/NumericalStabilitySanitizer.cpp
llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
llvm/lib/Transforms/Instrumentation/RealtimeSanitizer.cpp

2. [clang] Plug in PassManager pass to pipeline and introduce codegen

Now that we have a pass manager pass, we should plug it in to the pipeline, done here in BackendUtil.cpp

The rest of these files should look EXTREMELY similar to the existing v0 goal version, just insert the new LLVM attribute if it has the function effect we require (CGCall.cpp).

And introduce the driver, which exposes the -fsanitize=realtime flag.

It may be good to look at the tests, which again show that we are inserting what we want to when we compile.

3. [compiler-rt] Enable our tests for compiler-rt, proving that end-to-end works

This code is exactly identical to code in our upstream branch, nothing really to see here. The lit tests confirm that everything is happy all the way from start to finish.

@cjappl cjappl requested a review from davidtrevelyan July 24, 2024 16:04
@cjappl cjappl force-pushed the FULL-move-to-llvm branch 2 times, most recently from 2bac01d to c49ef92 Compare July 24, 2024 21:35
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Kept as a placeholder, as we need to add Options very soon to have different exit modes etc. I can delete this if it looks too wonky

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, old school c-style include guards seems to be the move around here!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not really know or understand what a preserved analysis is, but I pattern matched on this. It seems to indicate that "this pass did a thing"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now that we have the llvm work done (in the earlier commit), we now check if the sanitizer is enabled, and insert this "Pass" that we defined in LLVM.

As for where it should live registerScalarOptimizerLateEPCallback was my guess, as it says it is "very late in the pipeline" but we likely have to ask reviewers what is appropriate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was not able to figure out how to induce a multiple return statement in clang-IR. I may have to try to run a much more complex file through it.

That would be an additional test, but this one is good to start us out.

@cjappl cjappl force-pushed the FULL-move-to-llvm branch from c49ef92 to e805be2 Compare July 25, 2024 00:02
@cjappl cjappl closed this Jul 25, 2024
@cjappl cjappl deleted the FULL-move-to-llvm branch August 27, 2024 00:18
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.

1 participant