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

32bit memory error (uncovered with FixedPointNumbers) #11026

Closed
timholy opened this issue Apr 27, 2015 · 11 comments
Closed

32bit memory error (uncovered with FixedPointNumbers) #11026

timholy opened this issue Apr 27, 2015 · 11 comments
Labels
system:32-bit Affects only 32-bit systems

Comments

@timholy
Copy link
Member

timholy commented Apr 27, 2015

I'm just parroting @tkelman here (JuliaImages/Images.jl#284), but using FixedPointNumbers one can uncover some 32bit specific bugs. On 64bit, one gets this:

julia> using FixedPointNumbers

julia> Ufixed12[0.6 0.2; 1.4 0.8]
2x2 Array{FixedPointNumbers.UfixedBase{UInt16,12},2}:
 0.6  0.2
 1.4  0.8

On 32bit this either gives an InexactError or nonsense. However, scalars like Ufixed12(1.4) work fine.
@timholy
Copy link
Member Author

timholy commented Apr 27, 2015

Perhaps also related, however: JuliaMath/FixedPointNumbers.jl#15

@tkelman tkelman added the system:32-bit Affects only 32-bit systems label Apr 27, 2015
@timholy
Copy link
Member Author

timholy commented Apr 27, 2015

The bug is not present on 0.3

@tkelman
Copy link
Contributor

tkelman commented Apr 27, 2015

Based on the AppVeyor history for Images.jl https://ci.appveyor.com/project/timholy/images-jl/history it looks like it started a little less than 2 weeks ago? Am bisecting on 32-bit linux, should have more specifics soon.

@tkelman
Copy link
Contributor

tkelman commented Apr 27, 2015

Unsurprisingly, caused by #10380

tkelman@ygdesk:~/Julia/julia-linux32$ git bisect log
git bisect start
# bad: [982c60a71b02d30add6b983dbec88d09371494c4] Merge pull request #10974 from garrison/isdiag
git bisect bad 982c60a71b02d30add6b983dbec88d09371494c4
# good: [65ddb083c9a510b213f96397413c5f6d7732570b] Merge pull request #9468 from samoconnor/doc_notes_branch
git bisect good 65ddb083c9a510b213f96397413c5f6d7732570b
# good: [01a32160282917c389f3bd5a6e0ebc10d53989c6] Merge branch 'hayd-c2'
git bisect good 01a32160282917c389f3bd5a6e0ebc10d53989c6
# good: [ada46544e3016a247eff895f3ba66be91b9189bb] Fix #10829
git bisect good ada46544e3016a247eff895f3ba66be91b9189bb
# bad: [242b3d94d25ad36a1fbda07129869b9d92dbc4c7] Merge pull request #10923 from PalladiumConsulting/sg/codegen_warning
git bisect bad 242b3d94d25ad36a1fbda07129869b9d92dbc4c7
# bad: [2c9633e196bf2e3a40d91733f377ac13ed2d8f93] Merge pull request #10874 from JuliaLang/tk/raise-debugger-warning
git bisect bad 2c9633e196bf2e3a40d91733f377ac13ed2d8f93
# bad: [fb78bdb41ab2efaac32e87b85336e6871a6be8a8] fix a missing GC root
git bisect bad fb78bdb41ab2efaac32e87b85336e6871a6be8a8
# good: [bd748b973904fd23d43c7adf485cc2926f130dad] Merge branch 'amartgon-issues_#9664_#8416'
git bisect good bd748b973904fd23d43c7adf485cc2926f130dad
# good: [ba2b079523da4209fea7ada8572e4648c23f049d] NEWS updates and organization
git bisect good ba2b079523da4209fea7ada8572e4648c23f049d
# good: [d483948e5660f4e1bd84a2acb41ac7a8b4466f74] remove 2-argument `assert` performance trap
git bisect good d483948e5660f4e1bd84a2acb41ac7a8b4466f74
# bad: [4d1a4bfe153b9e072b49c445218c53ecec1ffb37] fix a missed jl_tuple_t in last rebase
git bisect bad 4d1a4bfe153b9e072b49c445218c53ecec1ffb37
# skip: [afd4eb038dcd9fa3e512509f4c332a9e7763ba59] redesign tuple types
git bisect skip afd4eb038dcd9fa3e512509f4c332a9e7763ba59
# only skipped commits left to test
# possible first bad commit: [4d1a4bfe153b9e072b49c445218c53ecec1ffb37] fix a missed jl_tuple_t in last rebase
# possible first bad commit: [afd4eb038dcd9fa3e512509f4c332a9e7763ba59] redesign tuple types

@JeffBezanson
Copy link
Member

Maybe related to #11003

@tkelman
Copy link
Contributor

tkelman commented May 14, 2015

This seems to have stopped happening on Linux recently, but it still happens on win32.

@timholy timholy mentioned this issue Jun 7, 2015
Merged
@tkelman
Copy link
Contributor

tkelman commented Jun 7, 2015

I can't seem to reproduce this any more on current master, but it may be obscured by a few other unresolved issues.

@tkelman
Copy link
Contributor

tkelman commented Jun 7, 2015

From a reverse-bisect, 5cb2835 caused this to stop happening. @mbauman does that make sense to you? We may just be avoiding the problematic code path now but that doesn't mean the underlying issue is necessarily solved.

At least reverting ebfebfd doesn't cause this one to come back, so it's unrelated to the other 32 bit problem from #10525.

@mbauman
Copy link
Member

mbauman commented Jun 7, 2015

Very strange. It doesn't really, besides the fact that these seem to be perturbation-sensitive LLVM codegen issues. It's not very surprising that a refactoring would change things.

@timholy
Copy link
Member Author

timholy commented Jun 7, 2015

This smells a bit like a dispatch/method priority problem. Don't know why it would manifest only on 32bit, however.

vtjnash added a commit that referenced this issue Jun 9, 2015
@vtjnash
store tuple and vector types to the stack eagerly [ci skip]
a34fae4 
fix #11187, fix #11450, fix #11026, ref #10525, fix #11003
TODO: confirm all of those numbers were fixed
TODO: ensure the lazy-loaded objects have gc-roots
TODO: re-enable VectorType objects, so small objects still end up in
registers in the calling convention
TODO: allow moving pointers sometimes rather than copying
TODO: teach the GC how it can re-use an existing pointer as a box

this also changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue
vtjnash added a commit that referenced this issue Jun 9, 2015
@vtjnash
store tuple and vector types to the stack eagerly [ci skip]
02e024f 
fix #11187, fix #11450, fix #11026, ref #10525, fix #11003
TODO: confirm all of those numbers were fixed
TODO: ensure the lazy-loaded objects have gc-roots
TODO: re-enable VectorType objects, so small objects still end up in
registers in the calling convention
TODO: allow moving pointers sometimes rather than copying
TODO: teach the GC how it can re-use an existing pointer as a box

this also changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc
vtjnash added a commit that referenced this issue Jun 9, 2015
@vtjnash
store tuple and vector types to the stack eagerly [ci skip]
244f1cb 
fix #11187, fix #11450, fix #11026, ref #10525, fix #11003
TODO: confirm all of those numbers were fixed
TODO: ensure the lazy-loaded objects have gc-roots
TODO: re-enable VectorType objects, so small objects still end up in
registers in the calling convention
TODO: allow moving pointers sometimes rather than copying
TODO: teach the GC how it can re-use an existing pointer as a box

this also changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc
vtjnash added a commit that referenced this issue Jun 11, 2015
@vtjnash
store tuple and vector types to the stack eagerly
727d2f7 
fix #11187 (pass struct and tuple objects by stack pointer)
fix #11450 (ccall emission was frobbing the stack)
likely may fix #11026 and may fix #11003 (ref #10525) invalid stack-read on 32-bit

this additionally changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc

this additionally prepares for turning back on allocating tuples as vectors,
since the gc now guarantees 16-byte alignment

future work this makes possible:
 - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure)
 - allow moving pointers sometimes rather than always copying immutable data
 - teach the GC how it can re-use an existing pointer as a box
vtjnash added a commit that referenced this issue Jun 12, 2015
@vtjnash
store tuple and vector types to the stack eagerly
4647931 
fix #11187 (pass struct and tuple objects by stack pointer)
fix #11450 (ccall emission was frobbing the stack)
likely may fix #11026 and may fix #11003 (ref #10525) invalid stack-read on 32-bit

this additionally changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc

this additionally prepares for turning back on allocating tuples as vectors,
since the gc now guarantees 16-byte alignment

future work this makes possible:
 - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure)
 - allow moving pointers sometimes rather than always copying immutable data
 - teach the GC how it can re-use an existing pointer as a box
vtjnash added a commit that referenced this issue Jun 16, 2015
@vtjnash
store tuple and vector types to the stack eagerly
dadf243 
fix #11187 (pass struct and tuple objects by stack pointer)
fix #11450 (ccall emission was frobbing the stack)
likely may fix #11026 and may fix #11003 (ref #10525) invalid stack-read on 32-bit

this additionally changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc

this additionally prepares for turning back on allocating tuples as vectors,
since the gc now guarantees 16-byte alignment

future work this makes possible:
 - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure)
 - allow moving pointers sometimes rather than always copying immutable data
 - teach the GC how it can re-use an existing pointer as a box
@timholy
Copy link
Member Author

timholy commented Jun 16, 2015

Nice!

fcard pushed a commit to fcard/julia that referenced this issue Jul 8, 2015
@vtjnash @fcard
store tuple and vector types to the stack eagerly
0d7a607 
fix JuliaLang#11187 (pass struct and tuple objects by stack pointer)
fix JuliaLang#11450 (ccall emission was frobbing the stack)
likely may fix JuliaLang#11026 and may fix JuliaLang#11003 (ref JuliaLang#10525) invalid stack-read on 32-bit

this additionally changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc

this additionally prepares for turning back on allocating tuples as vectors,
since the gc now guarantees 16-byte alignment

future work this makes possible:
 - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure)
 - allow moving pointers sometimes rather than always copying immutable data
 - teach the GC how it can re-use an existing pointer as a box
fcard pushed a commit to fcard/julia that referenced this issue Jul 8, 2015
@vtjnash @fcard
store tuple and vector types to the stack eagerly
ccd1428 
fix JuliaLang#11187 (pass struct and tuple objects by stack pointer)
fix JuliaLang#11450 (ccall emission was frobbing the stack)
likely may fix JuliaLang#11026 and may fix JuliaLang#11003 (ref JuliaLang#10525) invalid stack-read on 32-bit

this additionally changes the julia specSig calling convention to pass
non-primitive types by pointer instead of by-value

this additionally fixes a bug in gen_cfunction that could be exposed by
turning off specSig

this additionally moves the alloca calls in ccall (and other places) to
the entry BasicBlock in the function, ensuring that llvm detects them as
static allocations and moves them into the function prologue

this additionally fixes some undefined behavior from changing
a variable's size through a alloca-cast instead of zext/sext/trunc

this additionally prepares for turning back on allocating tuples as vectors,
since the gc now guarantees 16-byte alignment

future work this makes possible:
 - create a function to replace the jlallocobj_func+init_bits_value call pair (to reduce codegen pressure)
 - allow moving pointers sometimes rather than always copying immutable data
 - teach the GC how it can re-use an existing pointer as a box
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
system:32-bit Affects only 32-bit systems
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mbauman @JeffBezanson @timholy @tkelman