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

size_t wrongly translated to Cint #417

Closed
SimonDanisch opened this issue Mar 2, 2023 · 2 comments
Closed

size_t wrongly translated to Cint #417

SimonDanisch opened this issue Mar 2, 2023 · 2 comments

Comments

@SimonDanisch
Copy link
Collaborator

Clang version:
v0.17.2

c header:

  extern RPR_API_ENTRY rpr_status rprContextCreateMesh(rpr_context context, rpr_float const * vertices, size_t num_vertices, rpr_int vertex_stride, rpr_float const * normals, size_t num_normals, rpr_int normal_stride, rpr_float const * texcoords, size_t num_texcoords, rpr_int texcoord_stride, rpr_int const * vertex_indices, rpr_int vidx_stride, rpr_int const * normal_indices, rpr_int nidx_stride, rpr_int const * texcoord_indices, rpr_int tidx_stride, rpr_int const * num_face_vertices, size_t num_faces, rpr_shape * out_mesh);

julia translation:

function rprContextCreateMesh(context, vertices, num_vertices, vertex_stride, normals, num_normals, normal_stride, texcoords, num_texcoords, texcoord_stride, vertex_indices, vidx_stride, normal_indices, nidx_stride, texcoord_indices, tidx_stride, num_face_vertices, num_faces, out_mesh)
    ccall((:rprContextCreateMesh, libRadeonProRender64), rpr_status, (rpr_context, Ptr{rpr_float}, Cint, rpr_int, Ptr{rpr_float}, Cint, rpr_int, Ptr{rpr_float}, Cint, rpr_int, Ptr{rpr_int}, rpr_int, Ptr{rpr_int}, rpr_int, Ptr{rpr_int}, rpr_int, Ptr{rpr_int}, Cint, Ptr{rpr_shape}), context, vertices, num_vertices, vertex_stride, normals, num_normals, normal_stride, texcoords, num_texcoords, texcoord_stride, vertex_indices, vidx_stride, normal_indices, nidx_stride, texcoord_indices, tidx_stride, num_face_vertices, num_faces, out_mesh)
end

Generator code:

using Clang
using Clang.Generators

cd(@__DIR__)
# current version checked in is v2.2.9
include_dir = normpath(joinpath(@__DIR__, "RadeonProRenderSDK", "RadeonProRender", "inc"))
# LIBCLANG_HEADERS are those headers to be wrapped.
headers = joinpath.(include_dir, [
    "RadeonProRender_v2.h",
])

# wrapper generator options
options = load_options(joinpath(@__DIR__, "rpr.toml"))

# add compiler flags, e.g. "-DXXXXXXXXX"
args = get_default_args()
push!(args, "-I$include_dir")

ctx = create_context(headers, args, options)

# run generator
build!(ctx)

rpr.tom:

[general]
library_name = "libRadeonProRender64"
output_file_path = "../src/LibRPR.jl"
module_name = "RPR"
jll_pkg_name = "RadeonProRender_jll"
export_symbol_prefixes = ["RPR", "rpr"]

Repository: https://github.com/JuliaGraphics/RadeonProRender.jl/blob/master/build/generate-master.jl

The segfault resulting from it:
JuliaLang/julia#48865 (comment)

Am I doing something stupid, is the header file confusing Clang, or is Clang really mis-translating this?
Would be really surprising, since Clang is used to wrap so many projects in Julia?

@SimonDanisch
Copy link
Collaborator Author

Hm, seems to go wrong on Clangs side already?

args = Union{Expr, Symbol}[:rpr_context, :(Ptr{rpr_float}), :Cint, :rpr_int, :(Ptr{rpr_float}), :Cint, :rpr_int, :(Ptr{rpr_int}), :Cint, :rpr_int, :rpr_int, :(Ptr{Ptr{rpr_float}}), :(Ptr{Cint}), :(Ptr{rpr_int}), :(Ptr{rpr_int}), :rpr_int, :(Ptr{rpr_int}), :rpr_int, :(Ptr{Ptr{rpr_int}}), :(Ptr{rpr_int}), :(Ptr{rpr_int}), :Cint, :(Ptr{rpr_shape})]
  CLParmDecl a CLTypedef rpr_context context
  CLParmDecl a pointer to `CLType (CLTypedef) ` vertices
  CLParmDecl a CLInt int num_vertices
  CLParmDecl a CLTypedef rpr_int vertex_stride
  CLParmDecl a pointer to `CLType (CLTypedef) ` normals
  CLParmDecl a CLInt int num_normals
  CLParmDecl a CLTypedef rpr_int normal_stride
  CLParmDecl a pointer to `CLType (CLTypedef) ` perVertexFlag
  CLParmDecl a CLInt int num_perVertexFlags
  CLParmDecl a CLTypedef rpr_int perVertexFlag_stride
  CLParmDecl a CLTypedef rpr_int numberOfTexCoordLayers
  CLParmDecl a pointer to `CLType (CLPointer) ` texcoords
  CLParmDecl a pointer to `CLType (CLInt) ` num_texcoords
  CLParmDecl a pointer to `CLType (CLTypedef) ` texcoord_stride
  CLParmDecl a pointer to `CLType (CLTypedef) ` vertex_indices
  CLParmDecl a CLTypedef rpr_int vidx_stride
  CLParmDecl a pointer to `CLType (CLTypedef) ` normal_indices
  CLParmDecl a CLTypedef rpr_int nidx_stride
  CLParmDecl a pointer to `CLType (CLPointer) ` texcoord_indices
  CLParmDecl a pointer to `CLType (CLTypedef) ` tidx_stride
  CLParmDecl a pointer to `CLType (CLTypedef) ` num_face_vertices
  CLParmDecl a CLInt int num_faces
  CLParmDecl a pointer to `CLType (CLTypedef) ` out_mesh

See the CLParmDecl a CLInt int num_vertices ...
Maybe the included #include "cstddef" in RadeonProRender_v2.h is not resolved correctly?

@Gnimuc
Copy link
Member

Gnimuc commented Mar 11, 2023

To quote from Slack:

It seems that the original C header is not well formed.
The issue is fixed after I manually fixed the C header.
Not all compilation errors can lead to bugs.
If a type is translated mistakenly as int , then it’s almost 100% a compilation problem.

@Gnimuc Gnimuc closed this as completed Mar 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants