Skip to content

Feature - Adjustments of SIMD-0189 #92

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+80 −86
Open

Feature - Adjustments of SIMD-0189 #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e1f83b0
Swaps MM_BYTECODE_START and MM_RODATA_START.
Lichtso Aug 27, 2025
8e313a0
Adjusts the e_type, e_machine and program headers.
Lichtso Nov 5, 2025
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
6 changes: 3 additions & 3 deletions src/debugger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ fn get_host_ptr<C: ContextObject>(
if !interpreter
.executable
.get_sbpf_version()
.enable_lower_bytecode_vaddr()
&& vm_addr < ebpf::MM_RODATA_START
.enable_lower_rodata_vaddr()
&& vm_addr < ebpf::MM_BYTECODE_START
{
vm_addr += ebpf::MM_RODATA_START;
vm_addr += ebpf::MM_BYTECODE_START;
}
match interpreter.vm.memory_mapping.map(
AccessType::Load,
Expand Down
8 changes: 4 additions & 4 deletions src/ebpf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ pub const VIRTUAL_ADDRESS_BITS: usize = 32;

/// Size (and alignment) of a memory region
pub const MM_REGION_SIZE: u64 = 1 << VIRTUAL_ADDRESS_BITS;
/// Virtual address of the bytecode region (in SBPFv3)
pub const MM_BYTECODE_START: u64 = 0;
/// Virtual address of the readonly data region (also contains the bytecode until SBPFv3)
pub const MM_RODATA_START: u64 = MM_REGION_SIZE;
/// Virtual address of the readonly data region (in SBPFv3)
pub const MM_RODATA_START: u64 = 0;
/// Virtual address of the bytecode region (also contains the rodata until SBPFv3)
pub const MM_BYTECODE_START: u64 = MM_REGION_SIZE;
/// Virtual address of the stack region
pub const MM_STACK_START: u64 = MM_REGION_SIZE * 2;
/// Virtual address of the heap region
Expand Down
52 changes: 23 additions & 29 deletions src/elf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,15 @@ impl<C: ContextObject> Executable<C> {
Ok(Self {
elf_bytes,
sbpf_version,
ro_section: Section::Borrowed(ebpf::MM_RODATA_START as usize, 0..text_bytes.len()),
text_section_vaddr: if sbpf_version.enable_lower_bytecode_vaddr() {
ebpf::MM_BYTECODE_START
} else {
ebpf::MM_RODATA_START
},
ro_section: Section::Borrowed(
if sbpf_version.enable_lower_rodata_vaddr() {
ebpf::MM_RODATA_START
} else {
ebpf::MM_BYTECODE_START
} as usize,
0..text_bytes.len(),
),
text_section_vaddr: ebpf::MM_BYTECODE_START,
text_section_range: 0..text_bytes.len(),
entry_pc,
function_registry,
Expand Down Expand Up @@ -407,7 +410,7 @@ impl<C: ContextObject> Executable<C> {
loader: Arc<BuiltinProgram<C>>,
) -> Result<Self, ElfParserError> {
use crate::elf_parser::{
consts::{ELFMAG, EV_CURRENT, PF_R, PF_W, PF_X, PT_LOAD, SHN_UNDEF, STT_FUNC},
consts::{ELFMAG, EV_CURRENT, PF_R, PF_X, PT_LOAD, SHN_UNDEF, STT_FUNC},
types::{Elf64Ehdr, Elf64Shdr, Elf64Sym},
};

Expand All @@ -426,8 +429,8 @@ impl<C: ContextObject> Executable<C> {
|| file_header.e_ident.ei_osabi != ELFOSABI_NONE
|| file_header.e_ident.ei_abiversion != 0x00
|| file_header.e_ident.ei_pad != [0x00; 7]
|| file_header.e_type != ET_DYN
|| file_header.e_machine != EM_SBPF
// file_header.e_type
|| file_header.e_machine != EM_BPF
|| file_header.e_version != EV_CURRENT
// file_header.e_entry
|| file_header.e_phoff != mem::size_of::<Elf64Ehdr>() as u64
Expand All @@ -436,50 +439,45 @@ impl<C: ContextObject> Executable<C> {
|| file_header.e_ehsize != mem::size_of::<Elf64Ehdr>() as u16
|| file_header.e_phentsize != mem::size_of::<Elf64Phdr>() as u16
|| file_header.e_phnum < EXPECTED_PROGRAM_HEADERS.len() as u16
|| program_header_table_range.end >= elf_bytes.len()
|| program_header_table_range.end > elf_bytes.len()
|| file_header.e_shentsize != mem::size_of::<Elf64Shdr>() as u16
// file_header.e_shnum
|| file_header.e_shstrndx >= file_header.e_shnum
{
return Err(ElfParserError::InvalidFileHeader);
}

const EXPECTED_PROGRAM_HEADERS: [(u32, u64); 4] = [
(PF_X, ebpf::MM_BYTECODE_START), // byte code
(PF_R, ebpf::MM_RODATA_START), // read only data
(PF_R | PF_W, ebpf::MM_STACK_START), // stack
(PF_R | PF_W, ebpf::MM_HEAP_START), // heap
const EXPECTED_PROGRAM_HEADERS: [(u32, u64); 2] = [
(PF_R, ebpf::MM_RODATA_START), // read only data
(PF_X, ebpf::MM_BYTECODE_START), // byte code
];
let program_header_table =
Elf64::slice_from_bytes::<Elf64Phdr>(elf_bytes, program_header_table_range.clone())?;
let mut expected_offset = program_header_table_range.end as u64;
for (program_header, (p_flags, p_vaddr)) in program_header_table
.iter()
.zip(EXPECTED_PROGRAM_HEADERS.iter())
{
let p_filesz = if (*p_flags & PF_W) != 0 {
0
} else {
program_header.p_memsz
};
if program_header.p_type != PT_LOAD
|| program_header.p_flags != *p_flags
|| program_header.p_offset < program_header_table_range.end as u64
|| program_header.p_offset != expected_offset
Copy link
Collaborator

@LucasSte LucasSte Nov 26, 2025

Choose a reason for hiding this comment

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

This restriction is not guaranteed to work. I tested it here and LLD does not output it automatically. In one of the cases, I could only get it once I called llvm-objcopy --strip-all. I suggest reverting it to the original restriction.

Copy link
Collaborator

@LucasSte LucasSte Nov 26, 2025

Choose a reason for hiding this comment

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

llvm-objcopy --strip-all does not work with enable_symbol_and_section_labels set to true.

|| program_header.p_offset >= elf_bytes.len() as u64
|| program_header.p_offset.checked_rem(ebpf::INSN_SIZE as u64) != Some(0)
|| program_header.p_vaddr != *p_vaddr
|| program_header.p_paddr != *p_vaddr
|| program_header.p_filesz != p_filesz
|| program_header.p_filesz != program_header.p_memsz
|| program_header.p_filesz
> (elf_bytes.len() as u64).saturating_sub(program_header.p_offset)
|| program_header.p_filesz.checked_rem(ebpf::INSN_SIZE as u64) != Some(0)
|| program_header.p_memsz >= ebpf::MM_REGION_SIZE
{
return Err(ElfParserError::InvalidProgramHeader);
}
expected_offset = expected_offset.saturating_add(program_header.p_filesz);
}

let bytecode_header = &program_header_table[0];
let rodata_header = &program_header_table[1];
let rodata_header = &program_header_table[0];
let bytecode_header = &program_header_table[1];
let text_section_vaddr = bytecode_header.p_vaddr;
let text_section_range = bytecode_header.file_range().unwrap_or_default();
let ro_section = Section::Borrowed(
Expand All @@ -501,10 +499,6 @@ impl<C: ContextObject> Executable<C> {
.saturating_sub(bytecode_header.p_vaddr)
.checked_div(ebpf::INSN_SIZE as u64)
.unwrap_or_default() as usize;
let entry_insn = ebpf::get_insn(&elf_bytes[text_section_range.clone()], entry_pc);
if !entry_insn.is_function_start_marker() {
return Err(ElfParserError::InvalidFileHeader);
}

let mut function_registry = FunctionRegistry::<usize>::default();
let config = loader.get_config();
Expand Down Expand Up @@ -658,7 +652,7 @@ impl<C: ContextObject> Executable<C> {
return Err(ElfError::ValueOutOfBounds);
}
} else {
debug_assert_eq!(ro_section_vaddr, ebpf::MM_RODATA_START);
debug_assert_eq!(ro_section_vaddr, ebpf::MM_REGION_SIZE);
}

Ok(Self {
Expand Down
4 changes: 2 additions & 2 deletions src/memory_region.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@ impl CommonMemoryMapping<'_> {
stack_frame,
))
} else {
let region_name = match vm_addr & (!ebpf::MM_RODATA_START.saturating_sub(1)) {
ebpf::MM_RODATA_START => "program",
let region_name = match vm_addr & (!ebpf::MM_BYTECODE_START.saturating_sub(1)) {
ebpf::MM_BYTECODE_START => "program",
Comment on lines +207 to +208
Copy link

@buffalojoec buffalojoec Nov 25, 2025

Choose a reason for hiding this comment

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

Should we include another match arm in here for ebpf::MM_RODATA_START?

We could treat both as "program", like we do in < SBPF v3, or we could have "bytecode" and "rodata" messages. Seems probably useful to distinguish between the two, right?

Copy link
Collaborator

@LucasSte LucasSte Nov 25, 2025

Choose a reason for hiding this comment

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

https://github.com/anza-xyz/sbpf/pull/45/files

Copy link
Collaborator

@LucasSte LucasSte Nov 25, 2025

Choose a reason for hiding this comment

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

We didn't merge that PR yet, because we have not yet finalized the ABIv2 design.

Copy link

@buffalojoec buffalojoec Nov 25, 2025

Choose a reason for hiding this comment

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

Ah okay, thanks. Does it hurt to add the "rodata" arm here though?

ebpf::MM_STACK_START => "stack",
ebpf::MM_HEAP_START => "heap",
ebpf::MM_INPUT_START => "input",
Expand Down
2 changes: 1 addition & 1 deletion src/program.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl SBPFVersion {
self >= SBPFVersion::V3
}
/// ... SIMD-0189
pub fn enable_lower_bytecode_vaddr(self) -> bool {
pub fn enable_lower_rodata_vaddr(self) -> bool {
self >= SBPFVersion::V3
}
/// ... SIMD-0377
Expand Down
30 changes: 8 additions & 22 deletions tests/elf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ fn test_strict_header() {
assert_eq!(err, ElfParserError::OutOfBounds);

// Break the file header one byte at a time
let expected_results = std::iter::repeat_n(&Err(ElfParserError::InvalidFileHeader), 40)
let expected_results = std::iter::repeat_n(&Err(ElfParserError::InvalidFileHeader), 16)
.chain(std::iter::repeat_n(&Ok(()), 2))
.chain(std::iter::repeat_n(
&Err(ElfParserError::InvalidFileHeader),
22,
))
.chain(std::iter::repeat_n(&Ok(()), 12))
.chain(std::iter::repeat_n(
&Err(ElfParserError::InvalidFileHeader),
Expand All @@ -81,20 +86,9 @@ fn test_strict_header() {
std::iter::repeat_n(&Err(ElfParserError::InvalidProgramHeader), 48)
.chain(std::iter::repeat_n(&Ok(()), 8))
.collect::<Vec<_>>();
let expected_results_writable =
std::iter::repeat_n(&Err(ElfParserError::InvalidProgramHeader), 40)
.chain(std::iter::repeat_n(&Ok(()), 4))
.chain(std::iter::repeat_n(
&Err(ElfParserError::InvalidProgramHeader),
4,
))
.chain(std::iter::repeat_n(&Ok(()), 8))
.collect::<Vec<_>>();
let expected_results = vec![
expected_results_readonly.iter(),
expected_results_readonly.iter(),
expected_results_writable.iter(),
expected_results_writable.iter(),
];
for (header_index, expected_results) in expected_results.into_iter().enumerate() {
for (offset, expected) in (std::mem::size_of::<Elf64Ehdr>()
Expand All @@ -114,19 +108,11 @@ fn test_strict_header() {
// Check that an unaligned program header length fails
{
let mut elf_bytes = elf_bytes.clone();
elf_bytes[0x60] = 0x29;
elf_bytes[0x68] = 0x29;
elf_bytes[0x98] = 0x29;
elf_bytes[0xA0] = 0x29;
let err = ElfExecutable::load_with_strict_parser(&elf_bytes, loader.clone()).unwrap_err();
assert_eq!(err, ElfParserError::InvalidProgramHeader);
}

// Check that an entrypoint missing a function start marker fails
{
let mut elf_bytes = elf_bytes.clone();
elf_bytes[0x1B8] = 0x00;
let err = ElfExecutable::load_with_strict_parser(&elf_bytes, loader.clone()).unwrap_err();
assert_eq!(err, ElfParserError::InvalidFileHeader);
}
}

#[test]
Expand Down
10 changes: 5 additions & 5 deletions tests/elfs/elf.ld
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
SECTIONS
{
.text 0x000000000 : {
*(.text*)
} :text
.rodata 0x100000000 : {
.rodata 0x000000000 : {
*(.rodata*)
*(.data.rel.ro*)
BYTE(0);
. = ALIGN(8);
} :rodata
.text 0x100000000 : {
*(.text*)
Copy link
Collaborator

@LucasSte LucasSte Nov 6, 2025

Choose a reason for hiding this comment

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

No need to do it here, but if you want, you can remove the .bss.stack and .bss.heap from the linker script.

} :text
.bss.stack 0x200000000 (NOLOAD) : {
_stack_start = .;
. = . + 0x1000;
Expand Down Expand Up @@ -37,8 +37,8 @@ SECTIONS

PHDRS
{
text PT_LOAD FLAGS(1);
rodata PT_LOAD FLAGS(4);
text PT_LOAD FLAGS(1);
stack PT_LOAD FLAGS(6);
heap PT_LOAD FLAGS(6);
dynsym PT_NULL FLAGS(0);
Expand Down
Binary file modified tests/elfs/relative_call.so
100755 → 100644
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/reloc_64_64.so
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/reloc_64_relative.so
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/reloc_64_relative_data.so
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/rodata_section.so
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/strict_header.so
100755 → 100644
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/struct_func_pointer.so
View file
Open in desktop
Binary file not shown.
Binary file modified tests/elfs/syscall_static.so
View file
Open in desktop
Binary file not shown.
Loading