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

cconvert of map(pointer, String[...]) to Ptr{Ptr{Cchar}} fails on nightly #52025

Closed
vchuravy opened this issue Nov 4, 2023 · 1 comment · Fixed by #53659
Closed

cconvert of map(pointer, String[...]) to Ptr{Ptr{Cchar}} fails on nightly #52025

vchuravy opened this issue Nov 4, 2023 · 1 comment · Fixed by #53659
Assignees
@JeffBezanson
Milestone
1.11

Comments

@vchuravy
Copy link
Member

vchuravy commented Nov 4, 2023

Found during CI on https://github.com/cesmix-mit/LAMMPS.jl

 ERROR: LoadError: TypeError: in RefArray, in A, expected A<:(AbstractArray{Ptr{Int8}}), got Type{Vector{Ptr{UInt8}}}
Stacktrace:
 [1] Ref{Ptr{Int8}}(a::Vector{Ptr{UInt8}})
   @ Base ./refpointer.jl:158
 [2] cconvert(::Type{Ptr{Ptr{Int8}}}, a::Vector{Ptr{UInt8}})
   @ Base ./refpointer.jl:176
 [3] lammps_open_no_mpi(argc::Int64, argv::Vector{Ptr{UInt8}}, ptr::Ptr{Nothing})
   @ LAMMPS.API ~/work/LAMMPS.jl/LAMMPS.jl/src/api.jl:53
 [4] LMP(args::Vector{String}, comm::Nothing)
   @ LAMMPS ~/work/LAMMPS.jl/LAMMPS.jl/src/LAMMPS.jl:69

The ccall is generated by Clang.jl and takes as an argument a Ptr{Ptr{Cchar}}

https://github.com/cesmix-mit/LAMMPS.jl/blob/126d9711e37fef035d5c7327d1181daf12b7b22a/src/api.jl#L53

and while the argsv = map(pointer, args) should probably be inside the GC.@preserve

https://github.com/cesmix-mit/LAMMPS.jl/blob/126d9711e37fef035d5c7327d1181daf12b7b22a/src/LAMMPS.jl#L59

this used to work.
@KristofferC KristofferC added this to the 1.11 milestone Nov 4, 2023
@vtjnash
Copy link
Member

vtjnash commented Nov 4, 2023

Seems like it being a bit too strict now, while Int8 vs UInt8 would be an error in C, but usually it is allowed in Julia since the ABI is not affected by it (and the preserve and pointer calls are not necessary, as this call probably would have already worked if the user hadn't gone through that trouble)

@JeffBezanson JeffBezanson self-assigned this Mar 8, 2024
JeffBezanson added a commit that referenced this issue Mar 8, 2024
@JeffBezanson
fix #52025, re-allow cconvert between Int8 and UInt8 pointers
4a9d937
@JeffBezanson JeffBezanson mentioned this issue Mar 8, 2024
Merged
JeffBezanson added a commit that referenced this issue Mar 11, 2024
@JeffBezanson
fix #52025, re-allow all implicit pointer casts in cconvert for Array (…
f9e08f7 
…#53659)

fix #52025
KristofferC pushed a commit that referenced this issue Mar 15, 2024
@JeffBezanson
fix #52025, re-allow all implicit pointer casts in cconvert for Array (…
641ae2c 
…#53659)

fix #52025

(cherry picked from commit f9e08f7)
KristofferC added a commit that referenced this issue Mar 17, 2024
@KristofferC
Backports for 1.11.0-alpha2 (#53543)
150b895 
Backported PRs:
- [x] #39071 <!-- Add a lazy `logrange` function and `LogRange` type -->
- [x] #51802 <!-- Allow AnnotatedStrings in log messages -->
- [x] #53369 <!-- Orthogonalize re-indexing for FastSubArrays -->
- [x] #48050 <!-- improve `--heap-size-hint` arg handling -->
- [x] #53482 <!-- add IR encoding for EnterNode -->
- [x] #53499 <!-- Avoid compiler warning about redefining jl_globalref_t
-->
- [x] #53507 <!-- update staled `Core.Compiler.Effects` documentation
-->
- [x] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [x] #53523 <!-- add back an alias for `check_top_bit` -->
- [x] #53377 <!-- add _readdirx for returning more object info gathered
during dir scan -->
- [x] #53525 <!-- fix InteractiveUtils call in Base.runtests on failure
-->
- [x] #53540 <!-- use more efficient `_readdirx` for tab completion -->
- [x] #53545 <!-- use `_readdirx` for `walkdir` -->
- [x] #53551 <!-- revert "Add @create_log_macro for making custom styled
logging macros (#52196)" -->
- [x] #53554 <!-- Always return a value in 1-d circshift! of
abstractarray.jl -->
- [x] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [x] #53571 <!-- Update Documenter to v1.3 for inventory writing -->
- [x] #53403 <!-- Move parallel precompilation to Base -->
- [x] #53589 <!-- add back `unsafe_convert` to pointer for arrays -->
- [x] #53596 <!-- build: remove extra .a file -->
- [x] #53606 <!-- fix error path in `precompilepkgs` -->
- [x] #53004 <!-- Unexport with, at_with, and ScopedValue from Base -->
- [x] #53629 <!-- typo fix in scoped values docs -->
- [x] #53630 <!-- sroa: Fix incorrect scope counting -->
- [x] #53598 <!-- Use Base parallel precompilation to build stdlibs -->
- [x] #53649 <!-- precompilepkgs: package in boths deps and weakdeps are
in fact only weak -->
- [x] #53671 <!-- Fix bootstrap Base precompile in cross compile
configuration -->
- [x] #52125 <!-- Load Pkg if not already to reinstate missing package
add prompt -->
- [x] #53602 <!-- Handle zero on arrays of unions of number types and
missings -->
- [x] #53516 <!-- permit NamedTuple{<:Any, Union{}} to be created -->
- [x] #53643 <!-- Bump CSL to 1.1.1 to fix libgomp bug -->
- [x] #53679 <!-- move precompile workload back from Base -->
- [x] #53663 <!-- add isassigned methods for reinterpretarray -->
- [x] #53662 <!-- [REPL] fix incorrectly cleared line after completions
accepted -->
- [x] #53611 <!-- Linalg: matprod_dest for Diagonal and adjvec -->
- [x] #53659 <!-- fix #52025, re-allow all implicit pointer casts in
cconvert for Array -->
- [x] #53631 <!-- LAPACK: validate input parameters to throw informative
errors -->
- [x] #53628 <!-- Make some improvements to the Scoped Values
documentation. -->
- [x] #53655 <!-- Change tbaa of ptr_phi to tbaa_value  -->
- [x] #53391 <!-- Default to the medium code model in x86 linux -->
- [x] #53699 <!-- Move `isexecutable, isreadable, iswritable` to
`filesystem.jl` -->
- [x] #41232 <!-- Fix linear indexing for ReshapedArray if the parent
has offset axes -->
- [x] #53527 <!-- Enable analyzegc checks for try catch and fix found
issues -->
- [x] #52092 
- [x] #53682 <!-- Increase build precompilation -->
- [x] #53720 
- [x] #53553 <!-- typeintersect: fix `UnionAll` unaliasing bug caused by
innervars. -->

Contains multiple commits, manual intervention needed:
- [ ] #53305 <!-- Propagate inbounds in isassigned with CartesianIndex
indices -->

Non-merged PRs with backport label:
- [ ] #53736 <!-- fix literal-pow to return the right type when the base
is -1 -->
- [ ] #53707 <!-- Make ScopedValue public -->
- [ ] #53696 <!-- add invokelatest to on_done callback in bracketed
paste -->
- [ ] #53660 <!-- put Logging back in default sysimage -->
- [ ] #53509 <!-- revert moving "creating packages" from Pkg.jl -->
- [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #51928 <!-- Styled markdown, with a few tweaks -->
- [ ] #51816 <!-- User-themable stacktraces -->
- [ ] #51811 <!-- Make banner size depend on terminal size -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JeffBezanson JeffBezanson

Labels
None yet
Projects
None yet
Milestone
1.11
Development

Successfully merging a pull request may close this issue.

fix #52025, re-allow all implicit pointer casts in cconvert for Array JuliaLang/julia
4 participants
@vchuravy @vtjnash @JeffBezanson @KristofferC