Skip to content

Commit

Permalink
Fix buggy LLVM tests (#568)
Browse files Browse the repository at this point in the history
  • Loading branch information
b-studios authored Sep 13, 2024
2 parents 21343a7 + be43b58 commit 4e81699
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 48 deletions.
2 changes: 1 addition & 1 deletion effekt/jvm/src/main/scala/effekt/Runner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ object LLVMRunner extends Runner[String] {
val executableFile = basePath
var gccArgs = Seq(gcc, gccMainFile, "-o", executableFile, objPath) ++ libuvArgs

if (C.config.debug()) gccArgs ++= Seq("-fsanitize=address,undefined", "-fstack-protector-all", "-Og", "-g")
if (C.config.debug()) gccArgs ++= Seq("-fsanitize=address,undefined", "-fstack-protector-all", "-Og", "-g", "-Wall", "-Wextra")
if (C.config.valgrind()) gccArgs ++= Seq("-O0", "-g")

exec(gccArgs: _*)
Expand Down
29 changes: 6 additions & 23 deletions effekt/jvm/src/test/scala/effekt/LLVMTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,23 @@ class LLVMTests extends EffektTests {
)

lazy val bugs: List[File] = List(
examplesDir / "pos" / "issue108.effekt", // seg faults!

// names not sanitized (even?)
examplesDir / "pos" / "special_names.effekt",

// *** MALLOC PANIC
examplesDir / "pos" / "get_put.effekt",
// sanitizer/valgrind: segfault
examplesDir / "pos" / "parametrized.effekt",

// Valgrind leak/failure
examplesDir / "llvm" / "nested.effekt",
examplesDir / "llvm" / "strings.effekt",
// Valgrind leak, unclear
examplesDir / "llvm" / "polymorphism_map.effekt",
examplesDir / "pos" / "parser.effekt",
examplesDir / "pos" / "matchdef.effekt",
examplesDir / "pos" / "type_parameters_blocks.effekt",
examplesDir / "pos" / "long_string.effekt",
examplesDir / "pos" / "matchblock.effekt",
examplesDir / "pos" / "overloading.effekt",
examplesDir / "pos" / "withstatement.effekt",
examplesDir / "pos" / "dequeue.effekt",
examplesDir / "pos" / "higherorder_io_control.effekt",
examplesDir / "pos" / "infer" / "infer_overload.effekt",
examplesDir / "pos" / "bug1.effekt",
examplesDir / "pos" / "string_concat_pr493.effekt",
examplesDir / "pos" / "string" / "substring.effekt",
examplesDir / "pos" / "string" / "indexOf.effekt",
examplesDir / "benchmarks" / "other" / "variadic_combinators.effekt",

// Valgrind leak in array_new
examplesDir / "benchmarks" / "are_we_fast_yet" / "sieve.effekt",
examplesDir / "benchmarks" / "are_we_fast_yet" / "nbody.effekt",
examplesDir / "benchmarks" / "are_we_fast_yet" / "bounce.effekt",
examplesDir / "benchmarks" / "are_we_fast_yet" / "bounce.effekt", // + c_ref_fresh
examplesDir / "benchmarks" / "are_we_fast_yet" / "towers.effekt",
examplesDir / "benchmarks" / "are_we_fast_yet" / "permute.effekt",
examplesDir / "benchmarks" / "are_we_fast_yet" / "queens.effekt",
examplesDir / "benchmarks" / "effect_handlers_bench" / "tree_explore.effekt",
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ define ${show(returnType)} ${globalName(name)}(${commaSeparated(parameters.map(s

def show(callingConvention: CallingConvention): LLVMString = callingConvention match {
case Ccc() => "ccc"
case Tailcc() => "tailcc"
case Tailcc(_) => "tailcc"
}

def show(basicBlock: BasicBlock)(using Context): LLVMString = basicBlock match {
Expand All @@ -61,11 +61,13 @@ ${indentedLines(instructions.map(show).mkString("\n"))}
s"${localName(result)} = call ccc ${show(tpe)} ${globalName(name)}(${commaSeparated(arguments.map(show))})"
case Call(_, Ccc(), _, nonglobal, _) =>
C.abort(s"cannot call non-global operand: $nonglobal")
case Call(_, Tailcc(), VoidType(), ConstantGlobal(_, name), arguments) =>
s"musttail call tailcc void ${globalName(name)}(${commaSeparated(arguments.map(show))})"
case Call(_, Tailcc(), VoidType(), LocalReference(_, name), arguments) =>
s"musttail call tailcc void ${localName(name)}(${commaSeparated(arguments.map(show))})"
case Call(_, Tailcc(), tpe, _, _) =>
case Call(_, Tailcc(false), VoidType(), ConstantGlobal(_, name), arguments) =>
s"call tailcc void ${globalName(name)}(${commaSeparated(arguments.map(show))})"
case Call(_, Tailcc(false), VoidType(), LocalReference(_, name), arguments) =>
s"call tailcc void ${localName(name)}(${commaSeparated(arguments.map(show))})"
case Call(result, Tailcc(true), resultType, function, arguments) =>
s"musttail ${show(Call(result, Tailcc(false), resultType, function, arguments))}"
case Call(_, Tailcc(_), tpe, _, _) =>
C.abort(s"tail call to non-void function returning: $tpe")

case Load(result, tpe, LocalReference(PointerType(), name)) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ object Transformer {
val basicBlocks = FC.basicBlocks; FC.basicBlocks = null;
val instructions = BC.instructions; BC.instructions = null;

val transitionJump = Call("_", Tailcc(false), VoidType(), ConstantGlobal(FunctionType(VoidType(), Nil), "effektMainTailcc"), List())
val transitionBlock = BasicBlock("transition", List(transitionJump), RetVoid())
val transitionFunction = Function(Ccc(), VoidType(), "effektMain", List(), List(transitionBlock))

val entryBlock = BasicBlock("entry", instructions, terminator)
// TODO strictly speaking, the entry function should use the C calling convention
val entryFunction = Function(Tailcc(), VoidType(), "effektMain", List(), entryBlock :: basicBlocks)
declarations.map(transform) ++ definitions :+ entryFunction
val effektMain = Function(Tailcc(true), VoidType(), "effektMainTailcc", List(), entryBlock :: basicBlocks)
declarations.map(transform) ++ definitions :+ transitionFunction :+ effektMain
}

// context getters
Expand Down Expand Up @@ -506,13 +509,16 @@ object Transformer {
val instructions = BC.instructions; BC.instructions = null;

val entryBlock = BasicBlock("entry", instructions, terminator);
val function = Function(Tailcc(), VoidType(), name, parameters :+ Parameter(stackType, "stack"), entryBlock :: basicBlocks);
val function = Function(Tailcc(true), VoidType(), name, parameters :+ Parameter(stackType, "stack"), entryBlock :: basicBlocks);

emit(function)
}

def callLabel(name: Operand, arguments: List[Operand])(using BlockContext): Instruction =
Call("_", Tailcc(), VoidType(), name, arguments :+ getStack())
Call("_", Tailcc(true), VoidType(), name, arguments :+ getStack())

def callLabelTransition(name: Operand, arguments: List[Operand])(using BlockContext): Instruction =
Call("_", Tailcc(false), VoidType(), name, arguments :+ getStack())

def initialEnvironmentPointer = LocalReference(environmentType, "environment")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export Definition.*

enum CallingConvention {
case Ccc()
case Tailcc()
case Tailcc(musttail: Boolean)
}
export CallingConvention.*

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ object Transformer {
// TODO all of this can go away, if we desugar records in the translation to core!
val fields = DeclarationContext.getField(field).constructor.fields
val fieldIndex = fields.indexWhere(_.id == field)
val variables = fields.map { f => Variable(freshName("select"), transform(tpe)) }
val variables = fields.map { f => Variable(freshName("select"), transform(f.tpe)) }
transform(target).flatMap { value =>
Binding { k =>
Switch(value, List(0 -> Clause(variables, k(variables(fieldIndex)))), None)
Expand Down
3 changes: 2 additions & 1 deletion examples/llvm/polymorphism_map.effekt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ def main() = {
val values = [1,2,3,4,5]
val squares = map(values){ n => n * n }
map(squares){ n => println(n) }
}
()
}
1 change: 1 addition & 0 deletions libraries/common/effekt.effekt
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ extern pure def length(str: String): Int =
ml "String.size ${str}"
llvm """
%x = call %Int @c_buffer_length(%Pos ${str})
call void @erasePositive(%Pos ${str})
ret %Int %x
"""

Expand Down
2 changes: 1 addition & 1 deletion libraries/llvm/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ void c_array_erase_fields(void *envPtr) {
uint64_t *sizePtr = envPtr;
struct Pos *dataPtr = envPtr + sizeof(uint64_t);
uint64_t size = *sizePtr;
for(int i = 0; i < size; i++) {
for (uint64_t i = 0; i < size; i++) {
erasePositive(dataPtr[i]);
}
}
Expand Down
22 changes: 14 additions & 8 deletions libraries/llvm/buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ struct Pos c_buffer(const uint32_t offset, const uint32_t length, void* obj) {
};
}

void c_buffer_erase_noop(void *envPtr) {}
void c_buffer_erase_noop(void *envPtr) { (void)envPtr; }

uint64_t c_buffer_offset(const struct Pos buffer) {
return (uint32_t)(buffer.tag >> 32); // Extract offset from the upper 32 bits
}

// Function to get the length from Pos
// WARNING: This does not erase the Pos argument!
uint64_t c_buffer_length(const struct Pos buffer) {
return (uint32_t)(buffer.tag & 0xFFFFFFFF); // Extract length from the lower 32 bits
}
Expand All @@ -60,13 +61,14 @@ struct Pos c_buffer_construct(const uint64_t n, const uint8_t *data) {
return c_buffer(0, n, obj);
}

// TODO this allocates, copies, and frees immediately... improve this
struct Pos c_buffer_construct_zeroed(const uint64_t n) {
uint8_t *zeroes = calloc(n, sizeof *zeroes);
uint8_t *zeroes = calloc(sizeof(struct Header) + n, sizeof *zeroes);
ASSERT_NON_NULL(zeroes)
const struct Pos buffer = c_buffer_construct(n, zeroes);
free(zeroes);
return buffer;

struct Header *header = (void*) zeroes;
*header = (struct Header) { .rc = 0, .eraser = c_buffer_erase_noop, };

return c_buffer(0, n, zeroes);
}

struct Pos c_buffer_construct_uninitialized(const uint64_t n) {
Expand All @@ -93,7 +95,7 @@ struct Pos c_buffer_clone(const struct Pos buffer) {

void c_buffer_copy(const struct Pos from, struct Pos to, uint64_t startFrom, uint64_t startTo, uint64_t length) {
// Check bounds
if (startFrom < 0 || startTo < 0 || (startFrom + length > c_buffer_length(from)) || (startTo + length > c_buffer_length(to))) {
if ((startFrom + length > c_buffer_length(from)) || (startTo + length > c_buffer_length(to))) {
return;
}

Expand Down Expand Up @@ -169,7 +171,11 @@ struct Pos c_buffer_concatenate(const struct Pos left, const struct Pos right) {
struct Pos c_buffer_eq(const struct Pos left, const struct Pos right) {
uint64_t left_len = c_buffer_length(left);
uint64_t right_len = c_buffer_length(right);
if (left_len != right_len) return BooleanFalse;
if (left_len != right_len) {
erasePositive(left);
erasePositive(right);
return BooleanFalse;
}
for (uint64_t j = 0; j < left_len; ++j) {
if (c_buffer_bytes(left)[j] != c_buffer_bytes(right)[j]) {
erasePositive(left);
Expand Down
2 changes: 1 addition & 1 deletion libraries/llvm/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ int program_argc;
char** program_argv;

struct Pos c_get_arg(uint64_t idx) {
if(idx < program_argc) {
if(idx < (uint64_t)program_argc) {
return c_buffer_construct_from_null_terminated_string(program_argv[idx]);
} else {
return c_buffer_construct_zeroed(1);
Expand Down

0 comments on commit 4e81699

Please sign in to comment.