Skip to content

(c2rust-analyze) Handle inline const refs, including string literals #886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f34a3c1
(`c2rust-analyze/tests`) Added more `string_literals.rs` tests to nar…
kkysen Feb 19, 2023
96ad50c
(`c2rust-analyze/tests`) Split `string_literals` test cases in inline…
kkysen Apr 6, 2023
2ba20fa
(`c2rust-analyze`) Handle local `const` refs (incl. `""`, `b""`) with…
kkysen Apr 10, 2023
4ae78c8
(`c2rust-analyze`) Restrict `const_tys => const_ref_tys` to just `ty:…
kkysen Apr 13, 2023
bed143d
(`c2rust-analyze`) Add known perms for const refs before dataflow typ…
kkysen Apr 13, 2023
a196089
(`c2rust-analyze`) Move `fn const_perms` to `pub fn PermissionSet::fo…
kkysen Apr 17, 2023
de38cfc
(`c2rust-analyze`) Move and fully implement the post-typeck const ref…
kkysen Apr 17, 2023
91bfa85
(`c2rust-analyze`) Ignore `UNIQUE` in checking const refs' perms post…
kkysen Apr 21, 2023
7a64e58
(`c2rust-analyze`) Use `Self` in `PermissionSet::for_const`.
kkysen Apr 21, 2023
f48e239
(`c2rust-analyze`) Rename `PermissionSet::{for_const => for_const_ref…
kkysen Apr 21, 2023
e451ab0
(`c2rust-analyze`) Refactor out `PermissionSet::for_const_ref_ty` fro…
kkysen Apr 21, 2023
8665bc2
(`c2rust-analyze`) Replace `AnalysisCtxt::const_ref_{tys: HashMap<Con…
kkysen Apr 21, 2023
36cdf90
(`c2rust-analyze`) Refactor out `AnalysisCtxt::const_ref_tys() -> imp…
kkysen Apr 21, 2023
d4f269a
(`c2rust-analyze`) Refactor out `AnalysisCtxt::check_const_ref_perms`.
kkysen Apr 21, 2023
f14050b
(`c2rust-analyze`) Refactor out `AnalysisCtxt::const_ref_perms` (need…
kkysen Apr 21, 2023
57dd358
Merge branch 'master' into kkysen/analyze-string-literals
kkysen Apr 21, 2023
012c46f
(`c2rust-analyze`) Forgot to actually add the const ref `Location`s t…
kkysen Apr 21, 2023
6c8f65c
(`c2rust-analyze`) Rewrite the `ConstantKind` check as a `match` not …
kkysen Apr 22, 2023
6a06083
(`c2rust-analyze`) Error immediately rather than continue and later e…
kkysen Apr 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 60 additions & 8 deletions c2rust-analyze/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::pointer_id::{
GlobalPointerTable, LocalPointerTable, NextGlobalPointerId, NextLocalPointerId, PointerTable,
PointerTableMut,
};
use crate::util::{self, describe_rvalue, RvalueDesc};
use crate::util::{self, describe_rvalue, PhantomLifetime, RvalueDesc};
use crate::AssignPointerIds;
use bitflags::bitflags;
use rustc_hir::def_id::DefId;
Expand All @@ -13,7 +13,7 @@ use rustc_middle::mir::{
Body, Field, HasLocalDecls, Local, LocalDecls, Location, Operand, Place, PlaceElem, PlaceRef,
Rvalue,
};
use rustc_middle::ty::{AdtDef, FieldDef, Ty, TyCtxt, TyKind};
use rustc_middle::ty::{self, AdtDef, FieldDef, Ty, TyCtxt, TyKind};
use std::collections::HashMap;
use std::ops::Index;

Expand Down Expand Up @@ -51,6 +51,22 @@ bitflags! {
}
}

impl PermissionSet {
pub fn for_const_ref_ty(ty: Ty) -> Self {
let ty = match ty.kind() {
ty::Ref(_, ty, _) => ty,
_ => panic!("expected only `Ref`s for constants: {ty:?}"),
};
if ty.is_array() || ty.is_str() {
Self::READ | Self::OFFSET_ADD
} else if ty.is_primitive_ty() {
Self::READ
} else {
panic!("expected an array, str, or primitive type: {ty:?}");
}
}
}

bitflags! {
/// Additional flags describing a given pointer type. These are mainly derived from
/// `PermissionSet`, but don't follow the normal subtyping rules and propagation algorithm.
Expand Down Expand Up @@ -97,13 +113,44 @@ pub struct AnalysisCtxt<'a, 'tcx> {
/// those `PointerId`s consistent, the `Rvalue`'s type must be stored rather than recomputed on
/// the fly.
pub rvalue_tys: HashMap<Location, LTy<'tcx>>,

/// [`Location`]s of const ref [`rvalue_tys`](Self::rvalue_tys).
pub const_ref_locs: Vec<Location>,

next_ptr_id: NextLocalPointerId,
}

impl<'a, 'tcx> AnalysisCtxt<'_, 'tcx> {
pub fn const_ref_tys(&'a self) -> impl Iterator<Item = LTy<'tcx>> + 'a {
self.const_ref_locs.iter().map(|loc| self.rvalue_tys[loc])
}

pub fn const_ref_perms(
&'a self,
) -> impl Iterator<Item = (PointerId, PermissionSet)> + PhantomLifetime<'tcx> + 'a {
self.const_ref_tys()
.map(|lty| (lty.label, PermissionSet::for_const_ref_ty(lty.ty)))
}

pub fn check_const_ref_perms(&self, asn: &Assignment) {
for const_ref_lty in self.const_ref_tys() {
let ptr_id = const_ref_lty.label;
let expected_perms = PermissionSet::for_const_ref_ty(const_ref_lty.ty);
let mut actual_perms = asn.perms()[ptr_id];
// Ignore `UNIQUE` as it gets automatically added to all permissions
// and then removed later if it can't apply.
// We don't care about `UNIQUE` for const refs, so just unset it here.
actual_perms.set(PermissionSet::UNIQUE, false);
assert_eq!(expected_perms, actual_perms);
}
}
}

pub struct AnalysisCtxtData<'tcx> {
local_tys: IndexVec<Local, LTy<'tcx>>,
addr_of_local: IndexVec<Local, PointerId>,
rvalue_tys: HashMap<Location, LTy<'tcx>>,
const_ref_locs: Vec<Location>,
next_ptr_id: NextLocalPointerId,
}

Expand Down Expand Up @@ -196,6 +243,7 @@ impl<'a, 'tcx> AnalysisCtxt<'a, 'tcx> {
c_void_casts: CVoidCasts::new(mir, tcx),
addr_of_local: IndexVec::new(),
rvalue_tys: HashMap::new(),
const_ref_locs: Default::default(),
next_ptr_id: NextLocalPointerId::new(),
}
}
Expand All @@ -209,6 +257,7 @@ impl<'a, 'tcx> AnalysisCtxt<'a, 'tcx> {
local_tys,
addr_of_local,
rvalue_tys,
const_ref_locs,
next_ptr_id,
} = data;
AnalysisCtxt {
Expand All @@ -218,6 +267,7 @@ impl<'a, 'tcx> AnalysisCtxt<'a, 'tcx> {
c_void_casts: CVoidCasts::default(),
addr_of_local,
rvalue_tys,
const_ref_locs,
next_ptr_id,
}
}
Expand All @@ -227,6 +277,7 @@ impl<'a, 'tcx> AnalysisCtxt<'a, 'tcx> {
local_tys: self.local_tys,
addr_of_local: self.addr_of_local,
rvalue_tys: self.rvalue_tys,
const_ref_locs: self.const_ref_locs,
next_ptr_id: self.next_ptr_id,
}
}
Expand Down Expand Up @@ -363,12 +414,13 @@ impl<'tcx> AnalysisCtxtData<'tcx> {
map: PointerTable<PointerId>,
counter: NextLocalPointerId,
) {
let AnalysisCtxtData {
ref mut local_tys,
ref mut addr_of_local,
ref mut rvalue_tys,
ref mut next_ptr_id,
} = *self;
let Self {
local_tys,
addr_of_local,
rvalue_tys,
const_ref_locs: _,
next_ptr_id,
} = self;

for lty in local_tys {
*lty = remap_lty_pointers(lcx, &map, lty);
Expand Down
4 changes: 4 additions & 0 deletions c2rust-analyze/src/dataflow/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,10 @@ pub fn visit<'tcx>(
equiv_constraints: Vec::new(),
};

for (ptr, perms) in acx.const_ref_perms() {
tc.constraints.add_all_perms(ptr, perms);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not add the constraints as you see consts in visit_rvalue?

}

for (bb, bb_data) in mir.basic_blocks().iter_enumerated() {
for (i, stmt) in bb_data.statements.iter().enumerate() {
tc.visit_statement(
Expand Down
28 changes: 26 additions & 2 deletions c2rust-analyze/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_index::vec::IndexVec;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{
AggregateKind, BindingForm, Body, LocalDecl, LocalInfo, LocalKind, Location, Operand, Rvalue,
StatementKind,
AggregateKind, BindingForm, Body, ConstantKind, LocalDecl, LocalInfo, LocalKind, Location,
Operand, Rvalue, StatementKind,
};
use rustc_middle::ty::tls;
use rustc_middle::ty::{GenericArgKind, Ty, TyCtxt, TyKind, WithOptConstParam};
Expand Down Expand Up @@ -397,6 +397,28 @@ fn label_rvalue_tys<'tcx>(acx: &mut AnalysisCtxt<'_, 'tcx>, mir: &Body<'tcx>) {
_ => continue,
},
Rvalue::Cast(_, _, ty) => acx.assign_pointer_ids(*ty),
Rvalue::Use(Operand::Constant(c)) => {
let c = &**c;
if !c.ty().is_ref() {
continue;
}
// Constants can include, for example, `""` and `b""` string literals.
match c.literal {
ConstantKind::Val(_, ty) => {
// The [`Constant`] is an inline value and thus local to this function,
// as opposed to a global, named `const`s, for example.
// This might miss local, named `const`s.
acx.const_ref_locs.push(loc);
acx.assign_pointer_ids(ty)
}
ConstantKind::Ty(ty) => {
::log::error!(
"TODO: handle global, named `const` refs: {c:?}, ty = {ty:?}"
);
continue;
}
}
}
_ => continue,
};

Expand Down Expand Up @@ -609,6 +631,8 @@ fn run(tcx: TyCtxt) {
let mut asn = gasn.and(&mut info.lasn);
info.dataflow.propagate_cell(&mut asn);

acx.check_const_ref_perms(&asn);

// Print labeling and rewrites for the current function.

eprintln!("\nfinal labeling for {:?}:", name);
Expand Down
3 changes: 3 additions & 0 deletions c2rust-analyze/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,6 @@ pub fn is_null_const(constant: Constant) -> bool {
_ => false,
}
}

pub trait PhantomLifetime<'a> {}
impl<'a, T: ?Sized> PhantomLifetime<'a> for T {}
31 changes: 27 additions & 4 deletions c2rust-analyze/tests/analyze/string_literals.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,32 @@
pub fn inline_desugared_bstr() -> &'static [u8; 0] {
&[]
}

const DESUGARED_BSTR: &'static [u8; 0] = &[];

#[cfg(any())]
pub fn outline_desugared_bstr() -> &'static [u8; 0] {
DESUGARED_BSTR
}

pub fn inline_bstr() -> &'static [u8; 0] {
b""
}

const BSTR: &'static [u8; 0] = b"";

#[cfg(any())]
pub fn str() {
"";
pub fn outline_bstr() -> &'static [u8; 0] {
BSTR
}

pub fn inline_str() -> &'static str {
""
}

const STR: &'static str = "";

#[cfg(any())]
pub fn bstr() {
b"";
pub fn outline_str() -> &'static str {
STR
}