public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marvin Häuser" <mhaeuser@posteo.de>
To: devel@edk2.groups.io, ayushdevel1325@gmail.com
Cc: jiewen.yao@intel.com
Subject: Re: [edk2-devel] [Patch] Fix edkii-rust brach in edk2-staging
Date: Thu, 17 Mar 2022 19:33:41 +0000	[thread overview]
Message-ID: <CB1B6A6D-D981-49B5-8FC7-93BAA6510E8F@posteo.de> (raw)
In-Reply-To: <q7x2.1647528854451677985.Py85@groups.io>

[-- Attachment #1: Type: text/plain, Size: 18987 bytes --]

CC Jiewen

(needs whitespace changes cleaned and the bench needs to be rebased beyond 2019 too)

Also see: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

Best regards,
Marvin

> On 17. Mar 2022, at 15:54, ayushdevel1325@gmail.com wrote:
> 
> From: Ayush Singh <ayushdevel1325@gmail.com>
> 
> Hello everyone,
> 
> I am Ayush Singh, an applicant for for GSoC 2022. My Introduction can be found here.
> 
> To get myself acquainted with the project, I was trying to build the Rust tests in edkii-rust branch in edk2-staging and found out that due to changes in the Rust Allocator API, Test/TestRustLangLib and Test/RustLangApp fail to build. Also, cargo now supports cross-compiling sysroot (build-std feature) so there is no need for cargo-xbuild.
> 
> I have tried to fix Test/TestRustLangLib and would like someone to review if it is correct. Here is the diff:
> 
> diff --git a/RustPkg/Library/UefiRustAllocationLib/src/lib.rs b/RustPkg/Library/UefiRustAllocationLib/src/lib.rs
> index f369e1bb17..6a65f0a5f9 100644
> --- a/RustPkg/Library/UefiRustAllocationLib/src/lib.rs
> +++ b/RustPkg/Library/UefiRustAllocationLib/src/lib.rs
> @@ -15,44 +15,42 @@
>  #![feature(alloc_layout_extra)] 
>  #![feature(allocator_api)] 
>  #![feature(alloc_error_handler)] 
> - 
>  #![cfg_attr(not(test), no_std)] 
> - 
>  #![allow(unused)] 
>   
>  extern crate uefi_rust_panic_lib; 
>   
> -use core::alloc::{GlobalAlloc, Layout, Alloc}; 
> -use r_efi::efi; 
> -use r_efi::efi::{Status}; 
> +use core::alloc::{GlobalAlloc, Layout}; 
>  use core::ffi::c_void; 
> +use r_efi::efi; 
> +use r_efi::efi::Status; 
>   
>  pub struct MyAllocator; 
>   
> -static mut ST : *mut efi::SystemTable = core::ptr::null_mut(); 
> -static mut BS : *mut efi::BootServices = core::ptr::null_mut(); 
> +static mut ST: *mut efi::SystemTable = core::ptr::null_mut(); 
> +static mut BS: *mut efi::BootServices = core::ptr::null_mut(); 
>   
>  unsafe impl GlobalAlloc for MyAllocator { 
>      unsafe fn alloc(&self, layout: Layout) -> *mut u8 { 
> -      let size = layout.size(); 
> -      let align = layout.align(); 
> -      if align > 8 { 
> -        return core::ptr::null_mut(); 
> -      } 
> +        let size = layout.size(); 
> +        let align = layout.align(); 
> +        if align > 8 { 
> +            return core::ptr::null_mut(); 
> +        } 
>   
> -      let mut address : *mut c_void = core::ptr::null_mut(); 
> -      let status = ((*BS).allocate_pool) ( 
> -                     efi::MemoryType::BootServicesData, 
> -                     size, 
> -                     &mut address as *mut *mut c_void 
> -                     ); 
> -      if status != Status::SUCCESS { 
> -        return core::ptr::null_mut(); 
> -      } 
> -      address as *mut u8 
> +        let mut address: *mut c_void = core::ptr::null_mut(); 
> +        let status = ((*BS).allocate_pool)( 
> +            efi::MemoryType::BootServicesData, 
> +            size, 
> +            &mut address as *mut *mut c_void, 
> +        ); 
> +        if status != Status::SUCCESS { 
> +            return core::ptr::null_mut(); 
> +        } 
> +        address as *mut u8 
>      } 
>      unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) { 
> -      ((*BS).free_pool) (ptr as *mut c_void); 
> +        ((*BS).free_pool)(ptr as *mut c_void); 
>      } 
>  } 
>   
> @@ -60,14 +58,13 @@ unsafe impl GlobalAlloc for MyAllocator {
>  static ALLOCATOR: MyAllocator = MyAllocator; 
>   
>  #[alloc_error_handler] 
> -fn alloc_error_handler(layout: core::alloc::Layout) -> ! 
> -{ 
> +fn alloc_error_handler(layout: core::alloc::Layout) -> ! { 
>      loop {} 
>  } 
>   
> -pub extern fn init(system_table: *mut efi::SystemTable) { 
> +pub extern "C" fn init(system_table: *mut efi::SystemTable) { 
>      unsafe { 
> -      ST = system_table; 
> -      BS = (*ST).boot_services; 
> +        ST = system_table; 
> +        BS = (*ST).boot_services; 
>      } 
>  } 
> diff --git a/RustPkg/Test/HelloWorld/.cargo/config.toml b/RustPkg/Test/HelloWorld/.cargo/config.toml
> new file mode 100644
> index 0000000000..3d6a3ff35c
> --- /dev/null
> +++ b/RustPkg/Test/HelloWorld/.cargo/config.toml
> @@ -0,0 +1,3 @@
> +[unstable]
> +build-std = ["core", "compiler_builtins"]
> +build-std-features = ["compiler-builtins-mem"]
> diff --git a/RustPkg/Test/HelloWorld2/.cargo/config.toml b/RustPkg/Test/HelloWorld2/.cargo/config.toml
> new file mode 100644
> index 0000000000..3d6a3ff35c
> --- /dev/null
> +++ b/RustPkg/Test/HelloWorld2/.cargo/config.toml
> @@ -0,0 +1,3 @@
> +[unstable]
> +build-std = ["core", "compiler_builtins"]
> +build-std-features = ["compiler-builtins-mem"]
> diff --git a/RustPkg/Test/TestRustLangLib/.cargo/config.toml b/RustPkg/Test/TestRustLangLib/.cargo/config.toml
> new file mode 100644
> index 0000000000..422bf9d2ab
> --- /dev/null
> +++ b/RustPkg/Test/TestRustLangLib/.cargo/config.toml
> @@ -0,0 +1,3 @@
> +[unstable]
> +build-std = ["core", "compiler_builtins", "alloc"]
> +build-std-features = ["compiler-builtins-mem"]
> diff --git a/RustPkg/Test/TestRustLangLib/src/lib.rs b/RustPkg/Test/TestRustLangLib/src/lib.rs
> index ee3a0d7cc8..888733232b 100644
> --- a/RustPkg/Test/TestRustLangLib/src/lib.rs
> +++ b/RustPkg/Test/TestRustLangLib/src/lib.rs
> @@ -14,24 +14,22 @@
>   
>  #![feature(alloc_layout_extra)] 
>  #![feature(allocator_api)] 
> -#![feature(core_panic_info)] 
> - 
> +#![feature(slice_ptr_get)] 
>  #![cfg_attr(not(test), no_std)] 
> - 
>  #![allow(unused)] 
>   
>  mod mem; 
>   
>  use r_efi::efi; 
> -use r_efi::efi::{Status}; 
> +use r_efi::efi::Status; 
>   
> -extern { 
> -  fn AllocatePool (Size: usize) -> *mut c_void; 
> -  fn FreePool (Buffer: *mut c_void); 
> +extern "C" { 
> +    fn AllocatePool(Size: usize) -> *mut c_void; 
> +    fn FreePool(Buffer: *mut c_void); 
>  } 
>   
> -use core::panic::PanicInfo; 
>  use core::ffi::c_void; 
> +use core::panic::PanicInfo; 
>   
>  use core::mem::size_of; 
>  use core::mem::transmute; 
> @@ -40,30 +38,22 @@ use core::slice;
>  use core::slice::from_raw_parts; 
>  use core::slice::from_raw_parts_mut; 
>   
> -extern crate uefi_rust_panic_lib; 
>  extern crate uefi_rust_allocation_lib; 
> +extern crate uefi_rust_panic_lib; 
>   
>  extern crate alloc; 
>   
> -use alloc::vec::Vec; 
> +use alloc::alloc::{handle_alloc_error, Allocator, Global, Layout}; 
>  use alloc::boxed::Box; 
> -use alloc::{ 
> -    alloc::{handle_alloc_error, Alloc, Global, Layout}, 
> -}; 
> - 
> +use alloc::vec::Vec; 
>   
>  #[no_mangle] 
>  #[export_name = "TestIntegerOverflow"] 
> -pub extern fn test_integer_overflow ( 
> -    buffer_size: usize, 
> -    width : u32, 
> -    height : u32, 
> -    ) -> Status 
> -{ 
> +pub extern "C" fn test_integer_overflow(buffer_size: usize, width: u32, height: u32) -> Status { 
>      let data_size = width * height * 4; 
>   
>      if data_size as usize > buffer_size { 
> -      return Status::UNSUPPORTED; 
> +        return Status::UNSUPPORTED; 
>      } 
>   
>      Status::SUCCESS 
> @@ -71,26 +61,24 @@ pub extern fn test_integer_overflow (
>   
>  #[no_mangle] 
>  #[export_name = "TestIntegerCheckedOverflow"] 
> -pub extern fn test_integer_checked_overflow ( 
> +pub extern "C" fn test_integer_checked_overflow( 
>      buffer_size: usize, 
> -    width : u32, 
> -    height : u32, 
> -    ) -> Status 
> -{ 
> - 
> +    width: u32, 
> +    height: u32, 
> +) -> Status { 
>      let mut data_size: u32 = 0; 
>   
>      match width.checked_mul(height) { 
> -      Some(size) => {data_size = size}, 
> -      None => {return Status::INVALID_PARAMETER}, 
> +        Some(size) => data_size = size, 
> +        None => return Status::INVALID_PARAMETER, 
>      } 
>      match data_size.checked_mul(4) { 
> -      Some(size) => {data_size = size}, 
> -      None => {return Status::INVALID_PARAMETER}, 
> +        Some(size) => data_size = size, 
> +        None => return Status::INVALID_PARAMETER, 
>      } 
>   
>      if data_size as usize > buffer_size { 
> -      return Status::UNSUPPORTED; 
> +        return Status::UNSUPPORTED; 
>      } 
>   
>      Status::SUCCESS 
> @@ -98,31 +86,27 @@ pub extern fn test_integer_checked_overflow (
>   
>  #[no_mangle] 
>  #[export_name = "TestIntegerCast"] 
> -pub extern fn test_integer_cast ( 
> -    buffer_size: u64, 
> -    ) -> u32 
> -{ 
> -    let data_size : u32 = buffer_size as u32; 
> +pub extern "C" fn test_integer_cast(buffer_size: u64) -> u32 { 
> +    let data_size: u32 = buffer_size as u32; 
>      data_size 
>  } 
>   
> -extern { 
> -  fn ExternInit(Data: *mut usize); 
> +extern "C" { 
> +    fn ExternInit(Data: *mut usize); 
>  } 
>   
>  #[no_mangle] 
>  #[export_name = "TestUninitializedVariable"] 
> -pub extern fn test_uninitializd_variable ( 
> -    index: usize, 
> -    ) -> usize 
> -{ 
> -    let mut data : usize = 1; 
> +pub extern "C" fn test_uninitializd_variable(index: usize) -> usize { 
> +    let mut data: usize = 1; 
>   
>      if index > 10 { 
> -      data = 0; 
> +        data = 0; 
>      } 
>   
> -    unsafe { ExternInit (&mut data ); } 
> +    unsafe { 
> +        ExternInit(&mut data); 
> +    } 
>   
>      data = data + 1; 
>   
> @@ -131,11 +115,8 @@ pub extern fn test_uninitializd_variable (
>   
>  #[no_mangle] 
>  #[export_name = "TestArrayOutOfRange"] 
> -pub extern fn test_array_out_of_range ( 
> -    index: usize, 
> -    ) -> usize 
> -{ 
> -    let mut data : [u8; 8] = [0; 8]; 
> +pub extern "C" fn test_array_out_of_range(index: usize) -> usize { 
> +    let mut data: [u8; 8] = [0; 8]; 
>   
>      data[index] = 1; 
>   
> @@ -152,18 +133,21 @@ pub struct TestTable {
>   
>  #[no_mangle] 
>  #[export_name = "TestBufferOverflow"] 
> -pub extern fn test_buffer_overflow ( 
> +pub extern "C" fn test_buffer_overflow( 
>      buffer: &mut [u8; 0], 
>      buffer_size: usize, 
>      table: &TestTable, 
>      table_size: usize, 
> -    ) 
> -{ 
> -    let mut dest = crate::mem::MemoryRegion::new(buffer as *mut [u8; 0] as usize as u64, buffer_size as u64); 
> -    let mut source = crate::mem::MemoryRegion::new(&table.value as *const [u8; 0] as usize as u64, table_size as u64); 
> - 
> -    for index in 0_u64 .. table.length as u64 { 
> -      dest.write_u8(index, source.read_u8(index)); 
> +) { 
> +    let mut dest = 
> +        crate::mem::MemoryRegion::new(buffer as *mut [u8; 0] as usize as u64, buffer_size as u64); 
> +    let mut source = crate::mem::MemoryRegion::new( 
> +        &table.value as *const [u8; 0] as usize as u64, 
> +        table_size as u64, 
> +    ); 
> + 
> +    for index in 0_u64..table.length as u64 { 
> +        dest.write_u8(index, source.read_u8(index));
>      } 
>  } 
>   
> @@ -177,59 +161,49 @@ pub struct TestTableFixed {
>   
>  #[no_mangle] 
>  #[export_name = "TestBufferOverflowFixed"] 
> -pub extern fn test_buffer_overflow_fixed ( 
> -    buffer: &mut [u8; 32], 
> -    table: &TestTableFixed, 
> -    ) 
> -{ 
> -    (*buffer)[0_usize..(table.length as usize)].copy_from_slice( 
> -      &table.value[0_usize..(table.length as usize)] 
> -      ); 
> +pub extern "C" fn test_buffer_overflow_fixed(buffer: &mut [u8; 32], table: &TestTableFixed) { 
> +    (*buffer)[0_usize..(table.length as usize)] 
> +        .copy_from_slice(&table.value[0_usize..(table.length as usize)]); 
>  } 
>   
> -fn get_buffer<'a> () -> Option<&'a mut TestTableFixed> 
> -{ 
> -    let ptr : *mut c_void = unsafe { AllocatePool (size_of::<TestTableFixed>()) }; 
> +fn get_buffer<'a>() -> Option<&'a mut TestTableFixed> { 
> +    let ptr: *mut c_void = unsafe { AllocatePool(size_of::<TestTableFixed>()) }; 
>      if ptr.is_null() { 
> -      return None; 
> +        return None; 
>      } 
> -    let buffer : &mut TestTableFixed = unsafe { core::mem::transmute::<*mut c_void, &mut TestTableFixed>(ptr) }; 
> +    let buffer: &mut TestTableFixed = 
> +        unsafe { core::mem::transmute::<*mut c_void, &mut TestTableFixed>(ptr) }; 
>      Some(buffer) 
>  } 
>   
> -fn release_buffer (test_table : &mut TestTableFixed) 
> -{ 
> -  test_table.r#type = 0; 
> -  unsafe { FreePool (test_table as *mut TestTableFixed as *mut c_void) ; } 
> +fn release_buffer(test_table: &mut TestTableFixed) { 
> +    test_table.r#type = 0; 
> +    unsafe { 
> +        FreePool(test_table as *mut TestTableFixed as *mut c_void); 
> +    } 
>  } 
>   
>  #[no_mangle] 
>  #[export_name = "TestBufferDrop"] 
> -pub extern fn test_buffer_drop ( 
> -     
> -    ) 
> -{ 
> -    match get_buffer () { 
> -      Some(buffer) => { 
> -        buffer.r#type = 1; 
> -        release_buffer(buffer); 
> -        drop (buffer); // This is required. 
> -        //buffer.r#type = 1; // error 
> -      }, 
> -      None => {}, 
> +pub extern "C" fn test_buffer_drop() { 
> +    match get_buffer() { 
> +        Some(buffer) => { 
> +            buffer.r#type = 1; 
> +            release_buffer(buffer); 
> +            drop(buffer); // This is required. 
> +                          //buffer.r#type = 1; // error 
> +        } 
> +        None => {} 
>      } 
>  } 
>   
>  #[no_mangle] 
>  #[export_name = "TestBufferBorrow"] 
> -pub extern fn test_buffer_borrow ( 
> -    test_table : &mut TestTableFixed 
> -    ) 
> -{ 
> -    let test_table2 : &mut TestTableFixed = test_table; 
> +pub extern "C" fn test_buffer_borrow(test_table: &mut TestTableFixed) { 
> +    let test_table2: &mut TestTableFixed = test_table; 
>      test_table2.r#type = 1; 
>   
> -    let test_table3 : &mut [u8; 64] = &mut test_table.value; 
> +    let test_table3: &mut [u8; 64] = &mut test_table.value; 
>      test_table3[63] = 0; 
>   
>      //test_table2.r#type = 2; // error 
> @@ -237,44 +211,38 @@ pub extern fn test_buffer_borrow (
>   
>  #[no_mangle] 
>  #[export_name = "TestBufferAlloc"] 
> -pub extern fn test_buffer_alloc ( 
> -     
> -    ) 
> -{ 
> +pub extern "C" fn test_buffer_alloc() { 
>      let layout = unsafe { core::alloc::Layout::from_size_align_unchecked(32, 4) }; 
>      unsafe { 
> -      match Global.alloc (layout) { 
> -        Ok(buffer) => { 
> -          let mut box_buffer = Box::from_raw(from_raw_parts_mut(buffer.as_ptr(), layout.size())); 
> -          box_buffer[0] = 1; 
> -          Global.dealloc (buffer, layout); 
> -          drop (buffer); // It is useless 
> -          box_buffer[0] = 1; // cannot catch 
> -        }, 
> -        Err(_) => handle_alloc_error (layout), 
> -      } 
> +        match Global.allocate(layout) { 
> +            Ok(mut buffer) => { 
> +                let mut box_buffer = Box::from_raw(buffer.as_mut()); 
> +                box_buffer[0] = 1; 
> +                Global.deallocate(buffer.as_non_null_ptr(), layout);
> +                drop(buffer); // It is useless 
> +                box_buffer[0] = 1; // cannot catch 
> +            } 
> +            Err(_) => handle_alloc_error(layout), 
> +        } 
>      } 
>   
>      let layout = core::alloc::Layout::new::<u32>(); 
>      unsafe { 
> -      match Global.alloc (layout) { 
> -        Ok(buffer) => { 
> -          Global.dealloc (buffer, layout); 
> -        }, 
> -        Err(_) => handle_alloc_error (layout), 
> -      } 
> +        match Global.allocate(layout) { 
> +            Ok(buffer) => { 
> +                Global.deallocate(buffer.as_non_null_ptr(), layout);
> +            } 
> +            Err(_) => handle_alloc_error(layout), 
> +        } 
>      } 
>  } 
>   
> -fn get_box ( 
> -    r#type: u32 
> -    ) -> Box<TestTableFixed> 
> -{ 
> -    let mut a = Box::new(TestTableFixed{ 
> -                       r#type: 0, 
> -                       length: size_of::<TestTableFixed>() as u32, 
> -                       value: [0; 64] 
> -                       }); // it will call __rust_alloc(). 
> +fn get_box(r#type: u32) -> Box<TestTableFixed> {
> +    let mut a = Box::new(TestTableFixed { 
> +        r#type: 0, 
> +        length: size_of::<TestTableFixed>() as u32, 
> +        value: [0; 64], 
> +    }); // it will call __rust_alloc(). 
>      a.r#type = r#type; 
>   
>      a 
> @@ -282,35 +250,26 @@ fn get_box (
>   
>  #[no_mangle] 
>  #[export_name = "TestBoxAlloc"] 
> -pub extern fn test_box_alloc ( 
> -    r#type: u32 
> -    ) -> Box<TestTableFixed> 
> -{ 
> -  let mut a = get_box(1); 
> +pub extern "C" fn test_box_alloc(r#type: u32) -> Box<TestTableFixed> { 
> +    let mut a = get_box(1); 
>   
> -  a.r#type = r#type; 
> +    a.r#type = r#type; 
>   
> -  //test_box_free(a); // build fail. 
> +    //test_box_free(a); // build fail. 
>   
> -  let b = a; 
> -  b 
> +    let b = a; 
> +    b 
>  } 
>   
>  #[no_mangle] 
>  #[export_name = "TestBoxFree"] 
> -pub extern fn test_box_free ( 
> -    buffer: Box<TestTableFixed> 
> -    ) 
> -{ 
> -  // it will call __rust_dealloc() 
> +pub extern "C" fn test_box_free(buffer: Box<TestTableFixed>) { 
> +    // it will call __rust_dealloc() 
>  } 
>   
>  #[no_mangle] 
>  #[export_name = "TestBoxAllocFail"] 
> -pub extern fn test_box_alloc_fail ( 
> -    size: u32 
> -    ) -> Box<[u8; 0x800]> 
> -{ 
> +pub extern "C" fn test_box_alloc_fail(size: u32) -> Box<[u8; 0x800]> { 
>      let mut a = Box::new([0_u8; 0x800]); // it will call __rust_alloc(). 
>   
>      a 
> @@ -318,22 +277,17 @@ pub extern fn test_box_alloc_fail (
>   
>  #[no_mangle] 
>  #[export_name = "TestBoxConvert"] 
> -pub extern fn test_box_convert ( 
> -    size: usize 
> -    ) -> *mut u8 
> -{ 
> +pub extern "C" fn test_box_convert(size: usize) -> *mut u8 { 
>      let layout = unsafe { core::alloc::Layout::from_size_align_unchecked(size, 4) }; 
>      unsafe { 
> -      match Global.alloc (layout) { 
> -        Ok(buffer) => { 
> -          let mut box_buffer = Box::<u8>::from_raw(from_raw_parts_mut(buffer.as_ptr(), layout.size()) as *mut [u8] as *mut u8 ); 
> -          Global.dealloc (buffer, layout); 
> -          *box_buffer = 1; 
> -          Box::<u8>::into_raw(box_buffer) 
> -        }, 
> -        Err(_) => handle_alloc_error (layout), 
> -      } 
> +        match Global.allocate(layout) { 
> +            Ok(buffer) => { 
> +                let mut box_buffer = Box::<u8>::from_raw(buffer.as_mut_ptr()); 
> +                Global.deallocate(buffer.as_non_null_ptr(), layout);
> +                *box_buffer = 1; 
> +                Box::<u8>::into_raw(box_buffer) 
> +            } 
> +            Err(_) => handle_alloc_error(layout), 
> +        } 
>      } 
> - 
> - 
>  } 
> 
> 

[-- Attachment #2: Type: text/html, Size: 29074 bytes --]

  reply	other threads:[~2022-03-17 19:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-17 14:54 [Patch] Fix edkii-rust brach in edk2-staging ayushdevel1325
2022-03-17 19:33 ` Marvin Häuser [this message]
2022-03-17 19:53   ` [edk2-devel] " ayushdevel1325

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CB1B6A6D-D981-49B5-8FC7-93BAA6510E8F@posteo.de \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox