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

Disallow duplicated extern declarations #12707

Open
klutzy opened this issue Mar 5, 2014 · 11 comments
Open

Disallow duplicated extern declarations #12707

klutzy opened this issue Mar 5, 2014 · 11 comments
Labels
A-ffi Area: Foreign Function Interface (FFI) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@klutzy
Copy link
Contributor

klutzy commented Mar 5, 2014

mod a {
    extern {
        fn func();
    }
}

mod b {
    extern {
        fn func(i: i8); // different signature
    }
}

Currently rustc accepts this, but I think it should be disallowed to reduce potential mistakes.

@brson
Copy link
Contributor

brson commented Mar 5, 2014

This would disallow duplicate functions with different signatures, but still allow duplicate with the same?

@huonw huonw added the A-ffi label Mar 5, 2014
@klutzy
Copy link
Contributor Author

klutzy commented Mar 5, 2014

I thought identical duplicates are ok because make check-fast needs them. Some rpass tests use same extern fn then they are combined into one crate.

alexcrichton referenced this issue in klutzy/rust Mar 9, 2014
This fixes struct passing abi on x86 ffi: Structs are now passed
indirectly with byval attribute (as clang does).
@klutzy
Copy link
Contributor Author

klutzy commented Mar 10, 2014

Modified test case causing llvm assertion error on x64:

mod a {
    extern {
        fn func(); // declare void @func() unnamed_addr
    }
}

mod b {
    struct S {
        a: u64,
        b: u64,
        c: u64,
    }

    extern {
        fn func(s: S); // declare void @func(%"struct.b::S"* byval) unnamed_addr
    }
}
rustc: /media/a/lime/src/rust/src/llvm/include/llvm/Support/Casting.h:240:typename llvm::cast_retty<X, Y*>::ret_type llvm::cast(Y*) [with X = llvm::Argument; Y = llvm::Value; typename llvm::cast_retty<X, Y*>::ret_type = llvm::Argument*]: Assertion `isa<X>(Val) && "cast<Ty>() argument of incompatible type!"' failed.

In the case, argument has byval attribute.

@klutzy
Copy link
Contributor Author

klutzy commented Mar 10, 2014

cc #12762: it contains a patch to suppress llvm assertion error above. Not really.

@rprichard
Copy link
Contributor

klutzy's test case is causing a different error now on 64-bit Linux:

Attribute after last parameter!
void ()* @func
LLVM ERROR: Broken module found, compilation aborted!

Are duplicate declarations useful/necessary for calling functions like objc_msgSend? IIRC, that function isn't really varargs -- the prototype varies depending upon which ObjC method is invoked. (I discovered this GitHub issue because I did something similar to objc_msgSend.)

@steveklabnik
Copy link
Member

Traige: Still getting the same failure as @rprichard , but I'm also on x86-64. Does this fail everywhere now?

@Mark-Simulacrum
Copy link
Member

On x86-64 as well, does not reproduce with --crate-type lib and rustc 1.18.0-nightly (bbdaad0dc 2017-04-14). Going to presume the assertions are fixed, but the original issue (multiple externs with the same name) isn't, so leaving open.

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 20, 2017
@steveklabnik
Copy link
Member

Triage: no change

@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 12, 2020
@BurgundyWillow
Copy link

BurgundyWillow commented Dec 9, 2020

The rustc compiler does throw warnings when we use two conflicting function signatures in extern declarations. So, shall we change this to disallow it and instead throw an error?

@bjorn3
Copy link
Member

bjorn3 commented May 31, 2023

objc_msgSend is declared with multiple signatures in libstd. The signature of this function depends on the message sent. It forwards all arguments to the called method. Declaring it as a vararg function is not valid in AArch64 due to ABI differences.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2024
…omcc

Refactor uses of `objc_msgSend` to no longer have clashing definitions

This is very similar to what Apple's own headers encourage you to do (cast the function pointer before use instead of making new declarations).

Additionally, I'm documenting a few of the memory management rules we're following, ensuring that the `args` function doesn't leak memory (if you wrap it in an autorelease pool).

Motivation is to avoid issues with clashing definitions, like described in rust-lang#12707 (comment) and rust-lang#46188 (comment), CC `@bjorn3.`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2024
…omcc

Refactor uses of `objc_msgSend` to no longer have clashing definitions

This is very similar to what Apple's own headers encourage you to do (cast the function pointer before use instead of making new declarations).

Additionally, I'm documenting a few of the memory management rules we're following, ensuring that the `args` function doesn't leak memory (if you wrap it in an autorelease pool).

Motivation is to avoid issues with clashing definitions, like described in rust-lang#12707 (comment) and rust-lang#46188 (comment), CC ``@bjorn3.``
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 22, 2024
Rollup merge of rust-lang#117910 - madsmtm:msg-send-no-clashing, r=thomcc

Refactor uses of `objc_msgSend` to no longer have clashing definitions

This is very similar to what Apple's own headers encourage you to do (cast the function pointer before use instead of making new declarations).

Additionally, I'm documenting a few of the memory management rules we're following, ensuring that the `args` function doesn't leak memory (if you wrap it in an autorelease pool).

Motivation is to avoid issues with clashing definitions, like described in rust-lang#12707 (comment) and rust-lang#46188 (comment), CC ``@bjorn3.``
@madsmtm
Copy link
Contributor

madsmtm commented Jan 22, 2024

objc_msgSend is declared with multiple signatures in libstd. The signature of this function depends on the message sent. It forwards all arguments to the called method. Declaring it as a vararg function is not valid in AArch64 due to ABI differences.

Since #117910, this is no longer true, instead we explicitly cast the function pointer (which is arguably the more correct behaviour, at least it's what the C headers also declare and expect you to do).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ffi Area: Foreign Function Interface (FFI) C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants