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

analyze: Lifetimes are incorrectly added to public extern C functions #1104

Open
ahomescu opened this issue Jul 12, 2024 · 2 comments
Open
Assignees
Labels
bug Something isn't working c2rust-analyze

Comments

@ahomescu
Copy link
Contributor

Running the analyzer on amalgamated CFS adds lifetimes to pointer arguments of publicly exported C functions part of the public API (not called from inside CFS), e.g.

#[no_mangle]
pub unsafe extern "C" fn OS_GetLocalTime<'h0>(mut time_struct: &'h0 (OS_time_t)) -> int32 {
// 5927: mut time_struct: typeof(_1) = *mut {g68} DefId(0:6740 ~ cfs_rust_amalgamated[9f4b]::OS_time_t)
// 5927: mut time_struct:   g68 = UNIQUE | NON_NULL, (empty)
    if time_struct.is_null() { 

where the original function signature was pub unsafe extern "C" fn OS_GetLocalTime(mut time_struct: *mut OS_time_t) -> int32.

@ahomescu ahomescu added bug Something isn't working c2rust-analyze labels Jul 12, 2024
@ahomescu
Copy link
Contributor Author

@spernsteiner Do you have time to look into this? Should I take a pass at it?

@spernsteiner
Copy link
Collaborator

IIRC this is basically intended/expected behavior at the moment. We've mainly been targeting application code, where callers can be updated in lockstep when function signatures change. It's generally hard to tell which things are actually "publicly exported" as part of the library API as opposed to just being non-static so they can be called from another compilation unit within the same library. (Knowing that a particular function is/isn't called elsewhere within the library doesn't necessarily help with getting a precise answer here.)

One thing we've talked about doing in the past is splitting API functions into an internal version whose signature we can change and an external shim that keeps the original signature and converts the arguments/return value to match the new signature. In this case, it might look like this:

#[no_mangle]
pub unsafe extern "C" fn OS_GetLocalTime(mut time_struct: *mut OS_time_t) -> int32 {
    OS_GetLocalTime_internal(&*time_struct)
}

unsafe fn OS_GetLocalTime_internal<'h0>(mut time_struct: &'h0 (OS_time_t)) -> int32 {
    // Actual logic goes here
}

There's some code in rewrite::shim that could potentially be reused to generate the shims. We already do this in some cases where a function that's marked non-rewritable/FIXED calls into one that we do want to rewrite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c2rust-analyze
Projects
None yet
Development

No branches or pull requests

2 participants