Invalid read on 32-bits introduced by tupocolypse #11003
Comments
JeffBezanson
added
bug
|
I'm not sure if this will provide any clue, but I get the invalid read the first two times this is called, but never on any calls after the second time. This seems to be a minimal test case, too -- 1D arrays don't do it, and passing an integer as one argument to |
|
Comparing two matrices also gives the invalid read. And again, the warning happens precisely twice if the comparison is called repeatedly. I am able to get this warning up to two times, and then if I run the |
|
Got a 32-bit container and got vgdb working.... here's the backtrace. |
|
@Keno This is what I get following your advice. declare i1 @"julia_==_43936"([2 x i32], [2 x i32]) |
|
And the command on the gdb side |
|
Also, |
|
I assume that function is JITted? If so, you may have to enable KEEP_BODIES in |
|
Actually running from gdb directly works (might has sth to do with where it is called)..... |
|
Result from |
|
And yes it is JITed (otherwise trying to print a AFAICT this function shouldn't have any allocation and/or gc (didn't see any call to |
|
I wouldn't be surprised if something got miscompiled. Can you post the disassembly as well? I'll have a look tomorrow evening. |
|
I've just closed the container. Will post that later. |
|
These should be all what you want (you probably don't need all of them but it will take much longer to get them again...) All of these are obtained in the same session so the addresses in the backtrace/dump points directly into the disassemble from |
|
valgrind also reports a number of "Conditional jump or move depends on uninitialized value(s)" in llvm or llvm related paths and (the first warning from valgrind) Not sure if it matters. llvm is stock 3.6.0-5 from ArchLinux repo (both 32 and 64 bit). On 32bit, julia is compiled with |
|
Hmm, the problem is that one of the SSA variables gets spilled to a stack slot and then stack restore moves the stack pointer above it, causing valgrind to think we're accessing in invalid memory. I'm not sure if it's legal to carry SSA values across stackrestore, but if it is, I don't think there's anything we can do. |
|
(Or more precisely whether it's legal to use SSA values declared after stacksave). |
|
Does this show up only on 32 bits simply because there are fewer registers there to work with? |
|
Yes. I'm still debating whether there is actually something that needs to be fixed. Will do some experiments. |
|
@Keno Correct me if I'm wrong. Does this mean that if a signal handler kicks in at the right time, it can corrupt the stack variable? |
|
@Keno Any update. |
|
I inquired with the LLVM folks and this may actually be a legitimate bug. I haven't gotten around to investigating further though. |
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
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
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
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
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
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
|
Thanks @vtjnash for (quite possibly) fixing this! I'm trying to confirm that this actually fixes the invalid reads but am unable now to build 32-bit julia on my 64-bit machine. My and I believe this is due to a change in the build system (not a change on my computer), but I won't have time to fully investigate for a few weeks due to vacation. The file |
|
cmake handles cross-compilation particularly badly. I usually just set |
|
Thanks @vtjnash, everything seems to work correctly now. |
|
just with the build, or valgrind too? |
|
With valgrind too. |
|
Thanks! |
This particular issue is largely debian's fault for not allowing you to install i386 libraries without uninstalling large numbers of x86_64 libraries first. edit: hm, @garrison might not be using Debian if he was able to get openssl:i386 installed without tons of conflicts, but cmake's Also to nitpick on terminology, this isn't a cross-compile, it's multiarch. If we were using a true cross-compiler prefix |
|
@tkelman I am using Debian jessie, and overall I've had very few issues with multiarch once I figured out the right settings for |
|
The issue here lies with I think we'd have to come up with a dummy test library and use |
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
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
Since the merging of the tuple type redesign (#10380), valgrind (with
-DMEMDEBUG) frequently reports invalid reads on 32-bits.Here's a minimal test case:
complex([1 2; 3 4], [5 6; 7 8]).The text was updated successfully, but these errors were encountered: