Skip to content

Commit 719ae83

Browse files
Fix byte-related LLVM segfaults (#785)
Closes #783. And closes #778. I believe the way we currently handle size calculations for stack pushes/pops of non-structural types has some flaws for non-pointer values. Some changes in how we use the GEP instruction will probably fix this, but I find its syntax really confusing. I have a proof-of-concept alternate fix at the end. To bypass the calculation issues, we can also just change the size of a byte to 8 byte instead. This *hotfix* fixes the segfault in the original issue as well as all of my minified versions. Some additional explanations of the issue using an example program: ```scala def foo { action: Byte => Unit }: Unit = action(42.toByte) def main() = { foo { b => // does not segfault when printlns are swapped println(42) println(b) } } ``` <details> <summary>Pseudo-flow with relevant (for the segfault) lines marked</summary> ``` main(stack) main(stack): alloc(3 * 8 = 24) push returnAddress_1 push sharer_5 push eraser_7 blockLit = vtable[blockLit_3289_clause_13] foo(blockLit, stack) foo(blockLit, stack): alloc(3 * 8 = 24) push returnAddress_40 push sharer_5 push eraser_7 literal = 42 byte = toByte(literal) vtable = blockLit[0] closure = blockLit[1] fp = *vtable // = blockLit_3289_clause_13 fp(closure, byte, stack) blockLit_3289_clause_13(closure, byte, stack): alloc(3 * 8 + 1 = 25) // <--------------------------- push byte push returnAddress_14 push sharer_26 push eraser_30 literal = 42 printlnInt(literal, stack) printlnInt(value, stack): // _4 str = show(value) unit = println(str) dealloc(3 * 8 = 24) pop _ pop _ pop returnAddress // _14 returnAddress(unit, stack) // <--------------------------- returnAddress_14(ret, stack): dealloc(1) pop byte erase(ret) alloc(3 * 8 = 24) push returnAddress_17 push sharer_5 push eraser_7 printlnByte(byte, stack) printlnByte(byte, stack): str = show(byte) unit = println(str) dealloc(24) pop _ pop _ pop returnAddress // _17 returnAddress(unit, stack) returnAddress_17(unit, stack): dealloc(24) pop _ pop _ pop returnAddress // _40 returnAddress(unit, stack) returnAddress_40(unit, stack) dealloc(24) pop _ pop _ pop returnAddress // _1 returnAddress(unit, stack) returnAddress_1(unit, stack): dealloc(24) pop _ pop _ pop returnAddress returnAddress(unit, stack) ``` </details> The corresponding LLVM code of the relevant sections: ```llvm define tailcc void @blockLit_3289_clause_13(%Object %closure, i8 %b_2365, %Stack %stack) { entry: ; new blockLit_3289_clause_13, 1 parameters %stackPointer_33 = call ccc %StackPointer @stackAllocate(%Stack %stack, i64 25) ; <--------------- %b_2365_pointer_34 = getelementptr {i8}, %StackPointer %stackPointer_33, i64 0, i32 0 store i8 %b_2365, ptr %b_2365_pointer_34, !noalias !2 %returnAddress_pointer_35 = getelementptr {{i8}, %FrameHeader}, %StackPointer %stackPointer_33, i64 0, i32 1, i32 0 %sharer_pointer_36 = getelementptr {{i8}, %FrameHeader}, %StackPointer %stackPointer_33, i64 0, i32 1, i32 1 %eraser_pointer_37 = getelementptr {{i8}, %FrameHeader}, %StackPointer %stackPointer_33, i64 0, i32 1, i32 2 store ptr @returnAddress_14, ptr %returnAddress_pointer_35, !noalias !2 store ptr @sharer_26, ptr %sharer_pointer_36, !noalias !2 store ptr @eraser_30, ptr %eraser_pointer_37, !noalias !2 ; literalInt longLiteral_3291, n=42 %longLiteral_3291 = add i64 42, 0 ; substitution ; substitution [value_3 !-> longLiteral_3291] ; jump println_4 musttail call tailcc void @println_4(i64 %longLiteral_3291, %Stack %stack) ret void } ... define tailcc void @println_4(i64 %value_3, %Stack %stack) { entry: ; definition println_4, environment length 1 ; foreignCall pureApp_3284 : String(), foreign show_14, 1 values %pureApp_3284 = call ccc %Pos @show_14(i64 %value_3) ; foreignCall pureApp_3283 : Positive(), foreign println_1, 1 values %pureApp_3283 = call ccc %Pos @println_1(%Pos %pureApp_3284) ; substitution ; substitution [v_r_3164_3278 !-> pureApp_3283] ; return, 1 values %stackPointer_56 = call ccc %StackPointer @stackDeallocate(%Stack %stack, i64 24) %returnAddress_pointer_57 = getelementptr %FrameHeader, %StackPointer %stackPointer_56, i64 0, i32 0 %returnAddress_55 = load %ReturnAddress, ptr %returnAddress_pointer_57, !noalias !2 musttail call tailcc void %returnAddress_55(%Pos %pureApp_3283, %Stack %stack) ; <------------------------- ; here is the segfault, since returnAddress_55 is missing one byte! ret void } ``` --- A fix I came up with that does not involve making the size of a byte large: ``` llvm define tailcc void @blockLit_3289_clause_13(%Object %closure, %Byte %b_2365, %Stack %stack) { entry: ; new blockLit_3289_clause_13, 1 parameters ; SPLIT the allocation into two segments <----------------------- %stackPointar_33 = call ccc %StackPointer @stackAllocate(%Stack %stack, i64 1) %b_2365_pointer_34 = getelementptr %Byte, %StackPointer %stackPointar_33, i64 0 store %Byte %b_2365, ptr %b_2365_pointer_34, !noalias !2 %stackPointer_33 = call ccc %StackPointer @stackAllocate(%Stack %stack, i64 24) %returnAddress_pointer_35 = getelementptr %FrameHeader, %StackPointer %stackPointer_33, i64 0, i32 0 %sharer_pointer_36 = getelementptr %FrameHeader, %StackPointer %stackPointer_33, i64 0, i32 1 %eraser_pointer_37 = getelementptr %FrameHeader, %StackPointer %stackPointer_33, i64 0, i32 2 store ptr @returnAddress_14, ptr %returnAddress_pointer_35, !noalias !2 store ptr @sharer_26, ptr %sharer_pointer_36, !noalias !2 store ptr @eraser_30, ptr %eraser_pointer_37, !noalias !2 ; literalInt longLiteral_3291, n=42 %longLiteral_3291 = add i64 42, 0 ; substitution ; substitution [value_3 !-> longLiteral_3291] ; jump println_4 musttail call tailcc void @println_4(i64 %longLiteral_3291, %Stack %stack) ret void } ... ``` --------- Co-authored-by: Philipp Schuster <[email protected]>
1 parent 9747e03 commit 719ae83

File tree

6 files changed

+6
-14
lines changed

6 files changed

+6
-14
lines changed

effekt/jvm/src/test/scala/effekt/StdlibTests.scala

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ class StdlibLLVMTests extends StdlibTests {
4141
override def debug = sys.env.get("EFFEKT_DEBUG").nonEmpty
4242

4343
override def ignored: List[File] = List(
44-
// segfaults
45-
examplesDir / "stdlib" / "stream" / "fuse_newlines.effekt",
46-
4744
// Syscall param write(buf) points to uninitialised byte(s)
4845
examplesDir / "stdlib" / "io" / "filesystem" / "files.effekt",
4946
examplesDir / "stdlib" / "io" / "filesystem" / "async_file_io.effekt",

effekt/shared/src/main/scala/effekt/generator/llvm/Transformer.scala

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,6 @@ object Transformer {
415415
case machine.Type.Int() => IntegerType64()
416416
case machine.Type.Byte() => IntegerType8()
417417
case machine.Type.Double() => DoubleType()
418-
case machine.Type.String() => positiveType
419418
case machine.Type.Reference(tpe) => referenceType
420419
}
421420

@@ -429,9 +428,8 @@ object Transformer {
429428
case machine.Type.Prompt() => 8 // TODO Make fat?
430429
case machine.Type.Stack() => 8 // TODO Make fat?
431430
case machine.Type.Int() => 8 // TODO Make fat?
432-
case machine.Type.Byte() => 1
431+
case machine.Type.Byte() => 8
433432
case machine.Type.Double() => 8 // TODO Make fat?
434-
case machine.Type.String() => 16
435433
case machine.Type.Reference(_) => 16
436434
}
437435

@@ -666,7 +664,6 @@ object Transformer {
666664
case machine.Positive() => Call("_", Ccc(), VoidType(), sharePositive, List(transform(value)))
667665
case machine.Negative() => Call("_", Ccc(), VoidType(), shareNegative, List(transform(value)))
668666
case machine.Type.Stack() => Call("_", Ccc(), VoidType(), shareResumption, List(transform(value)))
669-
case machine.Type.String() => Call("_", Ccc(), VoidType(), shareString, List(transform(value)))
670667
}.map(emit)
671668
}
672669

@@ -675,7 +672,6 @@ object Transformer {
675672
case machine.Positive() => Call("_", Ccc(), VoidType(), erasePositive, List(transform(value)))
676673
case machine.Negative() => Call("_", Ccc(), VoidType(), eraseNegative, List(transform(value)))
677674
case machine.Type.Stack() => Call("_", Ccc(), VoidType(), eraseResumption, List(transform(value)))
678-
case machine.Type.String() => Call("_", Ccc(), VoidType(), eraseString, List(transform(value)))
679675
}.map(emit)
680676
}
681677

@@ -703,14 +699,12 @@ object Transformer {
703699
val shareNegative = ConstantGlobal("shareNegative");
704700
val shareResumption = ConstantGlobal("shareResumption");
705701
val shareFrames = ConstantGlobal("shareFrames");
706-
val shareString = ConstantGlobal("sharePositive");
707702

708703
val eraseObject = ConstantGlobal("eraseObject");
709704
val erasePositive = ConstantGlobal("erasePositive");
710705
val eraseNegative = ConstantGlobal("eraseNegative");
711706
val eraseResumption = ConstantGlobal("eraseResumption");
712707
val eraseFrames = ConstantGlobal("eraseFrames");
713-
val eraseString = ConstantGlobal("erasePositive");
714708

715709
val alloc = ConstantGlobal("alloc")
716710
val getPointer = ConstantGlobal("getPointer")

effekt/shared/src/main/scala/effekt/machine/PrettyPrinter.scala

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ object PrettyPrinter extends ParenPrettyPrinter {
3232
case Type.Int() => "Int"
3333
case Type.Byte() => "Byte"
3434
case Type.Double() => "Double"
35-
case Type.String() => "String"
3635
case Type.Reference(tpe) => toDoc(tpe) <> "*"
3736
}
3837

effekt/shared/src/main/scala/effekt/machine/Transformer.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ object Transformer {
390390
}
391391

392392
case core.Literal(javastring: String, _) =>
393-
val literal_binding = Variable(freshName("utf8StringLiteral"), Type.String());
393+
val literal_binding = Variable(freshName("utf8StringLiteral"), builtins.StringType);
394394
Binding { k =>
395395
LiteralUTF8String(literal_binding, javastring.getBytes("utf-8"), k(literal_binding))
396396
}
@@ -472,7 +472,7 @@ object Transformer {
472472
case core.Type.TByte => Type.Byte()
473473
case core.Type.TBoolean => builtins.BooleanType
474474
case core.Type.TDouble => Type.Double()
475-
case core.Type.TString => Type.String()
475+
case core.Type.TString => Positive()
476476
case core.ValueType.Data(symbol, targs) => Positive()
477477
}
478478

effekt/shared/src/main/scala/effekt/machine/Tree.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,6 @@ enum Type {
222222
case Int()
223223
case Byte()
224224
case Double()
225-
case String()
226225
case Reference(tpe: Type)
227226
}
228227
export Type.{ Positive, Negative }
@@ -241,5 +240,7 @@ object builtins {
241240
val False: Tag = 0
242241
val BooleanType = Positive()
243242

243+
val StringType = Positive()
244+
244245
val SingletonRecord: Tag = 0
245246
}

libraries/llvm/main.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,5 @@ int main(int argc, char *argv[]) {
4040
uv_loop_t *loop = uv_default_loop();
4141
uv_run(loop, UV_RUN_DEFAULT);
4242
uv_loop_close(loop);
43+
return 0;
4344
}

0 commit comments

Comments
 (0)