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.

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

Jump to bottom

raw-dylib compilation fails targeting nightly-i686-pc-windows-gnu when function name begins with 'L' #104453

Closed
kennykerr opened this issue Nov 15, 2022 · 3 comments
Closed

raw-dylib compilation fails targeting nightly-i686-pc-windows-gnu when function name begins with 'L' #104453

kennykerr opened this issue Nov 15, 2022 · 3 comments
Assignees
@dpaoliello
Labels
C-bug Category: This is a bug.

Comments

@kennykerr
Copy link
Contributor

kennykerr commented Nov 15, 2022 •
edited by rustbot
Loading

While testing raw-dylib support against nightly-i686-pc-windows-gnu I noticed that it would fail with a cryptic error.

Here's a minimal repro: https:/kennykerr/repro-raw-dylib

#![feature(raw_dylib)]
#![feature(native_link_modifiers_verbatim)]

#[link(name = "kernel32.dll", kind = "raw-dylib", modifiers = "+verbatim", import_name_type = "undecorated")]
extern "stdcall" {
    fn LoadLibraryA(name: *const u8) -> isize;
}

#[test]
fn test() {
    assert_ne!(0, unsafe { LoadLibraryA("kernel32.dll\0".as_ptr()) });
}

This compiles and runs cleanly i686-pc-windows-msvc but fails on i686-pc-windows-gnu with the following error:

error: assembler label 'LoadLibraryA' can not be undefined

rust version 1.67.0-nightly (e631891f7 2022-11-13)

@dpaoliello pointed out that it seems that if we have an undecorated symbol that begins with 'L' (yep) then LLVM assumes that it is an assembler label, hence the error.
@kennykerr kennykerr added the C-bug Category: This is a bug. label Nov 15, 2022
@dpaoliello
Copy link
Contributor

@rustbot claim

@kennykerr kennykerr mentioned this issue Nov 15, 2022
Merged
This was referenced Nov 15, 2022
Closed
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 21, 2022
@matthiaskrgr
Rollup merge of rust-lang#104511 - dpaoliello:privateglobalworkaround…
4eb13ef 
…, r=michaelwoerister

Mark functions created for `raw-dylib` on x86 with DllImport storage class

Fix for rust-lang#104453

## Issue Details
On x86 Windows, LLVM uses 'L' as the prefix for any private global symbols (`PrivateGlobalPrefix`), so when the `raw-dylib` feature creates an undecorated function symbol that begins with an 'L' LLVM misinterprets that as a private global symbol that it created and so fails the compilation at a later stage since such a symbol must have a definition.

## Fix Details
Mark the function we are creating for `raw-dylib` with `DllImport` storage class (this was already being done for MSVC at a later point for `callee::get_fn` but not for GNU (due to "backwards compatibility")): this will cause LLVM to prefix the name with `__imp_` and so it won't mistake it for a private global symbol.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 21, 2022
@matthiaskrgr
Rollup merge of rust-lang#104511 - dpaoliello:privateglobalworkaround…
cc2397b 
…, r=michaelwoerister

Mark functions created for `raw-dylib` on x86 with DllImport storage class

Fix for rust-lang#104453

## Issue Details
On x86 Windows, LLVM uses 'L' as the prefix for any private global symbols (`PrivateGlobalPrefix`), so when the `raw-dylib` feature creates an undecorated function symbol that begins with an 'L' LLVM misinterprets that as a private global symbol that it created and so fails the compilation at a later stage since such a symbol must have a definition.

## Fix Details
Mark the function we are creating for `raw-dylib` with `DllImport` storage class (this was already being done for MSVC at a later point for `callee::get_fn` but not for GNU (due to "backwards compatibility")): this will cause LLVM to prefix the name with `__imp_` and so it won't mistake it for a private global symbol.
@mati865 mati865 mentioned this issue Jan 22, 2023
Merged
@goffrie
Copy link
Contributor

goffrie commented Jan 25, 2023

This was fixed by #104511, right?

@dpaoliello
Copy link
Contributor

This was fixed by #104511, right?

Correct

@kennykerr kennykerr mentioned this issue Jan 27, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dpaoliello dpaoliello

Labels
C-bug Category: This is a bug.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@goffrie @ChrisDenton @kennykerr @dpaoliello