Skip to content

Commit b5d34a1

Browse files
committed
Add a subtype to PassManager to prevent it from being run on the
incorrect type which would have led to a memory error
1 parent b95c72d commit b5d34a1

File tree

5 files changed

+135
-48
lines changed

5 files changed

+135
-48
lines changed

examples/kaleidoscope/main.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ impl<'a> Parser<'a> {
840840
pub struct Compiler<'a> {
841841
pub context: &'a Context,
842842
pub builder: &'a Builder,
843-
pub fpm: &'a PassManager,
843+
pub fpm: &'a PassManager<FunctionValue>,
844844
pub module: &'a Module,
845845
pub function: &'a Function,
846846

@@ -1146,7 +1146,7 @@ impl<'a> Compiler<'a> {
11461146

11471147
// return the whole thing after verification and optimization
11481148
if function.verify(true) {
1149-
self.fpm.run_on_function(&function);
1149+
self.fpm.run_on(&function);
11501150

11511151
Ok(function)
11521152
} else {
@@ -1159,7 +1159,7 @@ impl<'a> Compiler<'a> {
11591159
}
11601160

11611161
/// Compiles the specified `Function` in the given `Context` and using the specified `Builder`, `PassManager`, and `Module`.
1162-
pub fn compile(context: &'a Context, builder: &'a Builder, pass_manager: &'a PassManager, module: &'a Module, function: &Function) -> Result<FunctionValue, &'static str> {
1162+
pub fn compile(context: &'a Context, builder: &'a Builder, pass_manager: &'a PassManager<FunctionValue>, module: &'a Module, function: &Function) -> Result<FunctionValue, &'static str> {
11631163
let mut compiler = Compiler {
11641164
context: context,
11651165
builder: builder,
@@ -1228,7 +1228,7 @@ pub fn main() {
12281228
let builder = context.create_builder();
12291229

12301230
// Create FPM
1231-
let fpm = PassManager::create_for_function(&module);
1231+
let fpm = PassManager::create(&module);
12321232

12331233
fpm.add_instruction_combining_pass();
12341234
fpm.add_reassociate_pass();

src/builder.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
//! A `Builder` enables you to build instructions.
2+
13
use either::{Either, Left, Right};
24
use llvm_sys::core::{LLVMBuildAdd, LLVMBuildAlloca, LLVMBuildAnd, LLVMBuildArrayAlloca, LLVMBuildArrayMalloc, LLVMBuildBr, LLVMBuildCall, LLVMBuildCast, LLVMBuildCondBr, LLVMBuildExtractValue, LLVMBuildFAdd, LLVMBuildFCmp, LLVMBuildFDiv, LLVMBuildFence, LLVMBuildFMul, LLVMBuildFNeg, LLVMBuildFree, LLVMBuildFSub, LLVMBuildGEP, LLVMBuildICmp, LLVMBuildInsertValue, LLVMBuildIsNotNull, LLVMBuildIsNull, LLVMBuildLoad, LLVMBuildMalloc, LLVMBuildMul, LLVMBuildNeg, LLVMBuildNot, LLVMBuildOr, LLVMBuildPhi, LLVMBuildPointerCast, LLVMBuildRet, LLVMBuildRetVoid, LLVMBuildStore, LLVMBuildSub, LLVMBuildUDiv, LLVMBuildUnreachable, LLVMBuildXor, LLVMDisposeBuilder, LLVMGetElementType, LLVMGetInsertBlock, LLVMGetReturnType, LLVMGetTypeKind, LLVMInsertIntoBuilder, LLVMPositionBuilderAtEnd, LLVMTypeOf, LLVMBuildExtractElement, LLVMBuildInsertElement, LLVMBuildIntToPtr, LLVMBuildPtrToInt, LLVMInsertIntoBuilderWithName, LLVMClearInsertionPosition, LLVMCreateBuilder, LLVMPositionBuilder, LLVMPositionBuilderBefore, LLVMBuildAggregateRet, LLVMBuildStructGEP, LLVMBuildInBoundsGEP, LLVMBuildPtrDiff, LLVMBuildNSWAdd, LLVMBuildNUWAdd, LLVMBuildNSWSub, LLVMBuildNUWSub, LLVMBuildNSWMul, LLVMBuildNUWMul, LLVMBuildSDiv, LLVMBuildSRem, LLVMBuildURem, LLVMBuildFRem, LLVMBuildNSWNeg, LLVMBuildNUWNeg, LLVMBuildFPToUI, LLVMBuildFPToSI, LLVMBuildSIToFP, LLVMBuildUIToFP, LLVMBuildFPTrunc, LLVMBuildFPExt, LLVMBuildIntCast, LLVMBuildFPCast, LLVMBuildSExtOrBitCast, LLVMBuildZExtOrBitCast, LLVMBuildTruncOrBitCast, LLVMBuildSwitch, LLVMAddCase, LLVMBuildShl, LLVMBuildAShr, LLVMBuildLShr, LLVMBuildGlobalString, LLVMBuildGlobalStringPtr, LLVMBuildExactSDiv, LLVMBuildTrunc, LLVMBuildSExt, LLVMBuildZExt, LLVMBuildSelect, LLVMBuildAddrSpaceCast, LLVMBuildBitCast, LLVMBuildShuffleVector, LLVMBuildVAArg, LLVMBuildIndirectBr, LLVMAddDestination};
35
use llvm_sys::prelude::{LLVMBuilderRef, LLVMValueRef};
@@ -17,7 +19,7 @@ pub struct Builder {
1719

1820
impl Builder {
1921
pub(crate) fn new(builder: LLVMBuilderRef) -> Self {
20-
assert!(!builder.is_null());
22+
debug_assert!(!builder.is_null());
2123

2224
Builder {
2325
builder: builder
@@ -397,7 +399,7 @@ impl Builder {
397399
},
398400
None => unsafe {
399401
LLVMInsertIntoBuilder(self.builder, instruction.as_value_ref());
400-
}
402+
},
401403
}
402404
}
403405

src/passes.rs

Lines changed: 107 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ use crate::module::Module;
1414
use crate::targets::TargetData;
1515
use crate::values::{AsValueRef, FunctionValue};
1616

17+
use std::borrow::Borrow;
18+
use std::marker::PhantomData;
19+
1720
// REVIEW: Opt Level might be identical to targets::Option<CodeGenOptLevel>
1821
#[derive(Debug)]
1922
pub struct PassManagerBuilder {
@@ -74,22 +77,72 @@ impl PassManagerBuilder {
7477
}
7578
}
7679

77-
// SubType: pass_manager: &PassManager<FunctionValue>
78-
pub fn populate_function_pass_manager(&self, pass_manager: &PassManager) {
80+
/// Populates a PassManager<FunctionValue> with the expectation of function
81+
/// transformations.
82+
///
83+
/// # Example
84+
///
85+
/// ```
86+
/// use inkwell::OptimizationLevel::Aggressive;
87+
/// use inkwell::module::Module;
88+
/// use inkwell::passes::{PassManager, PassManagerBuilder};
89+
///
90+
/// let module = Module::create("mod");
91+
/// let pass_manager_builder = PassManagerBuilder::create();
92+
///
93+
/// pass_manager_builder.set_optimization_level(Aggressive);
94+
///
95+
/// let fpm = PassManager::create(&module);
96+
///
97+
/// pass_manager_builder.populate_function_pass_manager(&fpm);
98+
/// ```
99+
pub fn populate_function_pass_manager(&self, pass_manager: &PassManager<FunctionValue>) {
79100
unsafe {
80101
LLVMPassManagerBuilderPopulateFunctionPassManager(self.pass_manager_builder, pass_manager.pass_manager)
81102
}
82103
}
83104

84-
// SubType: pass_manager: &PassManager<Module>
85-
pub fn populate_module_pass_manager(&self, pass_manager: &PassManager) {
105+
/// Populates a PassManager<Module> with the expectation of whole module
106+
/// transformations.
107+
///
108+
/// # Example
109+
///
110+
/// ```
111+
/// use inkwell::OptimizationLevel::Aggressive;
112+
/// use inkwell::passes::{PassManager, PassManagerBuilder};
113+
///
114+
/// let pass_manager_builder = PassManagerBuilder::create();
115+
///
116+
/// pass_manager_builder.set_optimization_level(Aggressive);
117+
///
118+
/// let fpm = PassManager::create(());
119+
///
120+
/// pass_manager_builder.populate_module_pass_manager(&fpm);
121+
/// ```
122+
pub fn populate_module_pass_manager(&self, pass_manager: &PassManager<Module>) {
86123
unsafe {
87124
LLVMPassManagerBuilderPopulateModulePassManager(self.pass_manager_builder, pass_manager.pass_manager)
88125
}
89126
}
90127

91-
// SubType: Need LTO subtype?
92-
pub fn populate_lto_pass_manager(&self, pass_manager: &PassManager, internalize: bool, run_inliner: bool) {
128+
/// Populates a PassManager<Module> with the expectation of link time
129+
/// optimization transformations.
130+
///
131+
/// # Example
132+
///
133+
/// ```
134+
/// use inkwell::OptimizationLevel::Aggressive;
135+
/// use inkwell::passes::{PassManager, PassManagerBuilder};
136+
///
137+
/// let pass_manager_builder = PassManagerBuilder::create();
138+
///
139+
/// pass_manager_builder.set_optimization_level(Aggressive);
140+
///
141+
/// let lpm = PassManager::create(());
142+
///
143+
/// pass_manager_builder.populate_lto_pass_manager(&lpm, false, false);
144+
/// ```
145+
pub fn populate_lto_pass_manager(&self, pass_manager: &PassManager<Module>, internalize: bool, run_inliner: bool) {
93146
unsafe {
94147
LLVMPassManagerBuilderPopulateLTOPassManager(self.pass_manager_builder, pass_manager.pass_manager, internalize as i32, run_inliner as i32)
95148
}
@@ -104,37 +157,65 @@ impl Drop for PassManagerBuilder {
104157
}
105158
}
106159

160+
// This is an ugly privacy hack so that PassManagerSubType can stay private
161+
// to this module and so that super traits using this trait will be not be
162+
// implementable outside this library
163+
pub trait PassManagerSubType {
164+
type Input;
165+
166+
unsafe fn create<I: Borrow<Self::Input>>(input: I) -> LLVMPassManagerRef;
167+
unsafe fn run_in_pass_manager(&self, pass_manager: &PassManager<Self>) -> bool where Self: Sized;
168+
}
169+
170+
impl PassManagerSubType for Module {
171+
type Input = ();
172+
173+
unsafe fn create<I: Borrow<Self::Input>>(_: I) -> LLVMPassManagerRef {
174+
LLVMCreatePassManager()
175+
}
176+
177+
unsafe fn run_in_pass_manager(&self, pass_manager: &PassManager<Self>) -> bool {
178+
LLVMRunPassManager(pass_manager.pass_manager, self.module.get()) == 1
179+
}
180+
}
181+
182+
// With GATs https://github.com/rust-lang/rust/issues/44265 this could be
183+
// type Input<'a> = &'a Module;
184+
impl PassManagerSubType for FunctionValue {
185+
type Input = Module;
186+
187+
unsafe fn create<I: Borrow<Self::Input>>(input: I) -> LLVMPassManagerRef {
188+
LLVMCreateFunctionPassManagerForModule(input.borrow().module.get())
189+
}
190+
191+
unsafe fn run_in_pass_manager(&self, pass_manager: &PassManager<Self>) -> bool {
192+
LLVMRunFunctionPassManager(pass_manager.pass_manager, self.as_value_ref()) == 1
193+
}
194+
}
195+
107196
// SubTypes: PassManager<Module>, PassManager<FunctionValue>
108197
/// A manager for running optimization and simplification passes. Much of the
109198
/// documenation for specific passes is directly from the [LLVM
110199
/// documentation](https://llvm.org/docs/Passes.html).
111200
#[derive(Debug)]
112-
pub struct PassManager {
201+
pub struct PassManager<T> {
113202
pub(crate) pass_manager: LLVMPassManagerRef,
203+
sub_type: PhantomData<T>,
114204
}
115205

116-
impl PassManager {
117-
pub(crate) fn new(pass_manager: LLVMPassManagerRef) -> PassManager {
206+
impl<T: PassManagerSubType> PassManager<T> {
207+
pub(crate) fn new(pass_manager: LLVMPassManagerRef) -> Self {
118208
assert!(!pass_manager.is_null());
119209

120210
PassManager {
121211
pass_manager,
212+
sub_type: PhantomData,
122213
}
123214
}
124215

125-
// SubTypes: PassManager<Module>::create()
126-
pub fn create_for_module() -> Self {
127-
let pass_manager = unsafe {
128-
LLVMCreatePassManager()
129-
};
130-
131-
PassManager::new(pass_manager)
132-
}
133-
134-
// SubTypes: PassManager<FunctionValue>::create()
135-
pub fn create_for_function(module: &Module) -> Self {
216+
pub fn create<I: Borrow<T::Input>>(input: I) -> PassManager<T> {
136217
let pass_manager = unsafe {
137-
LLVMCreateFunctionPassManagerForModule(module.module.get())
218+
T::create(input)
138219
};
139220

140221
PassManager::new(pass_manager)
@@ -153,17 +234,11 @@ impl PassManager {
153234
}
154235
}
155236

156-
// SubTypes: For PassManager<FunctionValue> only, rename run_on
157-
pub fn run_on_function(&self, fn_value: &FunctionValue) -> bool {
158-
unsafe {
159-
LLVMRunFunctionPassManager(self.pass_manager, fn_value.as_value_ref()) == 1
160-
}
161-
}
162-
163-
// SubTypes: For PassManager<Module> only, rename run_on
164-
pub fn run_on_module(&self, module: &Module) -> bool {
237+
/// This method returns true if any of the passes modified the module and
238+
/// false otherwise.
239+
pub fn run_on(&self, input: &T) -> bool {
165240
unsafe {
166-
LLVMRunPassManager(self.pass_manager, module.module.get()) == 1
241+
input.run_in_pass_manager(self)
167242
}
168243
}
169244

@@ -984,7 +1059,7 @@ impl PassManager {
9841059
}
9851060
}
9861061

987-
impl Drop for PassManager {
1062+
impl<T> Drop for PassManager<T> {
9881063
fn drop(&mut self) {
9891064
unsafe {
9901065
LLVMDisposePassManager(self.pass_manager)

src/targets.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ impl TargetMachine {
914914
}
915915

916916
// TODO: Move to PassManager?
917-
pub fn add_analysis_passes(&self, pass_manager: &PassManager) {
917+
pub fn add_analysis_passes<T>(&self, pass_manager: &PassManager<T>) {
918918
unsafe {
919919
LLVMAddAnalysisPasses(self.target_machine, pass_manager.pass_manager)
920920
}

tests/all/test_passes.rs

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
extern crate inkwell;
22

3+
use self::inkwell::OptimizationLevel::Aggressive;
34
use self::inkwell::context::Context;
45
use self::inkwell::passes::{PassManagerBuilder, PassManager, PassRegistry};
5-
use self::inkwell::OptimizationLevel::Aggressive;
66

77
#[test]
88
fn test_init_all_passes_for_module() {
99
let context = Context::create();
1010
let module = context.create_module("my_module");
11-
let pass_manager = PassManager::create_for_module();
11+
let pass_manager = PassManager::create(());
1212

1313
pass_manager.add_argument_promotion_pass();
1414
pass_manager.add_constant_merge_pass();
@@ -85,7 +85,7 @@ fn test_init_all_passes_for_module() {
8585
assert!(!pass_manager.initialize());
8686
assert!(!pass_manager.finalize());
8787

88-
pass_manager.run_on_module(&module);
88+
pass_manager.run_on(&module);
8989

9090
assert!(!pass_manager.initialize());
9191
assert!(!pass_manager.finalize());
@@ -107,7 +107,7 @@ fn test_pass_manager_builder() {
107107
let context = Context::create();
108108
let module = context.create_module("my_module");
109109

110-
let fn_pass_manager = PassManager::create_for_function(&module);
110+
let fn_pass_manager = PassManager::create(&module);
111111

112112
pass_manager_builder.populate_function_pass_manager(&fn_pass_manager);
113113

@@ -123,20 +123,30 @@ fn test_pass_manager_builder() {
123123
// TODO: Test with actual changes? Would be true in that case
124124
// REVIEW: Segfaults in 4.0
125125
#[cfg(not(feature = "llvm4-0"))]
126-
assert!(!fn_pass_manager.run_on_function(&fn_value));
126+
assert!(!fn_pass_manager.run_on(&fn_value));
127127

128-
let module_pass_manager = PassManager::create_for_module();
128+
let module_pass_manager = PassManager::create(());
129129

130130
pass_manager_builder.populate_module_pass_manager(&module_pass_manager);
131131

132+
let module2 = module.clone();
133+
132134
// TODOC: Seems to return true in 3.7, 6.0, & 7.0 even though no changes were made.
133135
// In 3.6, 3.8, & 3.9 it returns false. Seems like a LLVM bug?
134136
#[cfg(not(any(feature = "llvm3-7", feature = "llvm6-0", feature = "llvm7-0")))]
135-
assert!(!module_pass_manager.run_on_module(&module));
137+
assert!(!module_pass_manager.run_on(&module));
136138
#[cfg(any(feature = "llvm3-7", feature = "llvm6-0", feature = "llvm7-0"))]
137-
assert!(module_pass_manager.run_on_module(&module));
139+
assert!(module_pass_manager.run_on(&module));
140+
141+
let lto_pass_manager = PassManager::create(());
138142

139-
// TODO: Populate LTO pass manager?
143+
pass_manager_builder.populate_lto_pass_manager(&lto_pass_manager, false, false);
144+
145+
// See above note on version differences
146+
#[cfg(not(any(feature = "llvm3-7", feature = "llvm6-0", feature = "llvm7-0")))]
147+
assert!(!lto_pass_manager.run_on(&module2));
148+
#[cfg(any(feature = "llvm3-7", feature = "llvm6-0", feature = "llvm7-0"))]
149+
assert!(lto_pass_manager.run_on(&module2));
140150
}
141151

142152
#[test]

0 commit comments

Comments
 (0)