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

use case: @cImport of Kero Platform library #2257

Open
UltimaN3rd opened this issue Apr 11, 2019 · 16 comments
Open

use case: @cImport of Kero Platform library #2257

UltimaN3rd opened this issue Apr 11, 2019 · 16 comments
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Milestone

Comments

@UltimaN3rd
Copy link
Contributor

UltimaN3rd commented Apr 11, 2019

Using Zig 0.4.0 from the Snap store on Linux Mint 19.1 Mate.

I've got a C library called Kero Platform which just takes care of platform specific stuff like opening a window. I'm trying to compile a simple Zig program by importing the library and calling the first function, KPInit and am getting the error above. Here's the main zig file:

const std = @import("std");
const warn = @import("std").debug.warn;
const c = @cImport({
    @cInclude("kero_platform.h");
});

pub fn main() void {
    c.KPInit(1280, 720, c"Zig test");
}

The full project is here: https://gitlab.com/UltimaN3rd/hello-zig
And my Kero Platform library is here: https://gitlab.com/UltimaN3rd/croaking-kero-c-libraries/blob/master/include/kero_platform.h

So what am I doing wrong?

@andrewrk
Copy link
Member

Have a look at #2259 which might explain a little bit about what's going on. One thing you can do is use --verbose-cimport and Zig will print a file that you can look at to see the generated .zig code. Once #2259 is solved, this file would mention why KPInit could not be translated. Until that's done it will print to stderr directly the warnings. Do they mention KPInit?

@UltimaN3rd
Copy link
Contributor Author

Compiling with --verbose-cimport there were a couple of warnings about variables being initialized so I just removed their initialization which cleared that. However there seems to be a warning for most or all functions saying:
warning: unable to translate function
There are also many variations of this error:
warning: TODO handle C CK_IntegralToFloating
Oddly this seems to include stdbool.h:
/snap/zig/425/lib/zig/include/stdbool.h:32:14: warning: TODO handle C CK_IntegralToBoolean

The first error is:
error: cast increases pointer alignment kp_frame_buffer.pixels = @ptrCast([*c]u32, calloc(@sizeOf(u32), c_ulong(kp_window.w *% kp_window.h)));

Am I supposed to do something special in my C code to cast pointers? This used to be a malloc call which produced the same error so I tried the calloc call which includes the size of the data type because I thought that might fix the alignment of the pointer. Also getting:

error: expected type 'c_uint', found 'c_int' ximage = XCreateImage(display, visual_info.visual, c_uint(visual_info.depth)[...]

Not sure what I'm supposed to do about this one. It seems like Zig thinks visual_info.depth is int when it should be uint, but visual_info is of XVisualInfo type so I can't change what type its depth member is 0.o Another pointer cast alignment error:

error: cast increases pointer alignment graphics_context = (&@ptrCast(_XPrivDisplay, display).?.screens[screen]).?.default_gc;

graphics_context is X11's type GC so I'm not sure what I'm supposed to do about this one either.

Thanks Andrew!

@andrewrk
Copy link
Member

andrewrk commented Apr 11, 2019

It looks like you've found a great use case which is finding faults in zig's translate-c code and related semantics. As a rule of thumb, zig code using C API should be able to roughly look the same as the C code that uses the C API without too much hoop jumping. So all this stuff you are running into is definitely considered problems with Zig that need to be solved.

I think this is sort of the thing where someone knowledgeable about how translate-c works in Zig (or wishes to become so) will have to sit down and try this out for themselves, fixing the issues along the way one by one.

This issue is a "use case" issue, meaning the way to solve it is to do the thing that @UltimaN3rd is trying to do in the original post, and identify and file other issues for the problems that arise. Then once those issues are all solved and this use case works well, we can close it.

@andrewrk andrewrk changed the title error: container '.cimport:3:11' has no member called 'KPInit' use case: @cImport of Kero Platform library Apr 11, 2019
@andrewrk andrewrk added the contributor friendly This issue is limited in scope and/or knowledge of Zig internals. label Apr 11, 2019
@daurnimator
Copy link
Contributor

The first error is:
error: cast increases pointer alignment kp_frame_buffer.pixels = @ptrCast([*c]u32, calloc(@sizeOf(u32), c_ulong(kp_window.w *% kp_window.h)));

Am I supposed to do something special in my C code to cast pointers? This used to be a malloc call which produced the same error so I tried the calloc call which includes the size of the data type because I thought that might fix the alignment of the pointer.

This was fixed via #2318

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Apr 30, 2019
@andrewrk andrewrk added the translate-c C to Zig source translation feature (@cImport) label Jun 16, 2019
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Oct 17, 2019
@andrewrk andrewrk added the frontend Tokenization, parsing, AstGen, Sema, and Liveness. label Jan 2, 2020
@daurnimator
Copy link
Contributor

@UltimaN3rd could you give this another go with zig master?

@UltimaN3rd
Copy link
Contributor Author

Downloaded the zig master branch, made a small update to hello-zig to use the latest version of Kero_Platform and tried to compile. Got this error:

cimport.zig:5376:34: error: invalid character: ';'
pub const _POSIX_VDISABLE = '\x00;

@LemonBoy
Copy link
Contributor

LemonBoy commented Jan 3, 2020

That bug got fixed yesterday by #4047

@UltimaN3rd
Copy link
Contributor Author

Downloaded master again. Having trouble initializing a struct. Looked at the docs, searched the threads and could only find that a struct has to have every single member named and assigned a value? So I can't do this:

var platform: c.kero_platform_t = {0};

If there is a way to initialize a struct to 0 I'm happy to try again and see if it'll compile.

@andrewrk
Copy link
Member

andrewrk commented Jan 4, 2020

var platform: c.kero_platform_t = undefined;
@memset(@ptrCast([*]u8, &platform), 0, @sizeOf(c.kero_platform_t));

with a (as far as I'm aware does not exist yet) std lib helper function:

var platform = std.mem.zeroes(c.kero_platform_t);

@UltimaN3rd
Copy link
Contributor Author

UltimaN3rd commented Jan 5, 2020

Thanks Andrew, I'll certainly look forward to that std lib function to init structs to 0. I compiled with the suggested code and got a bunch of errors:

/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3918:21: error: unable to translate function
pub const KP_Init = @compileError("unable to translate function"); // /home/nick/git/croaking-kero-c-libraries/include/kero_platform.h:582:115: warning: TODO access of anonymous field
                    ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/hello.zig:10:6: note: referenced here
    c.KP_Init(&platform, 1280, 720, "Zig test");
     ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3939:28: error: unable to translate function
pub const KP_UpdateMouse = @compileError("unable to translate function");
                           ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3922:5: note: referenced here
    KP_UpdateMouse(platform);
    ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3955:92: error: expected type '[*c]const u8', found '[1]u8'
        var pixmap: Pixmap = XCreateBitmapFromData(platform.*.display, platform.*.xwindow, data, @bitCast(c_uint, @as(c_int, 1)), @bitCast(c_uint, @as(c_int, 1)));
                                                                                           ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3973:17: error: expected type '[*c]u8', found '[1024]u8'
    _ = sprintf(command, "xdg-open %s", url);
                ^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+b91eaba38/hello-zig/zig-cache/o/j7OGFKanV6q1meO57izkriqvBt6WzljKvGZBLg-DFXM4Y2oIxD0W3gJ8KM5zBYvm/cimport.zig:3979:17: error: expected type '[*c]u8', found '[1024]u8'
    _ = sprintf(command, "mkdir \"%s\"", path);

I'm sure some casting can fix those expected type errors, but the "unable to translate function" errors seem to revolve around KP_UpdateMouse so I'll paste the source for that function:

    void KP_UpdateMouse(kero_platform_t *platform) {
        Window window_returned;
        int display_x, display_y;
        unsigned int mask_return;
        if(XQueryPointer(platform->display, platform->xwindow, &window_returned,
                         &window_returned, &display_x, &display_y, &platform->mouse.x, &platform->mouse.y, 
                         &mask_return) == True) {
        }
        if(platform->mouse.invertx) {
            platform->mouse.x = platform->window.w - platform->mouse.x - 1;
        }
        if(platform->mouse.inverty) {
            platform->mouse.y = platform->window.h - platform->mouse.y - 1;
        }
    }

I think that XQueryPointer call is in an if statement because there used to be some code in the brackets. Anyway I don't see anything special about this function so I wonder why it fails to translate.

@andrewrk
Copy link
Member

andrewrk commented Jan 7, 2020

Well @UltimaN3rd, it seems the Zig community is dead set on solving this use case 😄

@FireFox317 implemented std.mem.zeroes

@LemonBoy implemented #4078 and #4081 and notes that this should solve the remaining problems with importing Kero .h files.

@UltimaN3rd
Copy link
Contributor Author

Thanks @andrewrk, @FireFox317 and @LemonBoy! It compiles without a problem now. However when running it crashes with these errors:

Segmentation fault at address 0x0
???:?:?: 0x7f924f58931a in ??? (???)

incorrect alignment
/home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/debug.zig:123:34: 0x22bd91 in std.debug.dumpStackTraceFromBase (hello)
const first_return_address = @intToPtr(const usize, bp + @sizeof(usize)).;
^
/home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/debug.zig:2454:35: 0x227438 in std.debug.handleSegfaultLinux (hello)
dumpStackTraceFromBase(bp, ip);
^
???:?:?: 0x7f924f19af1f in ??? (???)

Panicked during a panic. Aborting.

@LemonBoy
Copy link
Contributor

Is your Zig code available somewhere? I tried the following snippet and everything seemed to work as expected:

const std = @import("std");
const warn = @import("std").debug.warn;
const c = @cImport({
    @cInclude("/home/lemonboy/Downloads/kero_platform.h");
});

pub fn main() void {
    var platform: c.kero_platform_t = undefined;
    c.KP_Init(&platform, 1280, 720, "Zig test");
}

And let's not forget @travisstaloch work on #4113, that solved the very last problem we had in translating your library headers!

@UltimaN3rd
Copy link
Contributor Author

My code looks like this:

const std = @import("std");
const warn = @import("std").debug.warn;
const c = @cImport({
@Cinclude("kero_platform.h");
});

pub fn main() void {
var platform: c.kero_platform_t = std.mem.zeroes(c.kero_platform_t);
c.KP_Init(&platform, 1280, 720, "Zig test");
c.KP_Sleep(2000);
}

My apologies for not linking it earlier, but I have a small repo just for this: https://gitlab.com/UltimaN3rd/hello-zig

I tried the exact code you provided too, changing the kero_platform.h directory and got the same errors as before.

I'm also using this build file:

const std = @import("std");
const path = std.os.path;
const Builder = std.build.Builder;

pub fn build(b: *Builder) void {
const exe = b.addExecutable("hello", "hello.zig");
exe.setBuildMode(b.standardReleaseOptions());
exe.linkSystemLibrary("c");
exe.linkSystemLibrary("X11");
exe.linkSystemLibrary("m");
exe.addIncludeDir("/home/nick/git/croaking-kero-c-libraries/include");

b.default_step.dependOn(&exe.step);

}

Perhaps that is the issue? Since the documentation for the zig build system is still TODO it's tough figuring out what to write just based on other build.zig files.

Ah and I'm on Linux. I haven't bothered to try it on Windows/Mac with Zig until I get it running on Linux first, but perhaps you're on Windows and it works there but not Linux? I had imagined that most Zig contributors would be using Linux so I didn't think of that possibility until now.

Cheers

@LemonBoy
Copy link
Contributor

That's extremely weird, I'm able to build & run your example code just fine, with or without zig build.
Can you dump a complete backtrace using gdb? That'd help to pinpoint what's wrong.

@UltimaN3rd
Copy link
Contributor Author

Here's the backtrace:

(gdb) bt
#0 0x00007ffff7ad931a in XMatchVisualInfo ()
from /usr/lib/x86_64-linux-gnu/libX11.so.6
#1 0x0000000000206a70 in KP_Init (arg_platform=0x7fffffffda28,
arg_width=1280, arg_height=720,
arg_title=0x204d40 <__unnamed_6> "Zig test")
at /home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/hello-zig/zig-cache/o/awLx9lpX--n8ewBRa55t3cUXQwje4GkikvN3K6Iu-iVhDCK3T3wb85zkB5W4vv6q/cimport.zig:3984
#2 0x00000000002067bb in main () at hello.zig:9
#3 0x0000000000206fc4 in std.start.callMain ()
at /home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/start.zig:249
#4 std.start.initEventLoopAndCallMain ()
at /home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/start.zig:231
#5 std.start.callMainWithArgs (argc=1, argv=0x7fffffffddb8, envp=...)
at /home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/start.zig:194
#6 std.start.main (c_argc=1, c_argv=0x7fffffffddb8, c_envp=0x7fffffffddc8)
at /home/nick/Downloads/zig-linux-x86_64-0.5.0+84e98405d/lib/zig/std/start.zig:201

After looking at the code that calls XMatchVisualInfo I believe I found and fixed a bug in kero_platform.h, so thanks Zig! platform->visual_info is an XVisualInfo pointer which is undefined so I changed it to a normal XVisualInfo instance and passed its address to the function instead. Now it compiles and runs with no errors! Thanks a lot everyone :)

@andrewrk andrewrk removed this from the 0.7.0 milestone Aug 13, 2020
@andrewrk andrewrk added this to the 0.8.0 milestone Aug 13, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.10.0 May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor friendly This issue is limited in scope and/or knowledge of Zig internals. frontend Tokenization, parsing, AstGen, Sema, and Liveness. translate-c C to Zig source translation feature (@cImport)
Projects
None yet
Development

No branches or pull requests

4 participants