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.

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

incompatible i2c_t types #24

Open
mikafreiwald opened this issue Mar 2, 2023 · 7 comments
Open

incompatible i2c_t types #24

mikafreiwald opened this issue Mar 2, 2023 · 7 comments

Comments

@mikafreiwald
Copy link

When trying to generate bindings for the bmx280 device driver, I noticed that the size of the struct bmx280_params_t differs from C to Rust. I used the module USEMODULE += bmp280_i2c and added these lines to rust-riot-sys/riot-headers.h to generate the bindings:

#include "bmx280.h"
#include "bmx280_params.h"

The struct contains an i2c_dev of type i2c_t which seems to be the reason for the difference. I used the following programs to print the sizes. In C:

#include <stdio.h>
#include <bmx280.h>
int main(void) {
    printf("sizeof bmx280_params_t = %d\n", sizeof(bmx280_params_t));
    printf("sizeof i2c_t = %d\n", sizeof(i2c_t));
}
sizeof bmx280_params_t = 32
sizeof i2c_t = 4

In Rust:

use riot_wrappers::{println, riot_main};
use riot_sys::{bmx280_params_t, i2c_t};
use core::mem::size_of;
riot_main!(main);
fn main() {
    println!("sizeof bmx280_params_t = {}", size_of::<bmx280_params_t>());
    println!("sizeof i2c_t = {}", size_of::<i2c_t>());
}
sizeof bmx280_params_t = 28
sizeof i2c_t = 1

In the file RIOT/drivers/include/periph/i2c.h the type is defined as follows:

#ifndef HAVE_I2C_T
typedef uint_fast8_t i2c_t;
#endif

For native, xtensa, risc-v and arm, it seems like all the fast integer types are defined through internal compiler defines like typedef __INT_FAST8_TYPE__ int_fast8_t; These types differ since RIOT is compiled by gcc and the Rust bindings are created with llvm. I did some tests and got the following types using different architectures. The cells show the actual type when using gcc / clang:

Bit Native (x86-64) Xtensa RISC-V ARM
8 char / char int / char int / char int / char
16 long int / short int / short int / short int / short
32 long int / int int / int int / int int / int
64 long int / long int long long int /
long long int
long long int /
long long int
long long int /
long long int

One workaround is to redefine these macros when generating the bindings so that they match the types used by gcc. For the uint_fast8_t this can be done by adding the following lines to riot-bindgen.h and riot-c2rust.h:

#undef __UINT_FAST8_TYPE__
#define __UINT_FAST8_TYPE__ unsigned int

Alternatively, this also works:

#define HAVE_I2C_T
typedef unsigned int i2c_t

However there should be a better way that actually guarantees that the same type is used when compiling RIOT and creating the bindings.

@chrysn
Copy link
Member

chrysn commented Mar 2, 2023

Just to make sure I understand the issue right: If you built without modifications but setting TOOLCHAIN=llvm, then everything would also work, because LLVM would agree with itself on the sizes?

If so, then RIOT is breaking its (not very explicit) promise to have all cross-compilation-unit interfaces compiler independent (which it upholds in other places eg. by casting enums to explicitly sized types). A possible remedy might be ensuring that compatible compiler flags are used.

Please use your workarounds for the moment; I'd like to coordinate this with the RIOT maintainers. (A fix might be sold most easily if this also shows up when some modules are built with clang and others with gcc).

@mikafreiwald
Copy link
Author

yes, my guess is that everything works when also compiling RIOT with llvm. But I can't test this because I don't have a board that works with TOOLCHAIN=llvm.

@chrysn
Copy link
Member

chrysn commented Mar 2, 2023

I was unaware we had any such board (at least, outside AVR which is not suported by riot-sys yet). Which board is it / is there an open issue about that it doesn't work? (Fixing Rust-on-RIOT stuff is generally easier once LLVM works).

@mikafreiwald
Copy link
Author

When using an esp32 I get the message: CFLAGS_CPU must have been defined to use 'llvm'. But I can try to test this on an arm board. I just don't have one right now.

@chrysn
Copy link
Member

chrysn commented Mar 2, 2023

Chances are things work fine on the ARM board either way. Which ESP32 are you using, a RISC-V or an Xtensa one? (The latter is unsupported so far and the former might have other issues, but I'd guess you're from the same project as Remmirad, and you might be using adjustments from #20).

@mikafreiwald
Copy link
Author

Yes I am from the same project. I used both a RISC-V and Xtensa esp32. But we also had this problem with an arm board. I will try to reproduce this.

@mikafreiwald
Copy link
Author

When building for BOARD = bluepill-stm32f103c8 and not setting TOOLCHAIN I get sizeof(bmx280_params_t) == 12 and sizeof(i2c_t) == 4 in C and size_of::<riot_sys::bmx280_params_t>() == 8 and size_of::<riot_sys::i2c_t>() == 1 in Rust.

When setting TOOLCHAIN=llvm I get an error: RIOT/cpu/stm32/cpu_init.c:400:20: error: unused function '_backup_battery_connected' [-Werror,-Wunused-function]. If I comment out this function, everything compiles and I get sizeof(bmx280_params_t) == 8 and sizeof(i2c_t) == 1 in C and size_of::<riot_sys::bmx280_params_t>() == 8 and size_of::<riot_sys::i2c_t>() == 1 in Rust. So the sizes match when building both RIOT and Rust with LLVM.

Even when not using Rust and just building a pure C application, the size of i2c_t differs based on whether TOOLCHAIN=llvm is set or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants