Skip to content

Commit 7dd854d

Browse files
committed
Revert "Add configurables section offset in the preamble (#6522)"
This reverts commit 0555973.
1 parent 66ee4a0 commit 7dd854d

File tree

19 files changed

+160
-422
lines changed

19 files changed

+160
-422
lines changed

forc-pkg/src/pkg.rs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ use sway_core::{
4646
transform::AttributeKind,
4747
write_dwarf, BuildTarget, Engines, FinalizedEntry, LspConfig,
4848
};
49-
use sway_core::{set_bytecode_configurables_offset, PrintAsm, PrintIr};
49+
use sway_core::{PrintAsm, PrintIr};
5050
use sway_error::{error::CompileError, handler::Handler, warning::CompileWarning};
5151
use sway_features::ExperimentalFeatures;
5252
use sway_types::constants::{CORE, PRELUDE, STD};
@@ -1914,7 +1914,7 @@ pub fn compile(
19141914

19151915
let errored = handler.has_errors() || (handler.has_warnings() && profile.error_on_warnings);
19161916

1917-
let mut compiled = match bc_res {
1917+
let compiled = match bc_res {
19181918
Ok(compiled) if !errored => compiled,
19191919
_ => return fail(handler),
19201920
};
@@ -1923,12 +1923,9 @@ pub fn compile(
19231923

19241924
print_warnings(engines.se(), terse_mode, &pkg.name, &warnings, &tree_type);
19251925

1926-
// Metadata to be placed into the binary.
1927-
let mut md = [0u8, 0, 0, 0, 0, 0, 0, 0];
19281926
// TODO: This should probably be in `fuel_abi_json::generate_json_abi_program`?
19291927
// If ABI requires knowing config offsets, they should be inputs to ABI gen.
19301928
if let ProgramABI::Fuel(ref mut program_abi) = program_abi {
1931-
let mut configurables_offset = compiled.bytecode.len() as u64;
19321929
if let Some(ref mut configurables) = program_abi.configurables {
19331930
// Filter out all dead configurables (i.e. ones without offsets in the bytecode)
19341931
configurables.retain(|c| {
@@ -1937,22 +1934,12 @@ pub fn compile(
19371934
.contains_key(&c.name)
19381935
});
19391936
// Set the actual offsets in the JSON object
1940-
for (config, offset) in &compiled.named_data_section_entries_offsets {
1941-
if *offset < configurables_offset {
1942-
configurables_offset = *offset;
1943-
}
1944-
if let Some(idx) = configurables.iter().position(|c| &c.name == config) {
1945-
configurables[idx].offset = *offset;
1937+
for (config, offset) in compiled.named_data_section_entries_offsets {
1938+
if let Some(idx) = configurables.iter().position(|c| c.name == config) {
1939+
configurables[idx].offset = offset;
19461940
}
19471941
}
19481942
}
1949-
1950-
md = configurables_offset.to_be_bytes();
1951-
}
1952-
1953-
// We know to set the metadata only for fuelvm right now.
1954-
if let BuildTarget::Fuel = pkg.target {
1955-
set_bytecode_configurables_offset(&mut compiled, &md);
19561943
}
19571944

19581945
metrics.bytecode_size = compiled.bytecode.len();

forc-plugins/forc-client/test/data/deployed_script/deployed_script-loader-abi.json

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -251,62 +251,62 @@
251251
{
252252
"name": "BOOL",
253253
"concreteTypeId": "b760f44fa5965c2474a3b471467a22c43185152129295af588b022ae50b50903",
254-
"offset": 240
254+
"offset": 136
255255
},
256256
{
257257
"name": "U8",
258258
"concreteTypeId": "c89951a24c6ca28c13fd1cfdc646b2b656d69e61a92b91023be7eb58eb914b6b",
259-
"offset": 352
259+
"offset": 248
260260
},
261261
{
262262
"name": "U16",
263263
"concreteTypeId": "29881aad8730c5ab11d275376323d8e4ff4179aae8ccb6c13fe4902137e162ef",
264-
"offset": 296
264+
"offset": 192
265265
},
266266
{
267267
"name": "U32",
268268
"concreteTypeId": "d7649d428b9ff33d188ecbf38a7e4d8fd167fa01b2e10fe9a8f9308e52f1d7cc",
269-
"offset": 336
269+
"offset": 232
270270
},
271271
{
272272
"name": "U64",
273273
"concreteTypeId": "1506e6f44c1d6291cdf46395a8e573276a4fa79e8ace3fc891e092ef32d1b0a0",
274-
"offset": 344
274+
"offset": 240
275275
},
276276
{
277277
"name": "U256",
278278
"concreteTypeId": "1b5759d94094368cfd443019e7ca5ec4074300e544e5ea993a979f5da627261e",
279-
"offset": 304
279+
"offset": 200
280280
},
281281
{
282282
"name": "B256",
283283
"concreteTypeId": "7c5ee1cecf5f8eacd1284feb5f0bf2bdea533a51e2f0c9aabe9236d335989f3b",
284-
"offset": 208
284+
"offset": 104
285285
},
286286
{
287287
"name": "STR_4",
288288
"concreteTypeId": "94f0fa95c830be5e4f711963e83259fe7e8bc723278ab6ec34449e791a99b53a",
289-
"offset": 280
289+
"offset": 176
290290
},
291291
{
292292
"name": "TUPLE",
293293
"concreteTypeId": "e0128f7be9902d1fe16326cafe703b52038064a7997b03ebfc1c9dd607e1536c",
294-
"offset": 288
294+
"offset": 184
295295
},
296296
{
297297
"name": "ARRAY",
298298
"concreteTypeId": "d9fac01ab38fe10950758ae9604da330d6406a71fda3ef1ea818121261132d56",
299-
"offset": 192
299+
"offset": 88
300300
},
301301
{
302302
"name": "STRUCT",
303303
"concreteTypeId": "563310524b4f4447a10d0e50556310253dfb3b5eb4b29c3773222b737c8b7075",
304-
"offset": 264
304+
"offset": 160
305305
},
306306
{
307307
"name": "ENUM",
308308
"concreteTypeId": "37cd1cba311039a851ac8bfa614cc41359b4ad95c8656fcef2e8f504fe7a1272",
309-
"offset": 248
309+
"offset": 144
310310
}
311311
]
312312
}

forc-test/src/execute.rs

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use fuel_vm::{
99
self as vm,
1010
checked_transaction::builder::TransactionBuilderExt,
1111
interpreter::{Interpreter, NotSupportedEcal},
12-
prelude::SecretKey,
12+
prelude::{Instruction, SecretKey},
1313
storage::MemoryStorage,
1414
};
1515
use rand::{Rng, SeedableRng};
@@ -246,21 +246,18 @@ impl TestExecutor {
246246
/// The following is how the beginning of the bytecode is laid out:
247247
///
248248
/// ```ignore
249-
/// [ 0] ji i(4 + 2) ; Jumps to the data section setup.
250-
/// [ 1] noop
251-
/// [ 2] DATA_SECTION_OFFSET[0..32]
252-
/// [ 3] DATA_SECTION_OFFSET[32..64]
253-
/// [ 4] CONFIGURABLES_OFFSET[0..32]
254-
/// [ 5] CONFIGURABLES_OFFSET[32..64]
255-
/// [ 6] lw $ds $is 1 ; The data section setup, i.e. where the first ji lands.
256-
/// [ 7] add $$ds $$ds $is
257-
/// [ 8] <first-entry-point> ; This is where we want to jump from to our test code!
249+
/// [0] ji i4 ; Jumps to the data section setup.
250+
/// [1] noop
251+
/// [2] DATA_SECTION_OFFSET[0..32]
252+
/// [3] DATA_SECTION_OFFSET[32..64]
253+
/// [4] lw $ds $is 1 ; The data section setup, i.e. where the first ji lands.
254+
/// [5] add $$ds $$ds $is
255+
/// [6] <first-entry-point> ; This is where we want to jump from to our test code!
258256
/// ```
259257
fn patch_test_bytecode(bytecode: &[u8], test_offset: u32) -> std::borrow::Cow<[u8]> {
260-
// Each instruction is 4 bytes,
261-
// so we divide the total byte-size by 4 to get the instruction offset.
262-
const PROGRAM_START_INST_OFFSET: u32 = (sway_core::PRELUDE_SIZE_IN_BYTES / 4) as u32;
263-
const PROGRAM_START_BYTE_OFFSET: usize = sway_core::PRELUDE_SIZE_IN_BYTES;
258+
// TODO: Standardize this or add metadata to bytecode.
259+
const PROGRAM_START_INST_OFFSET: u32 = 6;
260+
const PROGRAM_START_BYTE_OFFSET: usize = PROGRAM_START_INST_OFFSET as usize * Instruction::SIZE;
264261

265262
// If our desired entry point is the program start, no need to jump.
266263
if test_offset == PROGRAM_START_INST_OFFSET {

forc/src/cli/commands/parse_bytecode.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,6 @@ pub(crate) fn exec(command: Command) -> ForcResult<()> {
5959
parsed_raw
6060
)
6161
}
62-
Err(fuel_asm::InvalidOpcode) if word_ix == 4 || word_ix == 5 => {
63-
let parsed_raw = u32::from_be_bytes([raw[0], raw[1], raw[2], raw[3]]);
64-
format!(
65-
"configurables offset {} ({})",
66-
if word_ix == 4 { "lo" } else { "hi" },
67-
parsed_raw
68-
)
69-
}
7062
Ok(_) | Err(fuel_asm::InvalidOpcode) => "".into(),
7163
};
7264
table.add_row(Row::new(vec![

sway-core/src/asm_generation/finalized_asm.rs

Lines changed: 12 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use super::{
33
fuel::{checks, data_section::DataSection},
44
ProgramABI, ProgramKind,
55
};
6-
use crate::asm_generation::fuel::data_section::{Datum, Entry, EntryName};
7-
use crate::asm_lang::allocated_ops::{AllocatedOp, AllocatedOpcode, FuelAsmData};
6+
use crate::asm_generation::fuel::data_section::{DataId, Datum, Entry};
7+
use crate::asm_lang::allocated_ops::{AllocatedOp, AllocatedOpcode};
88
use crate::decl_engine::DeclRefFunction;
99
use crate::source_map::SourceMap;
1010
use crate::BuildConfig;
@@ -16,6 +16,7 @@ use sway_error::handler::{ErrorEmitted, Handler};
1616
use sway_types::span::Span;
1717
use sway_types::SourceEngine;
1818

19+
use either::Either;
1920
use std::{collections::BTreeMap, fmt};
2021

2122
/// Represents an ASM set which has had register allocation, jump elimination, and optimization
@@ -111,7 +112,6 @@ fn to_bytecode_mut(
111112
{
112113
8
113114
}
114-
AllocatedOpcode::ConfigurablesOffsetPlaceholder => 8,
115115
AllocatedOpcode::DataSectionOffsetPlaceholder => 8,
116116
AllocatedOpcode::BLOB(count) => count.value as u64 * 4,
117117
AllocatedOpcode::CFEI(i) | AllocatedOpcode::CFSI(i) if i.value == 0 => 0,
@@ -141,29 +141,6 @@ fn to_bytecode_mut(
141141
&ops_padded
142142
};
143143

144-
let mut offset_from_instr_start = 0;
145-
for op in ops.iter() {
146-
match &op.opcode {
147-
AllocatedOpcode::LoadDataId(_reg, data_label)
148-
if !data_section
149-
.has_copy_type(data_label)
150-
.expect("data label references non existent data -- internal error") =>
151-
{
152-
// For non-copy type loads, pre-insert pointers into the data_section so that
153-
// from this point on, the data_section remains immutable. This is necessary
154-
// so that when we take addresses of configurables, that address doesn't change
155-
// later on if a non-configurable is added to the data-section.
156-
let offset_bytes = data_section.data_id_to_offset(data_label) as u64;
157-
// The -4 is because $pc is added in the *next* instruction.
158-
let pointer_offset_from_current_instr =
159-
offset_to_data_section_in_bytes - offset_from_instr_start + offset_bytes - 4;
160-
data_section.append_pointer(pointer_offset_from_current_instr);
161-
}
162-
_ => (),
163-
}
164-
offset_from_instr_start += op_size_in_bytes(data_section, op);
165-
}
166-
167144
let mut bytecode = Vec::with_capacity(offset_to_data_section_in_bytes as usize);
168145

169146
if build_config.print_bytecode {
@@ -189,7 +166,7 @@ fn to_bytecode_mut(
189166
offset_from_instr_start += op_size_in_bytes(data_section, op);
190167

191168
match fuel_op {
192-
FuelAsmData::DatasectionOffset(data) => {
169+
Either::Right(data) => {
193170
if build_config.print_bytecode {
194171
print!("{}{:#010x} ", " ".repeat(indentation), bytecode.len());
195172
println!(
@@ -205,23 +182,7 @@ fn to_bytecode_mut(
205182
bytecode.extend(data.iter().cloned());
206183
half_word_ix += 2;
207184
}
208-
FuelAsmData::ConfigurablesOffset(data) => {
209-
if build_config.print_bytecode {
210-
print!("{}{:#010x} ", " ".repeat(indentation), bytecode.len());
211-
println!(
212-
" ;; {:?}",
213-
data
214-
);
215-
}
216-
217-
// Static assert to ensure that we're only dealing with ConfigurablesOffsetPlaceholder,
218-
// a 1-word (8 bytes) data within the code. No other uses are known.
219-
let _: [u8; 8] = data;
220-
221-
bytecode.extend(data.iter().cloned());
222-
half_word_ix += 2;
223-
}
224-
FuelAsmData::Instructions(instructions) => {
185+
Either::Left(instructions) => {
225186
for instruction in instructions {
226187
// Print original source span only once
227188
if build_config.print_bytecode_spans {
@@ -334,9 +295,9 @@ fn to_bytecode_mut(
334295
};
335296
}
336297

337-
for (i, entry) in data_section.iter_all_entries().enumerate() {
338-
let entry_offset = data_section.absolute_idx_to_offset(i);
339-
print_entry(indentation, offset + entry_offset, &entry);
298+
for (i, entry) in data_section.value_pairs.iter().enumerate() {
299+
let entry_offset = data_section.data_id_to_offset(&DataId(i as u32));
300+
print_entry(indentation, offset + entry_offset, entry);
340301
}
341302

342303
println!(";; --- END OF TARGET BYTECODE ---\n");
@@ -345,19 +306,16 @@ fn to_bytecode_mut(
345306
assert_eq!(half_word_ix * 4, offset_to_data_section_in_bytes as usize);
346307
assert_eq!(bytecode.len(), offset_to_data_section_in_bytes as usize);
347308

348-
let num_nonconfigurables = data_section.non_configurables.len();
349309
let named_data_section_entries_offsets = data_section
350-
.configurables
310+
.value_pairs
351311
.iter()
352312
.enumerate()
313+
.filter(|entry| entry.1.name.is_some())
353314
.map(|(id, entry)| {
354-
let EntryName::Configurable(name) = &entry.name else {
355-
panic!("Non-configurable in configurables part of datasection");
356-
};
357315
(
358-
name.clone(),
316+
entry.name.as_ref().unwrap().clone(),
359317
offset_to_data_section_in_bytes
360-
+ data_section.absolute_idx_to_offset(id + num_nonconfigurables) as u64,
318+
+ data_section.raw_data_id_to_offset(id as u32) as u64,
361319
)
362320
})
363321
.collect::<BTreeMap<String, u64>>();

sway-core/src/asm_generation/fuel/allocated_abstract_instruction_set.rs

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
use crate::{
2-
asm_generation::fuel::data_section::EntryName,
3-
asm_lang::{
4-
allocated_ops::{AllocatedOpcode, AllocatedRegister},
5-
AllocatedAbstractOp, ConstantRegister, ControlFlowOp, Label, RealizedOp,
6-
VirtualImmediate12, VirtualImmediate18, VirtualImmediate24,
7-
},
1+
use crate::asm_lang::{
2+
allocated_ops::{AllocatedOpcode, AllocatedRegister},
3+
AllocatedAbstractOp, ConstantRegister, ControlFlowOp, Label, RealizedOp, VirtualImmediate12,
4+
VirtualImmediate18, VirtualImmediate24,
85
};
96

107
use super::{
@@ -351,13 +348,6 @@ impl AllocatedAbstractInstructionSet {
351348
comment: String::new(),
352349
});
353350
}
354-
ControlFlowOp::ConfigurablesOffsetPlaceholder => {
355-
realized_ops.push(RealizedOp {
356-
opcode: AllocatedOpcode::ConfigurablesOffsetPlaceholder,
357-
owning_span: None,
358-
comment: String::new(),
359-
});
360-
}
361351
ControlFlowOp::LoadLabel(r1, ref lab) => {
362352
// LoadLabel ops are inserted by `rewrite_far_jumps`.
363353
// So the next instruction must be a relative jump.
@@ -373,11 +363,8 @@ impl AllocatedAbstractInstructionSet {
373363
// We compute the relative offset w.r.t the actual jump.
374364
// Sub 1 because the relative jumps add a 1.
375365
let offset = rel_offset(curr_offset + 1, lab) - 1;
376-
let data_id = data_section.insert_data_value(Entry::new_word(
377-
offset,
378-
EntryName::NonConfigurable,
379-
None,
380-
));
366+
let data_id =
367+
data_section.insert_data_value(Entry::new_word(offset, None, None));
381368
realized_ops.push(RealizedOp {
382369
opcode: AllocatedOpcode::LoadDataId(r1, data_id),
383370
owning_span,
@@ -486,8 +473,6 @@ impl AllocatedAbstractInstructionSet {
486473
2
487474
}
488475

489-
Either::Right(ConfigurablesOffsetPlaceholder) => 2,
490-
491476
Either::Right(PushAll(_)) | Either::Right(PopAll(_)) => unreachable!(
492477
"fix me, pushall and popall don't really belong in control flow ops \
493478
since they're not about control flow"

0 commit comments

Comments
 (0)