Skip to content
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.

Sign up for GitHub

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

[Bugfix] Fix broken bytecode dependency #14929

Merged
merged 13 commits into from
Oct 30, 2024
Merged

[Bugfix] Fix broken bytecode dependency #14929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
b6da4f7
Add bytecode_deps to CompiledPackage
jasonxh Aug 19, 2024
bbe17fe
fmt
jasonxh Aug 19, 2024
1e90242
error message
jasonxh Aug 19, 2024
77bd075
fixed issue
Oct 9, 2024
2eef9d3
fixed issue
Oct 9, 2024
1aa7b3a
refactor
Oct 10, 2024
dff6918
bug fix
Oct 10, 2024
044d690
resolve merge conflicts
Oct 10, 2024
2fb122b
lint
Oct 10, 2024
b3be8c5
resolve comments
Oct 29, 2024
1058dc2
linter
Oct 29, 2024
14b78ec
config
Oct 30, 2024
5ab6988
sort
Oct 30, 2024
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
1 change: 1 addition & 0 deletions aptos-move/aptos-vm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ aptos-block-executor = { workspace = true, features = ["testing"] }
aptos-language-e2e-tests = { workspace = true }
aptos-types = { workspace = true, features = ["fuzzing"] }
claims = { workspace = true }
move-vm-types = { workspace = true, features = ["testing"] }
proptest = { workspace = true }
rand_core = { workspace = true }

Expand Down
9 changes: 9 additions & 0 deletions aptos-move/framework/src/built_package.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,15 @@ impl BuiltPackage {
},
CompiledUnit::Script(_) => None,
})
.chain(
self.package
.bytecode_deps
.iter()
.map(|(name, address)| PackageDep {
account: address.into_inner(),
package_name: name.as_str().to_string(),
}),
)
.collect::<BTreeSet<_>>()
.into_iter()
.collect();
Expand Down
169 changes: 107 additions & 62 deletions third_party/move/tools/move-package/src/compilation/compiled_package.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
},
Architecture, BuildConfig, CompilerConfig, CompilerVersion,
};
use anyhow::{bail, ensure, Result};
use anyhow::{bail, ensure, Context, Result};
use colored::Colorize;
use itertools::{Either, Itertools};
use move_abigen::{Abigen, AbigenOptions};
Expand Down Expand Up @@ -81,6 +81,8 @@ pub struct CompiledPackage {
pub root_compiled_units: Vec<CompiledUnitWithSource>,
/// The output compiled bytecode for dependencies
pub deps_compiled_units: Vec<(PackageName, CompiledUnitWithSource)>,
/// Bytecode dependencies of this compiled package
pub bytecode_deps: BTreeMap<PackageName, NumericalAddress>,

// Optional artifacts from compilation
//
Expand All @@ -100,6 +102,7 @@ pub struct OnDiskPackage {
pub compiled_package_info: CompiledPackageInfo,
/// Dependency names for this package.
pub dependencies: Vec<PackageName>,
pub bytecode_deps: Vec<PackageName>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down Expand Up @@ -198,6 +201,8 @@ impl OnDiskCompiledPackage {
compiled_package_info: self.package.compiled_package_info.clone(),
root_compiled_units,
deps_compiled_units,
// TODO: support bytecode deps
bytecode_deps: BTreeMap::new(),
compiled_docs,
compiled_abis,
})
Expand Down Expand Up @@ -628,72 +633,75 @@ impl CompiledPackage {
let effective_language_version = config.language_version.unwrap_or_default();
effective_compiler_version.check_language_support(effective_language_version)?;

let (file_map, all_compiled_units, optional_global_env) = match config
.compiler_version
.unwrap_or_default()
{
CompilerVersion::V1 => {
let mut paths = src_deps;
paths.push(sources_package_paths.clone());
let compiler =
Compiler::from_package_paths(paths, bytecode_deps, flags, &known_attributes);
compiler_driver_v1(compiler)?
},
version @ CompilerVersion::V2_0 | version @ CompilerVersion::V2_1 => {
let to_str_vec = |ps: &[Symbol]| {
ps.iter()
.map(move |s| s.as_str().to_owned())
.collect::<Vec<_>>()
};
let mut global_address_map = BTreeMap::new();
for pack in std::iter::once(&sources_package_paths)
.chain(src_deps.iter())
.chain(bytecode_deps.iter())
{
for (name, val) in &pack.named_address_map {
if let Some(old) = global_address_map.insert(name.as_str().to_owned(), *val)
{
if old != *val {
let pack_name = pack
.name
.map(|s| s.as_str().to_owned())
.unwrap_or_else(|| "<unnamed>".to_owned());
bail!(
let (file_map, all_compiled_units, optional_global_env) =
match config.compiler_version.unwrap_or_default() {
CompilerVersion::V1 => {
let mut paths = src_deps;
paths.push(sources_package_paths.clone());
let compiler = Compiler::from_package_paths(
paths,
bytecode_deps.clone(),
flags,
&known_attributes,
);
compiler_driver_v1(compiler)?
},
version @ CompilerVersion::V2_0 | version @ CompilerVersion::V2_1 => {
let to_str_vec = |ps: &[Symbol]| {
ps.iter()
.map(move |s| s.as_str().to_owned())
.collect::<Vec<_>>()
};
let mut global_address_map = BTreeMap::new();
for pack in std::iter::once(&sources_package_paths)
.chain(src_deps.iter())
.chain(bytecode_deps.iter())
{
for (name, val) in &pack.named_address_map {
if let Some(old) =
global_address_map.insert(name.as_str().to_owned(), *val)
{
if old != *val {
let pack_name = pack
.name
.map(|s| s.as_str().to_owned())
.unwrap_or_else(|| "<unnamed>".to_owned());
bail!(
"found remapped address alias `{}` (`{} != {}`) in package `{}`\
, please use unique address aliases across dependencies",
name, old, val, pack_name
)
}
}
}
}
}
let mut options = move_compiler_v2::Options {
sources: sources_package_paths
.paths
.iter()
.map(|path| path.as_str().to_owned())
.collect(),
sources_deps: src_deps.iter().flat_map(|x| to_str_vec(&x.paths)).collect(),
dependencies: bytecode_deps
.iter()
.flat_map(|x| to_str_vec(&x.paths))
.collect(),
named_address_mapping: global_address_map
.into_iter()
.map(|(k, v)| format!("{}={}", k, v))
.collect(),
skip_attribute_checks,
known_attributes: known_attributes.clone(),
language_version: Some(effective_language_version),
compiler_version: Some(version),
compile_test_code: flags.keep_testing_functions(),
experiments: config.experiments.clone(),
..Default::default()
};
options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
compiler_driver_v2(options)?
},
};
let mut options = move_compiler_v2::Options {
sources: sources_package_paths
.paths
.iter()
.map(|path| path.as_str().to_owned())
.collect(),
sources_deps: src_deps.iter().flat_map(|x| to_str_vec(&x.paths)).collect(),
dependencies: bytecode_deps
.iter()
.flat_map(|x| to_str_vec(&x.paths))
.collect(),
named_address_mapping: global_address_map
.into_iter()
.map(|(k, v)| format!("{}={}", k, v))
.collect(),
skip_attribute_checks,
known_attributes: known_attributes.clone(),
language_version: Some(effective_language_version),
compiler_version: Some(version),
compile_test_code: flags.keep_testing_functions(),
experiments: config.experiments.clone(),
..Default::default()
};
options = options.set_experiment(Experiment::ATTACH_COMPILED_MODULE, true);
compiler_driver_v2(options)?
},
};
Comment on lines +636 to +704
Copy link
Contributor

Choose a reason for hiding this comment

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

@fEst1ck it seems like these are just formatting changes. Can you confirm if this is indeed the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

let mut root_compiled_units = vec![];
let mut deps_compiled_units = vec![];
let obtain_package_name =
Expand Down Expand Up @@ -791,14 +799,24 @@ impl CompiledPackage {
};

let compiled_package = CompiledPackage {
root_compiled_units,
deps_compiled_units,
bytecode_deps: bytecode_deps
.iter()
.flat_map(|package| {
let name = package.name.unwrap();
package.paths.iter().map(move |pkg_path| {
get_addr_from_module_in_package(name, pkg_path.as_str())
.map(|addr| (name, addr))
})
})
.try_collect()?,
compiled_package_info: CompiledPackageInfo {
package_name: resolved_package.source_package.package.name,
address_alias_instantiation: resolved_package.resolution_table,
source_digest: Some(resolved_package.source_digest),
build_flags: resolution_graph.build_options.clone(),
},
root_compiled_units,
deps_compiled_units,
compiled_docs,
compiled_abis,
};
Expand Down Expand Up @@ -885,6 +903,13 @@ impl CompiledPackage {
.collect::<BTreeSet<_>>()
.into_iter()
.collect(),
bytecode_deps: self
.bytecode_deps
.keys()
.copied()
.collect::<BTreeSet<_>>()
.into_iter()
.collect(),
},
};

Expand Down Expand Up @@ -1135,3 +1160,23 @@ pub fn build_and_report_no_exit_v2_driver(
Some(env),
))
}

/// Returns the address of the module
fn get_addr_from_module_in_package(pkg_name: Symbol, pkg_path: &str) -> Result<NumericalAddress> {
// Read the bytecode file
let mut bytecode = Vec::new();
std::fs::File::open(pkg_path)
.context(format!("Failed to open bytecode file for {}", pkg_path))
.and_then(|mut file| {
// read contents of the file into bytecode
std::io::Read::read_to_end(&mut file, &mut bytecode)
.context(format!("Failed to read bytecode file {}", pkg_path))
})
.and_then(|_| {
CompiledModule::deserialize(&bytecode).context(format!(
"Failed to deserialize bytecode file for {}",
pkg_name
))
})
.map(|module| NumericalAddress::from_account_address(*module.self_addr()))
}
Loading