Skip to content

Commit

Permalink
analyze: more def list options (#1175)
Browse files Browse the repository at this point in the history 
This adds more command line options for controlling which defs get
rewritten. Specifically, it adds two new options:

* `--force-rewrite-defs-list FILE`: Reads from `FILE` a list of `DefId`s
that should always be rewritten, even if related defs encounter analysis
or rewriting errors.
* `--skip-pointee-defs-list FILE`: Reads from `FILE` a list of `DefId`s
for which pointee analysis should be skipped.

Both options take input in the same format as the existing
`--fixed-defs-list FILE` option.

These options were useful for getting the `buffer` module to rewrite
successfully while running `c2rust-analyze` against the entire lighttpd
codebase.

This PR also fixes a bug that caused `DefId`s to be erroneously rejected
when parsing the `--fixed-defs-list` file.
  • Loading branch information
@spernsteiner
spernsteiner authored Dec 3, 2024
2 parents ff0fe1a + 5073cc4 commit 33465fe
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 7 deletions.
81 changes: 74 additions & 7 deletions c2rust-analyze/src/analyze.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ fn parse_def_id(s: &str) -> Result<DefId, String> {
};

let rendered = format!("{:?}", def_id);
if &rendered != s {
if &rendered != orig_s {
return Err(format!(
"path mismatch: after parsing input {}, obtained a different path {:?}",
orig_s, def_id
Expand All @@ -435,7 +435,7 @@ fn parse_def_id(s: &str) -> Result<DefId, String> {
Ok(def_id)
}

fn read_fixed_defs_list(fixed_defs: &mut HashSet<DefId>, path: &str) -> io::Result<()> {
fn read_defs_list(defs: &mut HashSet<DefId>, path: &str) -> io::Result<()> {
let f = BufReader::new(File::open(path)?);
for (i, line) in f.lines().enumerate() {
let line = line?;
Expand All @@ -447,7 +447,7 @@ fn read_fixed_defs_list(fixed_defs: &mut HashSet<DefId>, path: &str) -> io::Resu
let def_id = parse_def_id(line).unwrap_or_else(|e| {
panic!("failed to parse {} line {}: {}", path, i + 1, e);
});
fixed_defs.insert(def_id);
defs.insert(def_id);
}
Ok(())
}
Expand Down Expand Up @@ -512,14 +512,30 @@ fn check_rewrite_path_prefixes(tcx: TyCtxt, fixed_defs: &mut HashSet<DefId>, pre
fn get_fixed_defs(tcx: TyCtxt) -> io::Result<HashSet<DefId>> {
let mut fixed_defs = HashSet::new();
if let Ok(path) = env::var("C2RUST_ANALYZE_FIXED_DEFS_LIST") {
read_fixed_defs_list(&mut fixed_defs, &path)?;
read_defs_list(&mut fixed_defs, &path)?;
}
if let Ok(prefixes) = env::var("C2RUST_ANALYZE_REWRITE_PATHS") {
check_rewrite_path_prefixes(tcx, &mut fixed_defs, &prefixes);
}
Ok(fixed_defs)
}

fn get_force_rewrite_defs() -> io::Result<HashSet<DefId>> {
let mut force_rewrite = HashSet::new();
if let Ok(path) = env::var("C2RUST_ANALYZE_FORCE_REWRITE_LIST") {
read_defs_list(&mut force_rewrite, &path)?;
}
Ok(force_rewrite)
}

fn get_skip_pointee_defs() -> io::Result<HashSet<DefId>> {
let mut skip_pointee = HashSet::new();
if let Ok(path) = env::var("C2RUST_ANALYZE_SKIP_POINTEE_LIST") {
read_defs_list(&mut skip_pointee, &path)?;
}
Ok(skip_pointee)
}

/// Local information, specific to a single function. Many of the data structures we use for
/// the pointer analysis have a "global" part that's shared between all functions and a "local"
/// part that's specific to the function being analyzed; this struct contains only the local
Expand Down Expand Up @@ -547,7 +563,14 @@ struct FuncInfo<'tcx> {
fn run(tcx: TyCtxt) {
debug!("all defs:");
for ldid in tcx.hir_crate_items(()).definitions() {
//debug!("{:?} @ {:?}", ldid, tcx.source_span(ldid));
debug!("{:?}", ldid);
if tcx.def_kind(ldid) == DefKind::Struct {
let adt_def = tcx.adt_def(ldid);
for field in &adt_def.non_enum_variant().fields {
debug!("{:?}", field.did);
}
}
}

// Load the list of fixed defs early, so any errors are reported immediately.
Expand All @@ -568,6 +591,14 @@ fn run(tcx: TyCtxt) {
debug!(" {:?}", ldid);
}

gacx.force_rewrite = get_force_rewrite_defs().unwrap();
eprintln!("{} force_rewrite defs", gacx.force_rewrite.len());
let mut xs = gacx.force_rewrite.iter().copied().collect::<Vec<_>>();
xs.sort();
for x in xs {
eprintln!("{:?}", x);
}

populate_field_users(&mut gacx, &all_fn_ldids);

// ----------------------------------
Expand Down Expand Up @@ -1838,6 +1869,8 @@ fn do_pointee_type<'tcx>(
GlobalPointerTable::<PointeeTypes>::new(gacx.num_global_pointers());
let mut pointee_vars = pointee_type::VarTable::default();

let skip_pointee = get_skip_pointee_defs().unwrap();

for &ldid in all_fn_ldids {
if gacx.fn_analysis_invalid(ldid.to_def_id()) {
continue;
Expand All @@ -1849,9 +1882,14 @@ fn do_pointee_type<'tcx>(
let mir = mir.borrow();
let acx = gacx.function_context_with_data(&mir, info.acx_data.take());

let r = panic_detail::catch_unwind(AssertUnwindSafe(|| {
pointee_type::generate_constraints(&acx, &mir, &mut pointee_vars)
}));
let r = if !skip_pointee.contains(&ldid.to_def_id()) {
panic_detail::catch_unwind(AssertUnwindSafe(|| {
pointee_type::generate_constraints(&acx, &mir, &mut pointee_vars)
}))
} else {
// For defs in the skip_pointee set, build an empty constraint set.
Ok(pointee_type::ConstraintSet::default())
};

let local_pointee_types = LocalPointerTable::new(acx.local_ptr_base(), acx.num_pointers());
info.acx_data.set(acx.into_data());
Expand Down Expand Up @@ -2598,7 +2636,15 @@ fn process_new_dont_rewrite_items(gacx: &mut GlobalAnalysisCtxt, asn: &mut Assig
let mut found_any = false;

for did in gacx.dont_rewrite_fns.take_new_keys() {
if gacx.force_rewrite.contains(&did) {
eprintln!("process_new_dont_rewrite_items: mark sig of {did:?} fixed: {:?} - IGNORED due to force_rewrite", gacx.dont_rewrite_fns.get(did));
continue;
}
found_any = true;
eprintln!(
"process_new_dont_rewrite_items: mark sig of {did:?} fixed: {:?}",
gacx.dont_rewrite_fns.get(did)
);
let lsig = &gacx.fn_sigs[&did];
make_sig_fixed(asn, lsig);

Expand All @@ -2608,6 +2654,11 @@ fn process_new_dont_rewrite_items(gacx: &mut GlobalAnalysisCtxt, asn: &mut Assig
};

for &field_ldid in gacx.fn_fields_used.get(ldid) {
if gacx.force_rewrite.contains(&field_ldid.to_def_id()) {
eprintln!("process_new_dont_rewrite_items: mark field {field_ldid:?} fixed: user {did:?} is not rewritten - IGNORED due to force_rewrite");
continue;
}
eprintln!("process_new_dont_rewrite_items: mark field {field_ldid:?} fixed: user {did:?} is not rewritten");
gacx.dont_rewrite_fields.add(
field_ldid.to_def_id(),
DontRewriteFieldReason::NON_REWRITTEN_USE,
Expand All @@ -2618,13 +2669,29 @@ fn process_new_dont_rewrite_items(gacx: &mut GlobalAnalysisCtxt, asn: &mut Assig
}

for did in gacx.dont_rewrite_statics.take_new_keys() {
if gacx.force_rewrite.contains(&did) {
eprintln!("process_new_dont_rewrite_items: mark static {did:?} fixed: {:?} - IGNORED due to force_rewrite", gacx.dont_rewrite_statics.get(did));
continue;
}
found_any = true;
eprintln!(
"process_new_dont_rewrite_items: mark static {did:?} fixed: {:?}",
gacx.dont_rewrite_statics.get(did)
);
let lty = gacx.static_tys[&did];
make_ty_fixed(asn, lty);
}

for did in gacx.dont_rewrite_fields.take_new_keys() {
if gacx.force_rewrite.contains(&did) {
eprintln!("process_new_dont_rewrite_items: mark field {did:?} fixed: {:?} - IGNORED due to force_rewrite", gacx.dont_rewrite_fields.get(did));
continue;
}
found_any = true;
eprintln!(
"process_new_dont_rewrite_items: mark field {did:?} fixed: {:?}",
gacx.dont_rewrite_fields.get(did)
);
let lty = gacx.field_ltys[&did];
make_ty_fixed(asn, lty);
}
Expand Down
5 changes: 5 additions & 0 deletions c2rust-analyze/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,9 @@ pub struct GlobalAnalysisCtxt<'tcx> {
pub dont_rewrite_fns: FlagMap<DefId, DontRewriteFnReason>,
pub dont_rewrite_statics: FlagMap<DefId, DontRewriteStaticReason>,
pub dont_rewrite_fields: FlagMap<DefId, DontRewriteFieldReason>,
/// Never mark these defs as `FIXED`, regardless of what `DontRewriteReason`s they might
/// acquire.
pub force_rewrite: HashSet<DefId>,

/// `DefId`s of functions where analysis failed, and a [`PanicDetail`] explaining the reason
/// for each failure.
Expand Down Expand Up @@ -810,6 +813,7 @@ impl<'tcx> GlobalAnalysisCtxt<'tcx> {
dont_rewrite_fns: FlagMap::new(),
dont_rewrite_statics: FlagMap::new(),
dont_rewrite_fields: FlagMap::new(),
force_rewrite: HashSet::new(),
fns_failed: HashMap::new(),
field_ltys: HashMap::new(),
field_users: MultiMap::new(),
Expand Down Expand Up @@ -889,6 +893,7 @@ impl<'tcx> GlobalAnalysisCtxt<'tcx> {
dont_rewrite_fns: _,
dont_rewrite_statics: _,
dont_rewrite_fields: _,
force_rewrite: _,
fns_failed: _,
ref mut field_ltys,
field_users: _,
Expand Down
20 changes: 20 additions & 0 deletions c2rust-analyze/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,16 @@ struct Args {
#[clap(long)]
fixed_defs_list: Option<PathBuf>,

/// Read a list of defs that should always be rewritable (not `FIXED`) from this file path.
/// This suppresses the rewriter's default behavior of skipping over defs that encounter
/// analysis or rewriting errors.
#[clap(long)]
force_rewrite_defs_list: Option<PathBuf>,

/// Read a list of defs on which the pointee type analysis should be skipped.
#[clap(long)]
skip_pointee_defs_list: Option<PathBuf>,

/// `cargo` args.
cargo_args: Vec<OsString>,
}
Expand Down Expand Up @@ -391,6 +401,8 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> {
rewrite_in_place,
use_manual_shims,
fixed_defs_list,
force_rewrite_defs_list,
skip_pointee_defs_list,
cargo_args,
} = Args::parse();

Expand Down Expand Up @@ -438,6 +450,14 @@ fn cargo_wrapper(rustc_wrapper: &Path) -> anyhow::Result<()> {
cmd.env("C2RUST_ANALYZE_FIXED_DEFS_LIST", fixed_defs_list);
}

if let Some(ref force_rewrite_defs_list) = force_rewrite_defs_list {
cmd.env("C2RUST_ANALYZE_FORCE_REWRITE_LIST", force_rewrite_defs_list);
}

if let Some(ref skip_pointee_defs_list) = skip_pointee_defs_list {
cmd.env("C2RUST_ANALYZE_SKIP_POINTEE_LIST", skip_pointee_defs_list);
}

if !rewrite_paths.is_empty() {
let rewrite_paths = rewrite_paths.join(OsStr::new(","));
cmd.env("C2RUST_ANALYZE_REWRITE_PATHS", rewrite_paths);
Expand Down

0 comments on commit 33465fe

Please sign in to comment.