Skip to content

Commit 9a49d6a

Browse files
k-sareenqinsoon
k-sareen
and
qinsoon
authored
Only map with executable permissions when using code space (#1176)
This PR closes #7. This PR * refactors `MmapStrategy` to include protection flags, and allows each space to pass `Mmapstrategy` to the mmapper. * removes exec permission from most spaces. Only code spaces will have exec permission. * adds a feature `exec_permission_on_all_spaces` for bindings that may allocate code into normal spaces. --------- Co-authored-by: Yi Lin <[email protected]>
1 parent 0c1c083 commit 9a49d6a

29 files changed

+280
-158
lines changed

‎Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,12 @@ set_unlog_bits_vm_space = []
9898
# TODO: This is not properly implemented yet. We currently use an immortal space instead, and do not guarantee read-only semantics.
9999
ro_space = []
100100
# A code space with execution permission.
101-
# TODO: This is not properly implemented yet. We currently use an immortal space instead, and all our spaces have execution permission at the moment.
102101
code_space = []
103102

103+
# By default, we only allow execution permission for code spaces. With this feature, all the spaces have execution permission.
104+
# Use with care.
105+
exec_permission_on_all_spaces = []
106+
104107
# Global valid object (VO) bit metadata.
105108
# The VO bit is set when an object is allocated, and cleared when the GC determines it is dead.
106109
# See `src/util/metadata/vo_bit/mod.rs`

‎docs/userguide/src/tutorial/code/mygc_semispace/global.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,9 @@ impl<VM: VMBinding> MyGC<VM> {
175175
let res = MyGC {
176176
hi: AtomicBool::new(false),
177177
// ANCHOR: copyspace_new
178-
copyspace0: CopySpace::new(plan_args.get_space_args("copyspace0", true, VMRequest::discontiguous()), false),
178+
copyspace0: CopySpace::new(plan_args.get_space_args("copyspace0", true, false, VMRequest::discontiguous()), false),
179179
// ANCHOR_END: copyspace_new
180-
copyspace1: CopySpace::new(plan_args.get_space_args("copyspace1", true, VMRequest::discontiguous()), true),
180+
copyspace1: CopySpace::new(plan_args.get_space_args("copyspace1", true, false, VMRequest::discontiguous()), true),
181181
common: CommonPlan::new(plan_args),
182182
};
183183

‎src/mmtk.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,6 @@ impl<VM: VMBinding> MMTK<VM> {
203203
},
204204
);
205205

206-
if *options.transparent_hugepages {
207-
MMAPPER.set_mmap_strategy(crate::util::memory::MmapStrategy::TransparentHugePages);
208-
}
209-
210206
MMTK {
211207
options,
212208
state,

‎src/plan/generational/copying/global.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,11 @@ impl<VM: VMBinding> GenCopy<VM> {
209209
};
210210

211211
let copyspace0 = CopySpace::new(
212-
plan_args.get_space_args("copyspace0", true, VMRequest::discontiguous()),
212+
plan_args.get_space_args("copyspace0", true, false, VMRequest::discontiguous()),
213213
false,
214214
);
215215
let copyspace1 = CopySpace::new(
216-
plan_args.get_space_args("copyspace1", true, VMRequest::discontiguous()),
216+
plan_args.get_space_args("copyspace1", true, false, VMRequest::discontiguous()),
217217
true,
218218
);
219219

‎src/plan/generational/global.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct CommonGenPlan<VM: VMBinding> {
4141
impl<VM: VMBinding> CommonGenPlan<VM> {
4242
pub fn new(mut args: CreateSpecificPlanArgs<VM>) -> Self {
4343
let nursery = CopySpace::new(
44-
args.get_space_args("nursery", true, VMRequest::discontiguous()),
44+
args.get_space_args("nursery", true, false, VMRequest::discontiguous()),
4545
true,
4646
);
4747
let full_heap_gc_count = args

‎src/plan/generational/immix/global.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ impl<VM: VMBinding> GenImmix<VM> {
247247
crate::plan::generational::new_generational_global_metadata_specs::<VM>(),
248248
};
249249
let immix_space = ImmixSpace::new(
250-
plan_args.get_space_args("immix_mature", true, VMRequest::discontiguous()),
250+
plan_args.get_space_args("immix_mature", true, false, VMRequest::discontiguous()),
251251
ImmixSpaceArgs {
252252
reset_log_bit_in_major_gc: false,
253253
// We don't need to unlog objects at tracing. Instead, we unlog objects at copying.

‎src/plan/global.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -396,11 +396,13 @@ impl<'a, VM: VMBinding> CreateSpecificPlanArgs<'a, VM> {
396396
&mut self,
397397
name: &'static str,
398398
zeroed: bool,
399+
permission_exec: bool,
399400
vmrequest: VMRequest,
400401
) -> PlanCreateSpaceArgs<VM> {
401402
PlanCreateSpaceArgs {
402403
name,
403404
zeroed,
405+
permission_exec,
404406
vmrequest,
405407
global_side_metadata_specs: self.global_side_metadata_specs.clone(),
406408
vm_map: self.global_args.vm_map,
@@ -409,7 +411,7 @@ impl<'a, VM: VMBinding> CreateSpecificPlanArgs<'a, VM> {
409411
constraints: self.constraints,
410412
gc_trigger: self.global_args.gc_trigger.clone(),
411413
scheduler: self.global_args.scheduler.clone(),
412-
options: &self.global_args.options,
414+
options: self.global_args.options.clone(),
413415
global_state: self.global_args.state.clone(),
414416
}
415417
}
@@ -423,24 +425,28 @@ impl<VM: VMBinding> BasePlan<VM> {
423425
code_space: ImmortalSpace::new(args.get_space_args(
424426
"code_space",
425427
true,
428+
true,
426429
VMRequest::discontiguous(),
427430
)),
428431
#[cfg(feature = "code_space")]
429432
code_lo_space: ImmortalSpace::new(args.get_space_args(
430433
"code_lo_space",
431434
true,
435+
true,
432436
VMRequest::discontiguous(),
433437
)),
434438
#[cfg(feature = "ro_space")]
435439
ro_space: ImmortalSpace::new(args.get_space_args(
436440
"ro_space",
437441
true,
442+
false,
438443
VMRequest::discontiguous(),
439444
)),
440445
#[cfg(feature = "vm_space")]
441446
vm_space: VMSpace::new(args.get_space_args(
442447
"vm_space",
443448
false,
449+
false, // it doesn't matter -- we are not mmapping for VM space.
444450
VMRequest::discontiguous(),
445451
)),
446452

@@ -585,15 +591,17 @@ impl<VM: VMBinding> CommonPlan<VM> {
585591
immortal: ImmortalSpace::new(args.get_space_args(
586592
"immortal",
587593
true,
594+
false,
588595
VMRequest::discontiguous(),
589596
)),
590597
los: LargeObjectSpace::new(
591-
args.get_space_args("los", true, VMRequest::discontiguous()),
598+
args.get_space_args("los", true, false, VMRequest::discontiguous()),
592599
false,
593600
),
594601
nonmoving: ImmortalSpace::new(args.get_space_args(
595602
"nonmoving",
596603
true,
604+
false,
597605
VMRequest::discontiguous(),
598606
)),
599607
base: BasePlan::new(args),

‎src/plan/immix/global.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ impl<VM: VMBinding> Immix<VM> {
150150
) -> Self {
151151
let immix = Immix {
152152
immix_space: ImmixSpace::new(
153-
plan_args.get_space_args("immix", true, VMRequest::discontiguous()),
153+
plan_args.get_space_args("immix", true, false, VMRequest::discontiguous()),
154154
space_args,
155155
),
156156
common: CommonPlan::new(plan_args),

‎src/plan/markcompact/global.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,12 @@ impl<VM: VMBinding> MarkCompact<VM> {
195195
global_side_metadata_specs,
196196
};
197197

198-
let mc_space =
199-
MarkCompactSpace::new(plan_args.get_space_args("mc", true, VMRequest::discontiguous()));
198+
let mc_space = MarkCompactSpace::new(plan_args.get_space_args(
199+
"mc",
200+
true,
201+
false,
202+
VMRequest::discontiguous(),
203+
));
200204

201205
let res = MarkCompact {
202206
mc_space,

‎src/plan/marksweep/global.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ impl<VM: VMBinding> MarkSweep<VM> {
109109
ms: MarkSweepSpace::new(plan_args.get_space_args(
110110
"ms",
111111
true,
112+
false,
112113
VMRequest::discontiguous(),
113114
)),
114115
common: CommonPlan::new(plan_args),

‎src/plan/nogc/global.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,19 @@ impl<VM: VMBinding> NoGC<VM> {
9898
nogc_space: NoGCImmortalSpace::new(plan_args.get_space_args(
9999
"nogc_space",
100100
cfg!(not(feature = "nogc_no_zeroing")),
101+
false,
101102
VMRequest::discontiguous(),
102103
)),
103104
immortal: ImmortalSpace::new(plan_args.get_space_args(
104105
"immortal",
105106
true,
107+
false,
106108
VMRequest::discontiguous(),
107109
)),
108110
los: ImmortalSpace::new(plan_args.get_space_args(
109111
"los",
110112
true,
113+
false,
111114
VMRequest::discontiguous(),
112115
)),
113116
base: BasePlan::new(plan_args),

‎src/plan/pageprotect/global.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ impl<VM: VMBinding> PageProtect<VM> {
105105

106106
let ret = PageProtect {
107107
space: LargeObjectSpace::new(
108-
plan_args.get_space_args("pageprotect", true, VMRequest::discontiguous()),
108+
plan_args.get_space_args("pageprotect", true, false, VMRequest::discontiguous()),
109109
true,
110110
),
111111
common: CommonPlan::new(plan_args),

‎src/plan/semispace/global.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,11 @@ impl<VM: VMBinding> SemiSpace<VM> {
143143
let res = SemiSpace {
144144
hi: AtomicBool::new(false),
145145
copyspace0: CopySpace::new(
146-
plan_args.get_space_args("copyspace0", true, VMRequest::discontiguous()),
146+
plan_args.get_space_args("copyspace0", true, false, VMRequest::discontiguous()),
147147
false,
148148
),
149149
copyspace1: CopySpace::new(
150-
plan_args.get_space_args("copyspace1", true, VMRequest::discontiguous()),
150+
plan_args.get_space_args("copyspace1", true, false, VMRequest::discontiguous()),
151151
true,
152152
),
153153
common: CommonPlan::new(plan_args),

‎src/policy/largeobjectspace.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,11 @@ impl<VM: VMBinding> LargeObjectSpace<VM> {
212212
} else {
213213
FreeListPageResource::new_contiguous(common.start, common.extent, vm_map)
214214
};
215-
pr.protect_memory_on_release = protect_memory_on_release;
215+
pr.protect_memory_on_release = if protect_memory_on_release {
216+
Some(common.mmap_strategy().prot)
217+
} else {
218+
None
219+
};
216220
LargeObjectSpace {
217221
pr,
218222
common,

‎src/policy/lockfreeimmortalspace.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,11 +232,10 @@ impl<VM: VMBinding> LockFreeImmortalSpace<VM> {
232232
};
233233

234234
// Eagerly memory map the entire heap (also zero all the memory)
235-
let strategy = if *args.options.transparent_hugepages {
236-
MmapStrategy::TransparentHugePages
237-
} else {
238-
MmapStrategy::Normal
239-
};
235+
let strategy = MmapStrategy::new(
236+
*args.options.transparent_hugepages,
237+
crate::util::memory::MmapProtection::ReadWrite,
238+
);
240239
crate::util::memory::dzmmap_noreplace(start, aligned_total_bytes, strategy).unwrap();
241240
if space
242241
.metadata

‎src/policy/space.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ use crate::util::heap::layout::Mmapper;
2828
use crate::util::heap::layout::VMMap;
2929
use crate::util::heap::space_descriptor::SpaceDescriptor;
3030
use crate::util::heap::HeapMeta;
31-
use crate::util::memory;
31+
use crate::util::memory::{self, HugePageSupport, MmapProtection, MmapStrategy};
3232
use crate::vm::VMBinding;
3333

3434
use std::marker::PhantomData;
@@ -137,13 +137,13 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
137137
);
138138
let bytes = conversions::pages_to_bytes(res.pages);
139139

140-
let map_sidemetadata = || {
140+
let mmap = || {
141141
// Mmap the pages and the side metadata, and handle error. In case of any error,
142142
// we will either call back to the VM for OOM, or simply panic.
143143
if let Err(mmap_error) = self
144144
.common()
145145
.mmapper
146-
.ensure_mapped(res.start, res.pages)
146+
.ensure_mapped(res.start, res.pages, self.common().mmap_strategy())
147147
.and(
148148
self.common()
149149
.metadata
@@ -160,7 +160,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
160160
// The scope of the lock is important in terms of performance when we have many allocator threads.
161161
if SFT_MAP.get_side_metadata().is_some() {
162162
// If the SFT map uses side metadata, so we have to initialize side metadata first.
163-
map_sidemetadata();
163+
mmap();
164164
// then grow space, which will use the side metadata we mapped above
165165
grow_space();
166166
// then we can drop the lock after grow_space()
@@ -170,7 +170,7 @@ pub trait Space<VM: VMBinding>: 'static + SFT + Sync + Downcast {
170170
grow_space();
171171
drop(lock);
172172
// and map side metadata without holding the lock
173-
map_sidemetadata();
173+
mmap();
174174
}
175175

176176
// TODO: Concurrent zeroing
@@ -421,11 +421,13 @@ pub struct CommonSpace<VM: VMBinding> {
421421
// the copy semantics for the space.
422422
pub copy: Option<CopySemantics>,
423423

424-
immortal: bool,
425-
movable: bool,
424+
pub immortal: bool,
425+
pub movable: bool,
426426
pub contiguous: bool,
427427
pub zeroed: bool,
428428

429+
pub permission_exec: bool,
430+
429431
pub start: Address,
430432
pub extent: usize,
431433

@@ -443,6 +445,7 @@ pub struct CommonSpace<VM: VMBinding> {
443445

444446
pub gc_trigger: Arc<GCTrigger<VM>>,
445447
pub global_state: Arc<GlobalState>,
448+
pub options: Arc<Options>,
446449

447450
p: PhantomData<VM>,
448451
}
@@ -459,6 +462,7 @@ pub struct PolicyCreateSpaceArgs<'a, VM: VMBinding> {
459462
pub struct PlanCreateSpaceArgs<'a, VM: VMBinding> {
460463
pub name: &'static str,
461464
pub zeroed: bool,
465+
pub permission_exec: bool,
462466
pub vmrequest: VMRequest,
463467
pub global_side_metadata_specs: Vec<SideMetadataSpec>,
464468
pub vm_map: &'static dyn VMMap,
@@ -467,7 +471,7 @@ pub struct PlanCreateSpaceArgs<'a, VM: VMBinding> {
467471
pub constraints: &'a PlanConstraints,
468472
pub gc_trigger: Arc<GCTrigger<VM>>,
469473
pub scheduler: Arc<GCWorkScheduler<VM>>,
470-
pub options: &'a Options,
474+
pub options: Arc<Options>,
471475
pub global_state: Arc<GlobalState>,
472476
}
473477

@@ -498,6 +502,7 @@ impl<VM: VMBinding> CommonSpace<VM> {
498502
immortal: args.immortal,
499503
movable: args.movable,
500504
contiguous: true,
505+
permission_exec: args.plan_args.permission_exec,
501506
zeroed: args.plan_args.zeroed,
502507
start: unsafe { Address::zero() },
503508
extent: 0,
@@ -511,6 +516,7 @@ impl<VM: VMBinding> CommonSpace<VM> {
511516
},
512517
acquire_lock: Mutex::new(()),
513518
global_state: args.plan_args.global_state,
519+
options: args.plan_args.options.clone(),
514520
p: PhantomData,
515521
};
516522

@@ -619,6 +625,21 @@ impl<VM: VMBinding> CommonSpace<VM> {
619625
pub fn vm_map(&self) -> &'static dyn VMMap {
620626
self.vm_map
621627
}
628+
629+
pub fn mmap_strategy(&self) -> MmapStrategy {
630+
MmapStrategy {
631+
huge_page: if *self.options.transparent_hugepages {
632+
HugePageSupport::TransparentHugePages
633+
} else {
634+
HugePageSupport::No
635+
},
636+
prot: if self.permission_exec || cfg!(feature = "exec_permission_on_all_spaces") {
637+
MmapProtection::ReadWriteExec
638+
} else {
639+
MmapProtection::ReadWrite
640+
},
641+
}
642+
}
622643
}
623644

624645
fn get_frac_available(frac: f32) -> usize {

0 commit comments

Comments
 (0)