To explain a bit more what I meant by it being invalid, this line:

is taking something (presumably an integer?) emitted from a C library and then trying to interpret it as an SpglibReturnCode. But the implementation of SpglibReturnCode does not contain an integer, it actually contains a union, it basically looks like this:

struct SpglibReturnCode
    data::Union{Variant{:SUCCESS, (), Tuple{}}, Variant{:SPACEGROUP_SEARCH_FAILED, (), Tuple{}},  ...}
end

where each of those Variants are singleton structs. So you can't just take an integer and turn it into one of these. What you could do is something like

function SpglibReturnCode(i::Int)
    if i == 0
        SUCCESS
    elseif i == 1
        SPACEGROUP_SEARCH_FAILED
    elseif ...
        ...
     else
        error("invalid return code")
    end
end

and then you'd have a sum type you can work with I guess.

Many thanks Mason, and also @singularitti for providing this very useful package!

Thank you for reporting this issue @kbarros and thank you for your guidance @MasonProtter! I probably misunderstood how to use your package. However, I am not sure what the bug is: the code actually work for my tests. Could you shed light on me what specific example is breaking? @kbarros

Yeah, that's because julia appears to use an Int under the hood in its implementation of union types, but that's not a stable thing and it probably is causing undefined behaviour:

julia> let r = Ref(0)
           p::Ptr{SpglibReturnCode} = pointer_from_objref(r)
           unsafe_load(p)
       end
SUCCESS::SpglibReturnCode

julia> let r = Ref(1)
           p::Ptr{SpglibReturnCode} = pointer_from_objref(r)
           unsafe_load(p)
       end
SPACEGROUP_SEARCH_FAILED::SpglibReturnCode

julia> let r = Ref(2)
           p::Ptr{SpglibReturnCode} = pointer_from_objref(r)
           unsafe_load(p)
       end
CELL_STANDARDIZATION_FAILED::SpglibReturnCode

julia> let r = Ref(3)
           p::Ptr{SpglibReturnCode} = pointer_from_objref(r)
           unsafe_load(p)
       end
SYMMETRY_OPERATION_SEARCH_FAILED::SpglibReturnCode

This works locally, but it's definitely possible for this to cause problems in unexpected ways

We at DFTK.jl are also noticing crashes since upgrading Spglib to 0.9.3 (see JuliaMolSim/DFTK.jl#947 (comment)). So the issues is not specific to @kbarros 's example.

@singularitti, I have reached out on the Julia Slack server. Id be happy to work together with you to resolve this, since it might be affecting quite a lot of users.

This fork has contains the fix proposed by @MasonProtter: https://github.com/kbarros/Spglib.jl. Specifically, here is the revised function:

function get_error_code()
    # The return value for the C function `spg_get_error_code` is a C enum
    # called SpglibError. The C compiler guarantees that enums are internally
    # stored as type `int`; its size is "implementation dependent". Typically 4
    # or 8 bytes, but this could depend on the architecture and compiler details
    # https://stackoverflow.com/a/366033/500314. Julia provides `CInt` to
    # guarantee an integer that has the same size as the C `int` value. Manually
    # map this enum tag to variants of the `@enum SpglibReturnCode`. The latter
    # is defined as a Union using the Julia SumTypes library, and its
    # representation details are also implementation dependent (e.g., the "tag"
    # of this Union need not have the same size as a `CInt`). Note that a
    # mistake here could cause memory corruption, as discussed in
    # https://github.com/singularitti/Spglib.jl/issues/183.
    code = @ccall libsymspg.spg_get_error_code()::CInt
    if code == 0
        return SUCCESS
    elseif code == 1
        return SPACEGROUP_SEARCH_FAILED
    elseif code == 2
        return CELL_STANDARDIZATION_FAILED
    elseif code == 3
        return SYMMETRY_OPERATION_SEARCH_FAILED
    elseif code == 4
        return ATOMS_TOO_CLOSE
    elseif code == 5
        return POINTGROUP_NOT_FOUND
    elseif code == 6
        return NIGGLI_FAILED
    elseif code == 7
        return DELAUNAY_FAILED
    elseif code == 8
        return ARRAY_SIZE_SHORTAGE
    elseif code == 9
        return NONE
    end
end

CI was already not passing, but I verified that this change does not cause any new test failures. I can't reproduce the crash on my Mac.

It's not ready yet, I need to update other places where SpglibReturnCode is used.

Given that this package is only using these return codes to pass values back and forth from a C library, it seems better to just revert the change and use @enum, SumTypes doesn't appear to be adding any value

Agreed. Updated my fork to remove SumTypes and label return types explicitly. Please see: https://github.com/kbarros/Spglib.jl/blob/main/src/error.jl

Note that Spglib <= v0.9.2 also seemed to be dependent on undefined behavior (casting Cint into a Julia @enum). I think the correct thing is to explicitly mark the C enum as a Cint.

Thank you for your help @kbarros @MasonProtter! And I apologize all the trouble this bug have caused @CedricTravelletti. The only reason I want to use SumTypes.jl is that I want to switch to a mechanism where SpglibReturnCode is not dependent on number enumeration, but it seems that it is not what I have imagined. I will just revert it back or use @kbarros's PR immediately.

Testing the fork is tricky because the memory corruption only causes problems on certain machines (not my Mac, for example). We have just now tested the fork on a colleague's machine and verified that it seems to work. I will open a pull request. (Now opened: #184).

@CedricTravelletti would you like to test the fork before merging?