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

PkgId objects change by printing and returning from function #54599

Closed
lgoettgens opened this issue May 28, 2024 · 2 comments · Fixed by #54604
Closed

PkgId objects change by printing and returning from function #54599

lgoettgens opened this issue May 28, 2024 · 2 comments · Fixed by #54604
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@lgoettgens
Copy link
Contributor

lgoettgens commented May 28, 2024

Running the following prints the UUID of Aqua as expected (on 1.10.3, 1.11.0-beta1, and b54dce2 (few days old master)):

using Pkg
Pkg.activate(; temp=true)
Pkg.add(["Aqua"])
Pkg.instantiate()

function foo()
    pkgid = Base.identify_package("Aqua")
    return pkgid
end

println(foo().uuid)

However, if we move the printing into the function, makes the code behave differently in different julia versions:

using Pkg
Pkg.activate(; temp=true)
Pkg.add(["Aqua"])
Pkg.instantiate()

function foo()
    pkgid = Base.identify_package("Aqua")
    println(pkgid.uuid)
end

foo()

output 1.10.3:

4c88cf16-eb10-579e-8560-4a9242c79595

output 1.11.0-beta1:

ERROR: LoadError: type Nothing has no field uuid
Stacktrace:
 [1] getproperty
   @ ./Base.jl:49 [inlined]
 [2] foo()
   @ Main ~/code/julia/julia/foo.jl:9
 [3] top-level scope
   @ ~/code/julia/julia/foo.jl:13

output b54dce2 (few days old master):

ERROR: LoadError: type Nothing has no field uuid
Stacktrace:
 [1] getproperty
   @ ./Base.jl:49 [inlined]
 [2] foo()
   @ Main ~/code/julia/julia/foo.jl:9
 [3] top-level scope
   @ ~/code/julia/julia/foo.jl:13
 [4] include
   @ ./Base.jl:559 [inlined]
 [5] exec_options(opts::Base.JLOptions)
   @ Base ./client.jl:325
 [6] _start()
   @ Base ./client.jl:533

If we add another print in there, the LoadError transforms into a segfault:

using Pkg
Pkg.activate(; temp=true)
Pkg.add(["Aqua"])
Pkg.instantiate()

function foo()
    pkgid = Base.identify_package("Aqua")
    println(pkgid)
    println(pkgid.uuid)
end

foo()

output 1.10.3:

Aqua [4c88cf16-eb10-579e-8560-4a9242c79595]
4c88cf16-eb10-579e-8560-4a9242c79595

output 1.11.0-beta1:

Base.PkgId(Base.UUID("4c88cf16-eb10-579e-8560-4a9242c79595"), "Aqua")

[925569] signal 11 (128): Segmentation fault
in expression starting at /home/lgoe/code/julia/julia/foo.jl:13
foo at /home/lgoe/code/julia/julia/foo.jl:6
unknown function (ip: 0x7417e9b3158f)
unknown function (ip: (nil))
Allocations: 6884270 (Pool: 6882848; Big: 1422); GC: 8
[1]    925569 segmentation fault (core dumped)  julia +1.11 foo.jl

output b54dce2 (few days old master):

Base.PkgId(Base.UUID("4c88cf16-eb10-579e-8560-4a9242c79595"), "Aqua")

[925819] signal 11 (128): Segmentation fault
in expression starting at /home/lgoe/code/julia/julia/foo.jl:13
foo at /home/lgoe/code/julia/julia/foo.jl:6
unknown function (ip: 0x7d7afbb2adff)
unknown function (ip: (nil))
Allocations: 4446232 (Pool: 4444892; Big: 1340); GC: 6
[1]    925819 segmentation fault (core dumped)  julia +nightly foo.jl

some device information:

Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, skylake)

all three julia versions are installed via juliaup

cc @fingolfin

@fingolfin fingolfin added the bug Indicates an unexpected problem or unintended behavior label May 28, 2024
@fingolfin
Copy link
Contributor

I see that @gbaraldi has a fix for this, great! In case it helps further: git bisect pinpoints the start of this to 00cbe4a from PR #53326 by @vtjnash but of course that change may have only exposed an instance of an already existing condition.

@fingolfin
Copy link
Contributor

Looking at the proposed fix it seems Julia is generating invalid code here, ouch

oscardssmith pushed a commit that referenced this issue Jun 5, 2024
We currently cause a alias analysis contradiction by saying that the
unionselbytes are on the stack, even if they are on a struct. LLVM is
then able to figure out that we giving it a impossible alias situation
(the object doesn't alias itself) and triggers UB.

https://godbolt.org/z/ssEKMzsPf

We may want to do a benchmarks run on this to see if anything too
critical hasn't regressed.

Fixes #54599

---------

Co-authored-by: Cody Tapscott <84105208+topolarity@users.noreply.github.com>
KristofferC pushed a commit that referenced this issue Jun 7, 2024
We currently cause a alias analysis contradiction by saying that the
unionselbytes are on the stack, even if they are on a struct. LLVM is
then able to figure out that we giving it a impossible alias situation
(the object doesn't alias itself) and triggers UB.

https://godbolt.org/z/ssEKMzsPf

We may want to do a benchmarks run on this to see if anything too
critical hasn't regressed.

Fixes #54599

---------

Co-authored-by: Cody Tapscott <84105208+topolarity@users.noreply.github.com>
(cherry picked from commit 30542e0)
KristofferC pushed a commit that referenced this issue Jun 7, 2024
We currently cause a alias analysis contradiction by saying that the
unionselbytes are on the stack, even if they are on a struct. LLVM is
then able to figure out that we giving it a impossible alias situation
(the object doesn't alias itself) and triggers UB.

https://godbolt.org/z/ssEKMzsPf

We may want to do a benchmarks run on this to see if anything too
critical hasn't regressed.

Fixes #54599

---------

Co-authored-by: Cody Tapscott <84105208+topolarity@users.noreply.github.com>
(cherry picked from commit 30542e0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants