access violation when returning across abi boundary

[round heart]

i have a dll thats called by a program, when calling a function things work and external calls work, however when the return statement is reached, an access violation is thrown

https://cdn.discordapp.com/attachments/1425810713787633746/1425810714471174235/image.png

it points to the return call, return bapi.getNull().inner;, defined as

    return ByondValue{
        .inner = .{
            .type = .Null,
            .data = .{ .ref = 0 },
        },
    };

pub const ByondValueType = enum(u1c) {...}`
pub const CByondValue = extern struct {
    type: ByondValueType,
    junk1: u1c = undefined,
    junk2: u1c = undefined,
    junk3: u1c = undefined,
    data: extern union {
        ref: u4c,
        num: f32,
    },
};
pub const u1c = u8;
pub const s1c = i8;
pub const u2c = c_ushort;
pub const s2c = c_short;
pub const u4c = c_uint;
pub const s4c = c_int;
pub const s8c = c_longlong;
pub const u8c = c_ulonglong;

header definition

typedef u1c ByondValueType;
typedef union ByondValueData {
    u4c ref;    //!< 4-byte reference ID
    float num;    //!< floating-point number
} ByondValueData;

typedef struct CByondValue {
    ByondValueType type;    //!< 1-byte intrinsic data type
    u1c junk1, junk2, junk3;    //!< padding
    ByondValueData data;    //!< 4-byte reference ID or floating point number
} CByondValue;

typedef unsigned char  u1c;
typedef signed   char  s1c;
typedef unsigned short u2c;
typedef signed   short s2c;
#ifdef DM_64BIT
    typedef unsigned int   u4c;
    typedef signed   int   s4c;
#else
    typedef unsigned long  u4c;
    typedef signed   long  s4c;
#endif

#if defined(i386) || defined(__i386) || defined(__i386__) || defined(_M_IX86) || defined(_X86_) || defined(__X86__)
    #define _X86
    #define _X86ORX64
    #define DM_32BIT
#elif defined(__amd64__) || defined(__amd64) || defined(__x86_64__) || defined(__x86_64) || defined(_M_AMD64) || defined(_M_X64) || defined(_WIN64) || defined(WIN64)
    #define _X64
    #define _X86ORX64
    #define DM_64BIT
#elif defined(__arm__) || defined(_M_ARM)
    #define _ARM
    #if defined(__LP64__) || defined(_LP64)
        #define DM_64BIT
    #else
        #define DM_32BIT
    #endif
#endif

[leaden trail]

is ByondValue here an alias for CByoneValue in the translated code?

[round heart]

wrapper one-value struct

[leaden trail]

can I see its definition?

and also the function type in c, as well as the definition line of the zig function

[round heart]

just this + a stupid amount of functions

pub const ByondValue = struct {
    inner: ByondValueRaw = undefined,

[leaden trail]

ah okay so ur returning the inner field, which is the translated type, right?

[round heart]

example fn which works

/**
 * Returns a reference to an existing string ID, but does not create a new string ID.
 * Blocks if not on the main thread.
 * @param str Null-terminated string
 * @return ID of string; NONE if string does not exist
 */
DUNGPUB u4c Byond_GetStrId(char const *str);

/* dll export stuff */
#ifdef WIN32
    #define DUNGPUB    __declspec(dllimport)
    #define BYOND_EXPORT __declspec(dllexport)    // for functions in user-defined DLLs for use with call_ext()
#else // unix/g++, combine with -fvisibility=hidden to hide non-exported symbols
    #define DUNGPUB
    #define BYOND_EXPORT __attribute__ ((visibility("default")))    // for functions in user-defined .so libraries for use with call_ext()
#endif

[round heart]

ye

[leaden trail]

can I see the disassembly around the segfault?

[round heart]

sure but you'll have to baby me through it as im still new to windbg

the assembly button right of source just toggles how to step through code

[leaden trail]

tbh I don't rly know windbg very well either, but try clicking the "assembly" button in the toolbar

oh rip

uhhhh

"disassembly" on the left hand side of the window

[round heart]

oh

ewllll time to check the deinit i guess

actually just have the entire function that'd prbably make things easier

[leaden trail]

no, this happens after the deinit call

can u send a screenshot of the "registers" tab?

[round heart]

note that the "fatal error in sandll" crash line gets reached successfully

[round heart]

oh

[leaden trail]

also, are u targeting 32-bit x86?

[round heart]

school wifi i beg of theeeeeeeeee

[round heart]

-Dtarget=x86-windows

[leaden trail]

ig the program ur plugging into is 32-bit for some reason?

[round heart]

the provided .lib doesnt have 64 bit bindings

if i try to compile as -Dtarget=x86_64-windows it just doesnt

[leaden trail]

what the

huh???

that makes no sense

[round heart]

theres a tiny change its header mismatch
BYOND public API version 516.1651
whereas the server reports 516.1669

but surely a minor wouldnt break like this??

[leaden trail]

oh hang on I misread the asm

[round heart]

ill check if theres a newer versino and diff

[leaden trail]

is this the function ur exposing from zig? if so, can I see the zig-side definition?

wait, no, it can't be cuz it doesn't return the right type

can I see the fn line for the zig function in which this segfault occurs, as well as the c-side declaration of that function?

[round heart]

https://bpa.st/X2HA

1664 so not NEWEST newest but eh

[round heart]

?

[leaden trail]

so, the function in which this segfault occurs is called from some c code, right? what type does it say the function should have?

I want to double check that the way you're defining it in zig matches what the c code expects

[round heart]

SCHOOL INTERNET, PLEASE. I BEG.

[round heart]

if you're asking about the dll function thats benig called from the engine, .

[round heart]

if you mean bapi.getNull() its .

[round heart]

the diff is the wrong way around but the shittty router handling my connection will implode if i try to wgetpaste again

[leaden trail]

oops, missed that msg

[round heart]

no its zig-side only
getNull() just returns a struct as value

[leaden trail]

init is called from c tho right? and that's the function that's segfaulting, not getNull

what's the type the c code expects for init?

[round heart]

its called from the byond engine, declared in the dm language

call_ext(SANLIB, "byond:init")()

im merely provided the header and .lib to build with

i hate this internet i wrote this ages ago

[leaden trail]

uhhh, okay, can u give me an example of a c function called through this system that u know works?

like an example from the docs or smth ig

[round heart]

@leaden trail

ill check the docs

agony

https://www.byond.com/docs/ref/#/{{appendix}}/Byondapi

wont load for me though

but iirc theres an example in there

official ref

Thank you eduroam

OH you have to open the section manually apparebtlky

bigger example in call_ext doc

https://www.byond.com/docs/ref/#/proc/call_ext

@leaden trail

Ill b home in half an hour ISH, can hop in VC then

or just provide stuff way smootherly

[leaden trail]

ofc it's fucking eduroam 😭

eduroam networks always suck, which is weird because afaik they're run by the school/uni themselves, and eduroam in theory should only be handing auth?? but in my experience they're consistently awful

anyway

[leaden trail]

the example in the docs uses BYOND_EXPORT, how is that macro defined?

[leaden trail]

oh it's here, okay

declspec(dllimport) shouldn't affect ABI afaik so that's fine

[leaden trail]

btw this code never actually exports the init function, where does that happen?

[round heart]

Root. Zig

[leaden trail]

using @export?

[round heart]

SM like export const whatever = import.init

[round heart]

cant paste verbatim rn

[leaden trail]

oh uhhh, that's... I'm surprised that even runs tbh

[round heart]

Wasn't supposed to be a reply

[round heart]

Lmao

[leaden trail]

is there a reason u don't simply do export fn init where it's originally defined?

[round heart]

exports are strewn around the codebase and zig doesn't have multiroot

and they're not inclyded by default because dead code

[leaden trail]

so just reference them in a comptime block in root.zig

[round heart]

FTR id rather define as export where it isis and not do this crossrefing

[round heart]

WIL try

[leaden trail]

id recommend either doing that or using @export

exporting a const alias to a function is weird and unlikely to work reliably

it should probably be banned tbh ^^'

[round heart]

is @export also lazyincluded or can i do sm like

@export(thing)
fn thing() void {}

and not worry about root files?

*putting things IN root file

s

[round heart]

@leaden trail didnt help

root.zig

comptime {
    const core = @import("_exports/core.zig");
    @export(&core.init, .{ .name = "init" });
}

_exports/core.zig

pub fn init() callconv(.c) bapi.ByondValueRaw {
  [...]
}

[leaden trail]

@export goes in a comptime block. those aren't lazy, but files themselves are, so it still won't work unless the file is explicitly used in the module root

[round heart]

hold on

nop nvm

[leaden trail]

can u try with pub export fn init and _ = @import("exports/core.zig").init;?

doubt it'll change anything tbh

but worth a shot

[round heart]

kk

[leaden trail]

after that, I think it's probably time to start trying to create a minimal repro to pin down whether this actually is an abi issue

do u have any other of these functions that are called the same way? and if so, do any of them work?

[round heart]

?

[leaden trail]

if not, start by writing a little hello world or smth and see if that works

[round heart]

none that i got to yet

[leaden trail]

yea :)

[round heart]

i do know that calling its noreturn fn DOES work

but thats uhhh not desirable to have as return of everything

since it- hold on cp time

that works fine

nope still no worky

i'll try to replace it with a short noop in root

doesnt work

can i like.. throw a too-big struct at it through a pointer and see how much it memsets?

to tell if its a mismatch of sizes

WAIT okay this is schizophrenic give me a second ill try a thing

okay its not breaking where i hoped it wouldnt

good

wait im a dumbass

so i still dont know how to fix it

but i just realized i DO actually pass the byondvalues and not just cstrs

to dll functions

.@"/turf/stacked" = bapi.callGlobalByID(sref.byond.proc_text2path, &[_]bapi.ByondValueRaw{bapi.writeStr("/turf/stacked").inner}),

that gives and returns bvals

i'll bitcast that and print whats in there

huh

okay soeven if i replace the bval with one double the intended size it doesnt crash (on lib calls)

i'll check how many bits it writes to

FFFFFFFFFFFFFFFF00000000FFFFFF00 if told to clear a u128

so does match the expected layout

i'll just redefine the fields as u32

since the header already defines these as "n chars of size"

[round heart]

MOTHERFUCKER

ok now it works

somehow

pub export fn init() callconv(.c) u64 {
    return 0;

how DO extern structs work even

i hope its not adding padding behind my back

replacing extern struct with packed struct works.. okay

it does prohibit me from using extern unions in it though

yep its extern

@leaden trail changing this from extern to packed magically fixes everything

similarly reverting re-crashes even if i nuke the inner union

[leaden trail]

yikes

probably an msvc moment

if u feel like trying to make a repro it's probably worth reporting

a c program that calls a zig function that returns a struct is probably enough to repro it? idk

[round heart]

sadly dont know enough c for that

nor just.. how dlls work in general lol

[leaden trail]

literally just c struct foo { // fields go here }; struct foo zigFn(); int main() { struct foo x = zigFn(); return 0; }

I expect DLLs don't have to be involved, u should be able to just add the c file to ur build command and have it statically link

[round heart]

hm, i'll try

i find it funny though how its not ME thats at fault or even the engine i work with

but the language i write in itself

[round heart]

@leaden trail ?

oh right build.zig

[round heart]

nice

i did my repro!

probably

nvm it breaks regardless UGHHHH give me a sec

[leaden trail]

tbf this is likely msvc being weird

oh wait actually

[round heart]

#1425901317884350567

[leaden trail]

what happens if u target x86-windows-msvc?

[round heart]

checking rn

[leaden trail]

it's possible the msvc ABI is different from mingw; zig targets mingw by default iirc

so when linking to stuff built with msvc u gotta explicitly specify that

[leaden trail]

uhhhh yeah u will need msvc installed

[round heart]

😭

thank u john gentoo for --oneshot

okay gentoo doesnt have msvc this is getting annoying UGHHHHH gcc can you mimic

oh right we're doing zig

uhhhhhh

can you just like run this for me and send back the result file lmao

const thing = extern struct{
  type: u8,
  junk1: u8 = undefined,
  junk2: u8 = undefined,
  junk3: u8 = undefined,
  data: u32,
};

pub export fn init() callconv(.c) thing {
  return .{ .type = 0x2A, .data = 42 };
}

@leaden trail ?

unless you're not on pc or w/e

[leaden trail]

I don't have msvc either

I don't do much windows dev

if ur on Linux u may have to install it in wine or smth

[round heart]

pain

[dawn cedar]

I'm on Windows with MSVC installed if you need something built? Although I'm not sure exactly what

[round heart]

i need a .dll that exports.. that chunk of code 6 messages up

to check if its msvc playing tricks on me with externs

because on -Dtarget=x86-windows, that piece of code crashes, but replacing the extern with a packed magically fixes stuff

which it shouldnt since theres no reason for padding here

const std = @import("std");

pub fn build(b: *std.Build) void {
    const target = b.standardTargetOptions(.{});
    const optimize = b.standardOptimizeOption(.{});

    const root = b.addModule("root", .{
        .root_source_file = b.path("src/root.zig"),
        .target = target,
        .optimize = optimize,
    });

    const lib = b.addLibrary(.{
        .name = "bepis",
        .root_module = root,
        .linkage = .dynamic,
    });
    b.installArtifact(lib);
}

i think thats all u need for build.zig

[dawn cedar]

Built with -Dtarget=x86-windows-msvc

(This was with 0.15.1, if that matters)

[round heart]

danke shoo

ok thank god it also breaks

@dawn cedar your contributions to the field of science will not be forgotten

[round heart]

for any newcomers
that crashes
but returning a u64 or a packed doesnt

[winter bear]

Do you know the definition of thing on the other side of the ABI boundary. The thread is quite big, but I understand you are communicating with external DLL

[round heart]

buried at the start of thread

[round heart]

.

[winter bear]

this one?

[round heart]

ye

[winter bear]

are you sure the padding is actually padding, you are setting things to undefined

packed structs dont' really support undefined fields (they actually get zeroed or something when you assign to other fields)

[keen seal]

I should note that all structures in C generally do the equivent of Zig's extern struct.
packed struct is a very different thing entirely.

[winter bear]

heck packed struct can give garbage data to the DLL as well because the layout is completely different

[round heart]

its defined in header as "junk" and similarly throwingi t at a "clear this struct out" function of the provided supplementary also leaves these be

u1c junk1, junk2, junk3; //!< padding

[winter bear]

yeah but is this reverse engineered stuff? It might be they just don't know what it is

[round heart]

officially provided header file

[winter bear]

even more scary lol

[keen seal]

Some static_asserts on the C side could be useful

Indeed, likewise on the Zig side

[round heart]

🤷‍♂️ bug the engine dev

[keen seal]

Currently it's bugging you lol

[winter bear]

DId you try setting those junk things to 0 anyways?

[keen seal]

Assert oriented programming

[round heart]

i mean i can try i guess

[round heart]

. aslisted here it ignores these anyways

but maybe externs leave holes in allocation or whatever lol

[winter bear]

idk anything about byond, but it could be they do touch the junks or use them for something internally or it's reserved space and ByondValueTYpe is actually u32 or something

in which case if they are not set 0 you get some humongous value as type

[round heart]

i mean at that point we're entering "dont trust the header" territory

and thats a whole other can of worms

[winter bear]

well that's everytime you have to deal with properitary binary ABI

[round heart]

crashes

[winter bear]

https://docs.rs/byondapi-sys/latest/byondapi_sys/struct.CByondValue.html this rust binding is equal to your extern struct, so it should work

[round heart]

well, again, it doesnt
im not really sure why are we snooping for issues with the structure itself when ive already said packeds just work

just having it as extern doesnt

[winter bear]

What I'm saying the packed probably doesn't actually work

it's just workarounding whatever is your real issue by luck

[round heart]

im not really sure how much luck there is to be had here
its not like im sending garbage data in and it freaking out over that, if i do try to send an invalid ref in, it handles that

bad ref error in log

and besides @winter bear didnt you say that packed was suppsoed to be identical in layout to extern?

in this specific case i mean

if the layout doesnt change and the data doesnt change, then what does?\

[winter bear]

In this case they should be equivalent yes, which I find strange. Extern struct is what you should use, but either it gets compiled wrong for some reason or there's mismatching ABI.

you can even check / assert this by creating same variable using both packed and extern and doing this:

std.mem.eql(u8, std.mem.asBytes(&b), std.mem.asBytes(&a));

do you compile to 32 bit by chance?

[round heart]

-Dtarget=x86-windows

no other bindings provided

gtg for a bit, theres a tiny repro which annoingly crashes regardless in #1425901317884350567

[winter bear]

which zig version are you using as well?

[round heart]

newest

[true basalt]

"newset" meaning tip of zig main (for anything sensical give the commit), or "newest" tagged release? (which next week will be something different, rendering your thread answer obsolete if you don't actually name the version.

[round heart]

1.15.1

[true basalt]

1.15.1 doesn't exist. You must mean 0.15.1