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

Detect and fix invalid LLVM IR generated by bpftrace #2297

Closed
lenticularis39 opened this issue Jul 12, 2022 · 2 comments · Fixed by #2316
Closed

Detect and fix invalid LLVM IR generated by bpftrace #2297

lenticularis39 opened this issue Jul 12, 2022 · 2 comments · Fixed by #2316
Labels
enhancement New feature or request, changes on existing features

Comments

@lenticularis39
Copy link
Contributor

When running bpftrace on LLVM 14, an issue related to bpftrace generating invalid LLVM IR were encountered (see #2228, #2222 and #2296). Furthermore, other failing tests in LLVM 14 also generate invalid LLVM IR, implying this could be a more widespread issue in the codegen.

As suggested by @fbs in #2222, a verifier pass detecting LLVM IR in bpftrace output could be added to all tests to ensure these issues are identified and fixed both in the present code and in the future.

@lenticularis39 lenticularis39 added the enhancement New feature or request, changes on existing features label Jul 12, 2022
@lenticularis39 lenticularis39 changed the title Add LLVM module verifier to tests Detect and fix invalid LLVM IR generated by bpftrace Jul 15, 2022
@BurntBrunch
Copy link

could be added to all tests

Locally, I found myself using this snippet to ensure all debug builds, tests or not, run through the verifier first. This has so far saved me hours as I work on codegen for new features.

diff --git a/src/ast/passes/codegen_llvm.cpp b/src/ast/passes/codegen_llvm.cpp
index e16c965f..a576e419 100644
--- a/src/ast/passes/codegen_llvm.cpp
+++ b/src/ast/passes/codegen_llvm.cpp
@@ -12,6 +12,7 @@
 #include <llvm/IR/LLVMContext.h>
 #include <llvm/IR/LegacyPassManager.h>
 #include <llvm/IR/Metadata.h>
+#include <llvm/IR/Verifier.h>
 #if LLVM_VERSION_MAJOR >= 14
 #include <llvm/Passes/PassBuilder.h>
 #endif
@@ -3154,6 +3155,14 @@ void CodegenLLVM::optimize()
 {
   assert(state_ == State::IR);
 
+#ifndef NDEBUG
+  if (verifyModule(*module_, &errs()))
+  {
+    DumpIR();
+    return;
+  }
+#endif
+
 #if LLVM_VERSION_MAJOR >= 14
   PipelineTuningOptions pto;
   pto.LoopUnrolling = false;
@@ -3185,6 +3194,7 @@ void CodegenLLVM::optimize()
   PassManagerBuilder PMB;
   PMB.OptLevel = 3;
   legacy::PassManager PM;
+  PM.add(createVerifierPass());
   PM.add(createFunctionInliningPass());
   /*
    * llvm < 4.0 needs

@lenticularis39
Copy link
Contributor Author

Thank you for the suggestion @BurntBrunch, I implemented a solution in the linked PR by calling VerifierPass directly and doing output redirection, using verifyModule is a simpler solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request, changes on existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants