Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Commit 18d2f46

Browse files
authored
Merge pull request #4630 from royAmmerschuber/pull-request/wildcard-provenance
initial implementation of wildcard provenence for tree borrows
2 parents 8e39bff + cab7fac commit 18d2f46

43 files changed

Lines changed: 2037 additions & 152 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/tools/miri/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ harness = false
7272
default = ["stack-cache", "native-lib"]
7373
genmc = ["dep:genmc-sys"]
7474
stack-cache = []
75-
stack-cache-consistency-check = ["stack-cache"]
75+
expensive-consistency-checks = ["stack-cache"]
7676
tracing = ["serde_json"]
7777
native-lib = ["dep:libffi", "dep:libloading", "dep:capstone", "dep:ipc-channel", "dep:nix", "dep:serde"]
7878

‎src/tools/miri/src/bin/miri.rs‎

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,6 @@ fn main() {
509509
Some(BorrowTrackerMethod::TreeBorrows(TreeBorrowsParams{
510510
precise_interior_mut:true,
511511
}));
512-
miri_config.provenance_mode = ProvenanceMode::Strict;
513512
}elseif arg == "-Zmiri-tree-borrows-no-precise-interior-mut"{
514513
match&mut miri_config.borrow_tracker{
515514
Some(BorrowTrackerMethod::TreeBorrows(params)) => {
@@ -703,17 +702,6 @@ fn main() {
703702
rustc_args.push(arg);
704703
}
705704
}
706-
// Tree Borrows implies strict provenance, and is not compatible with native calls.
707-
ifmatches!(miri_config.borrow_tracker,Some(BorrowTrackerMethod::TreeBorrows{ .. })){
708-
if miri_config.provenance_mode != ProvenanceMode::Strict{
709-
fatal_error!(
710-
"Tree Borrows does not support integer-to-pointer casts, and hence requires strict provenance"
711-
);
712-
}
713-
if !miri_config.native_lib.is_empty(){
714-
fatal_error!("Tree Borrows is not compatible with calling native functions");
715-
}
716-
}
717705

718706
// Native calls and strict provenance are not compatible.
719707
if !miri_config.native_lib.is_empty() && miri_config.provenance_mode == ProvenanceMode::Strict{

‎src/tools/miri/src/borrow_tracker/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
352352
let borrow_tracker = this.machine.borrow_tracker.as_ref().unwrap();
353353
// The body of this loop needs `borrow_tracker` immutably
354354
// so we can't move this code inside the following `end_call`.
355+
355356
for(alloc_id, tag)in&frame
356357
.extra
357358
.borrow_tracker
@@ -378,6 +379,7 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
378379
}
379380
}
380381
borrow_tracker.borrow_mut().end_call(&frame.extra);
382+
381383
interp_ok(())
382384
}
383385
}

‎src/tools/miri/src/borrow_tracker/stacked_borrows/stack.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ impl<'tcx> Stack {
153153
/// Panics if any of the caching mechanisms have broken,
154154
/// - The StackCache indices don't refer to the parallel items,
155155
/// - There are no Unique items outside of first_unique..last_unique
156-
#[cfg(feature = "stack-cache-consistency-check")]
156+
#[cfg(feature = "expensive-consistency-checks")]
157157
fnverify_cache_consistency(&self){
158158
// Only a full cache needs to be valid. Also see the comments in find_granting_cache
159159
// and set_unknown_bottom.
@@ -197,7 +197,7 @@ impl<'tcx> Stack {
197197
tag:ProvenanceExtra,
198198
exposed_tags:&FxHashSet<BorTag>,
199199
) -> Result<Option<usize>,()>{
200-
#[cfg(feature = "stack-cache-consistency-check")]
200+
#[cfg(feature = "expensive-consistency-checks")]
201201
self.verify_cache_consistency();
202202

203203
letProvenanceExtra::Concrete(tag) = tag else{
@@ -334,7 +334,7 @@ impl<'tcx> Stack {
334334
// This primes the cache for the next access, which is almost always the just-added tag.
335335
self.cache.add(new_idx, new);
336336

337-
#[cfg(feature = "stack-cache-consistency-check")]
337+
#[cfg(feature = "expensive-consistency-checks")]
338338
self.verify_cache_consistency();
339339
}
340340

@@ -417,7 +417,7 @@ impl<'tcx> Stack {
417417
self.unique_range.end = self.unique_range.end.min(disable_start);
418418
}
419419

420-
#[cfg(feature = "stack-cache-consistency-check")]
420+
#[cfg(feature = "expensive-consistency-checks")]
421421
self.verify_cache_consistency();
422422

423423
interp_ok(())
@@ -472,7 +472,7 @@ impl<'tcx> Stack {
472472
self.unique_range = 0..0;
473473
}
474474

475-
#[cfg(feature = "stack-cache-consistency-check")]
475+
#[cfg(feature = "expensive-consistency-checks")]
476476
self.verify_cache_consistency();
477477
interp_ok(())
478478
}

‎src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,10 @@ pub(super) struct TbError<'node> {
291291
pubconflicting_info:&'nodeNodeDebugInfo,
292292
// What kind of access caused this error (read, write, reborrow, deallocation)
293293
pubaccess_cause:AccessCause,
294-
/// Which tag the access that caused this error was made through, i.e.
294+
/// Which tag, if any, the access that caused this error was made through, i.e.
295295
/// which tag was used to read/write/deallocate.
296-
pubaccessed_info:&'nodeNodeDebugInfo,
296+
/// Not set on wildcard accesses.
297+
pubaccessed_info:Option<&'nodeNodeDebugInfo>,
297298
}
298299

299300
implTbError<'_>{
@@ -302,10 +303,20 @@ impl TbError<'_> {
302303
useTransitionError::*;
303304
let cause = self.access_cause;
304305
let accessed = self.accessed_info;
306+
let accessed_str =
307+
self.accessed_info.map(|v| format!("{v}")).unwrap_or_else(|| "<wildcard>".into());
305308
let conflicting = self.conflicting_info;
306-
let accessed_is_conflicting = accessed.tag == conflicting.tag;
309+
// An access is considered conflicting if it happened through a
310+
// different tag than the one who caused UB.
311+
// When doing a wildcard access (where `accessed` is `None`) we
312+
// do not know which precise tag the accessed happened from,
313+
// however we can be certain that it did not come from the
314+
// conflicting tag.
315+
// This is because the wildcard data structure already removes
316+
// all tags through which an access would cause UB.
317+
let accessed_is_conflicting = accessed.map(|a| a.tag) == Some(conflicting.tag);
307318
let title = format!(
308-
"{cause} through {accessed} at {alloc_id:?}[{offset:#x}] is forbidden",
319+
"{cause} through {accessed_str} at {alloc_id:?}[{offset:#x}] is forbidden",
309320
alloc_id = self.alloc_id,
310321
offset = self.error_offset
311322
);
@@ -316,7 +327,7 @@ impl TbError<'_> {
316327
letmut details = Vec::new();
317328
if !accessed_is_conflicting {
318329
details.push(format!(
319-
"the accessed tag {accessed} is a child of the conflicting tag {conflicting}"
330+
"the accessed tag {accessed_str} is a child of the conflicting tag {conflicting}"
320331
));
321332
}
322333
let access = cause.print_as_access(/* is_foreign */false);
@@ -330,7 +341,7 @@ impl TbError<'_> {
330341
let access = cause.print_as_access(/* is_foreign */true);
331342
let details = vec![
332343
format!(
333-
"the accessed tag {accessed} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
344+
"the accessed tag {accessed_str} is foreign to the {conflicting_tag_name} tag {conflicting} (i.e., it is not a child)"
334345
),
335346
format!(
336347
"this {access} would cause the {conflicting_tag_name} tag {conflicting} (currently {before_disabled}) to become Disabled"
@@ -343,16 +354,18 @@ impl TbError<'_> {
343354
let conflicting_tag_name = "strongly protected";
344355
let details = vec![
345356
format!(
346-
"the allocation of the accessed tag {accessed} also contains the {conflicting_tag_name} tag {conflicting}"
357+
"the allocation of the accessed tag {accessed_str} also contains the {conflicting_tag_name} tag {conflicting}"
347358
),
348359
format!("the {conflicting_tag_name} tag {conflicting} disallows deallocations"),
349360
];
350361
(title, details, conflicting_tag_name)
351362
}
352363
};
353364
letmut history = HistoryData::default();
354-
if !accessed_is_conflicting {
355-
history.extend(self.accessed_info.history.forget(),"accessed",false);
365+
ifletSome(accessed_info) = self.accessed_info
366+
&& !accessed_is_conflicting
367+
{
368+
history.extend(accessed_info.history.forget(),"accessed",false);
356369
}
357370
history.extend(
358371
self.conflicting_info.history.extract_relevant(self.error_offset,self.error_kind),
@@ -363,6 +376,20 @@ impl TbError<'_> {
363376
}
364377
}
365378

379+
/// Cannot access this allocation with wildcard provenance, as there are no
380+
/// valid exposed references for this access kind.
381+
pubfnno_valid_exposed_references_error<'tcx>(
382+
alloc_id:AllocId,
383+
offset:u64,
384+
access_cause:AccessCause,
385+
) -> InterpErrorKind<'tcx>{
386+
let title =
387+
format!("{access_cause} through <wildcard> at {alloc_id:?}[{offset:#x}] is forbidden");
388+
let details = vec![format!("there are no exposed tags which may perform this access here")];
389+
let history = HistoryData::default();
390+
err_machine_stop!(TerminationInfo::TreeBorrowsUb{ title, details, history })
391+
}
392+
366393
typeS = &'staticstr;
367394
/// Pretty-printing details
368395
///
@@ -623,10 +650,10 @@ impl DisplayRepr {
623650
}else{
624651
// We take this node
625652
let rperm = tree
626-
.rperms
653+
.locations
627654
.iter_all()
628-
.map(move |(_offset,perms)| {
629-
let perm = perms.get(idx);
655+
.map(move |(_offset,loc)| {
656+
let perm = loc.perms.get(idx);
630657
perm.cloned()
631658
})
632659
.collect::<Vec<_>>();
@@ -788,7 +815,7 @@ impl<'tcx> Tree {
788815
show_unnamed:bool,
789816
) -> InterpResult<'tcx>{
790817
letmut indenter = DisplayIndent::new();
791-
let ranges = self.rperms.iter_all().map(|(range,_perms)| range).collect::<Vec<_>>();
818+
let ranges = self.locations.iter_all().map(|(range,_loc)| range).collect::<Vec<_>>();
792819
ifletSome(repr) = DisplayRepr::from(self, show_unnamed){
793820
repr.print(
794821
&DEFAULT_FORMATTER,

‎src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs‎

Lines changed: 19 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod foreign_access_skipping;
1414
mod perms;
1515
mod tree;
1616
mod unimap;
17+
mod wildcard;
1718

1819
#[cfg(test)]
1920
mod exhaustive;
@@ -54,16 +55,10 @@ impl<'tcx> Tree {
5455
interpret::Pointer::new(alloc_id, range.start),
5556
range.size.bytes(),
5657
);
57-
// TODO: for now we bail out on wildcard pointers. Eventually we should
58-
// handle them as much as we can.
59-
let tag = match prov {
60-
ProvenanceExtra::Concrete(tag) => tag,
61-
ProvenanceExtra::Wildcard => returninterp_ok(()),
62-
};
6358
let global = machine.borrow_tracker.as_ref().unwrap();
6459
let span = machine.current_user_relevant_span();
6560
self.perform_access(
66-
tag,
61+
prov,
6762
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
6863
global,
6964
alloc_id,
@@ -79,19 +74,9 @@ impl<'tcx> Tree {
7974
size:Size,
8075
machine:&MiriMachine<'tcx>,
8176
) -> InterpResult<'tcx>{
82-
// TODO: for now we bail out on wildcard pointers. Eventually we should
83-
// handle them as much as we can.
84-
let tag = match prov {
85-
ProvenanceExtra::Concrete(tag) => tag,
86-
ProvenanceExtra::Wildcard => returninterp_ok(()),
87-
};
8877
let global = machine.borrow_tracker.as_ref().unwrap();
8978
let span = machine.current_user_relevant_span();
90-
self.dealloc(tag,alloc_range(Size::ZERO, size), global, alloc_id, span)
91-
}
92-
93-
pubfnexpose_tag(&mutself,_tag:BorTag){
94-
// TODO
79+
self.dealloc(prov,alloc_range(Size::ZERO, size), global, alloc_id, span)
9580
}
9681

9782
/// A tag just lost its protector.
@@ -109,7 +94,11 @@ impl<'tcx> Tree {
10994
) -> InterpResult<'tcx>{
11095
let span = machine.current_user_relevant_span();
11196
// `None` makes it the magic on-protector-end operation
112-
self.perform_access(tag,None, global, alloc_id, span)
97+
self.perform_access(ProvenanceExtra::Concrete(tag),None, global, alloc_id, span)?;
98+
99+
self.update_exposure_for_protector_release(tag);
100+
101+
interp_ok(())
113102
}
114103
}
115104

@@ -239,21 +228,22 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
239228
assert_eq!(ptr_size,Size::ZERO);// we did the deref check above, size has to be 0 here
240229
// This pointer doesn't come with an AllocId, so there's no
241230
// memory to do retagging in.
231+
let new_prov = place.ptr().provenance;
242232
trace!(
243-
"reborrow of size 0: reference {:?} derived from {:?} (pointee {})",
244-
new_tag,
233+
"reborrow of size 0: reusing {:?} (pointee {})",
245234
place.ptr(),
246235
place.layout.ty,
247236
);
248237
log_creation(this,None)?;
249238
// Keep original provenance.
250-
returninterp_ok(place.ptr().provenance);
239+
returninterp_ok(new_prov);
251240
}
252241
};
242+
253243
log_creation(this,Some((alloc_id, base_offset, parent_prov)))?;
254244

255245
let orig_tag = match parent_prov {
256-
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle wildcard pointers
246+
ProvenanceExtra::Wildcard => returninterp_ok(place.ptr().provenance),// TODO: handle retagging wildcard pointers
257247
ProvenanceExtra::Concrete(tag) => tag,
258248
};
259249

@@ -356,7 +346,7 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
356346
};
357347

358348
tree_borrows.perform_access(
359-
orig_tag,
349+
parent_prov,
360350
Some((range_in_alloc,AccessKind::Read, diagnostics::AccessCause::Reborrow)),
361351
this.machine.borrow_tracker.as_ref().unwrap(),
362352
alloc_id,
@@ -589,7 +579,11 @@ pub trait EvalContextExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
589579
// uncovers a non-supported `extern static`.
590580
let alloc_extra = this.get_alloc_extra(alloc_id)?;
591581
trace!("Tree Borrows tag {tag:?} exposed in {alloc_id:?}");
592-
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag);
582+
583+
let global = this.machine.borrow_tracker.as_ref().unwrap();
584+
let protected_tags = &global.borrow().protected_tags;
585+
let protected = protected_tags.contains_key(&tag);
586+
alloc_extra.borrow_tracker_tb().borrow_mut().expose_tag(tag, protected);
593587
}
594588
AllocKind::Function | AllocKind::VTable | AllocKind::TypeId | AllocKind::Dead => {
595589
// No tree borrows on these allocations.

‎src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ enum PermissionPriv {
5353
}
5454
useself::PermissionPriv::*;
5555
usesuper::foreign_access_skipping::IdempotentForeignAccess;
56+
usesuper::wildcard::WildcardAccessLevel;
5657

5758
implPartialOrdforPermissionPriv{
5859
/// PermissionPriv is ordered by the reflexive transitive closure of
@@ -372,6 +373,23 @@ impl Permission {
372373
pubfnstrongest_idempotent_foreign_access(&self,prot:bool) -> IdempotentForeignAccess{
373374
self.inner.strongest_idempotent_foreign_access(prot)
374375
}
376+
377+
/// Returns the strongest access allowed from a child to this node without
378+
/// causing UB (only considers possible transitions to this permission).
379+
pubfnstrongest_allowed_child_access(&self,protected:bool) -> WildcardAccessLevel{
380+
matchself.inner{
381+
// Everything except disabled can be accessed by read access.
382+
Disabled => WildcardAccessLevel::None,
383+
// Frozen references cannot be written to by a child.
384+
Frozen => WildcardAccessLevel::Read,
385+
// If the `conflicted` flag is set, then there was a foreign read
386+
// during the function call that is still ongoing (still `protected`),
387+
// this is UB (`noalias` violation).
388+
ReservedFrz{conflicted:true}if protected => WildcardAccessLevel::Read,
389+
// Everything else allows writes.
390+
_ => WildcardAccessLevel::Write,
391+
}
392+
}
375393
}
376394

377395
implPermTransition{
@@ -772,4 +790,32 @@ mod propagation_optimization_checks {
772790
);
773791
}
774792
}
793+
794+
/// Checks that `strongest_allowed_child_access` correctly
795+
/// represents which transitions are possible.
796+
#[test]
797+
fnstrongest_allowed_child_access(){
798+
for(permission, protected)in <(Permission,bool)>::exhaustive(){
799+
let strongest_child_access = permission.strongest_allowed_child_access(protected);
800+
801+
let is_read_valid = Permission::perform_access(
802+
AccessKind::Read,
803+
AccessRelatedness::LocalAccess,
804+
permission,
805+
protected,
806+
)
807+
.is_some();
808+
809+
let is_write_valid = Permission::perform_access(
810+
AccessKind::Write,
811+
AccessRelatedness::LocalAccess,
812+
permission,
813+
protected,
814+
)
815+
.is_some();
816+
817+
assert_eq!(is_read_valid, strongest_child_access >= WildcardAccessLevel::Read);
818+
assert_eq!(is_write_valid, strongest_child_access >= WildcardAccessLevel::Write);
819+
}
820+
}
775821
}

0 commit comments

Comments
 (0)