nitrix/klang
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

GenASM: Fix segfaults when calling malloc

Browse Source 
This was a tricky one. In order to call malloc the stack needs to be 16 byte aligned. GenASM up until this point was 8 byte aligned. This commit changes the code generation so that it is guaranteed that the stack is 16 byte aligned before visiting a child node. We need to do this before visiting *any* child node because we can not know whether somewhere downstream some child node calls malloc.

To make it possible for a visitor method to determine if it needs to align the stack it has to assume that the stack is currently aligned. This is the other reason we ensure that the stack is aligned before visiting any child node.

On top of that some visitor methods did not clean up after themselves. I am not sure why this did not result in segfaults already, but I changed the code so that each visitor method leaves the stack in the same state after completion.
This commit is contained in:
nitrix
2023-03-23 03:21:10 +01:00
parent 76419d86bb
commit 07e5a338a4
1 changed files with 117 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

144
src/main/java/de/hsrm/compiler/Klang/visitors/GenASM.java
View File

@@ -70,6 +70,7 @@ public class GenASM implements Visitor<Void> {
private int lCount = 0;
private int currentFunctionStartLabel = 0;
private long bytesToClearFromTheStack = 0;
private Parameter[] currentFunctionParams;
public GenASM(String mainName, Map<String, StructDefinition> structs) {
@@ -291,10 +292,19 @@ public class GenASM implements Visitor<Void> {
@Override
public Void visit(ModuloExpression e) {
e.lhs.welcome(this);
// A single pushq brings the stack out of 16 byte alignment,
// so we need to increase the stack by another 8 bytes
asm.sub("q", "$8", "%rsp");
asm.push("q", "%rax");
e.rhs.welcome(this);
asm.mov("q", "%rax", "%rbx");
asm.pop("q", "%rax");
// Which we remove afterwards
asm.add("q", "$8", "%rsp");
asm.cqto();
asm.idiv("q", "%rbx");
asm.mov("q", "%rdx", "%rax");
@@ -516,6 +526,14 @@ public class GenASM implements Visitor<Void> {
@Override
public Void visit(ReturnStatement e) {
e.expression.welcome(this);
// The ReturnStatement visitor is kindly removing the
// stack space that was allocated by the FunctionDefinition
// before executing the function body.
if (bytesToClearFromTheStack > 0) {
asm.add("q", "$" + bytesToClearFromTheStack, "%rsp");
}
asm.leave();
asm.ret();
return null;
@@ -614,6 +632,9 @@ public class GenASM implements Visitor<Void> {
}
// Check the stack alignment and correct if necessary
// The padding needs to happen after the register parameters
// and local variables were pushed to the stack because otherwise
// the relative offsets to rbp will be wrong.
if ((registerParameters.size() + e.localVariables.length) % 2 != 0) {
// Since each variable is 8 bytes and the stack is 16 byte aligned
// we need to add 8 bytes to the stack to make it aligned
@@ -622,6 +643,16 @@ public class GenASM implements Visitor<Void> {
}
e.block.welcome(this);
// I need to clear the stack here so that the top most element is the old rbp that
// ret uses to return to the caller but code that gets generated here will never be
// reached since the block node is guaranteed to contain a return node that results
// in a ret command being executed before this code would be reached.
// As a workaround (and a dirty, dirty hack) I indicate to the return node visitor
// how many bytes need to be cleared from the stack.
var wasStackPadded = (registerParameters.size() + e.localVariables.length) % 2 != 0;
bytesToClearFromTheStack = 8L * (registerParameters.size() + e.localVariables.length + (wasStackPadded ? 1 : 0));
return null;
}
@@ -649,6 +680,9 @@ public class GenASM implements Visitor<Void> {
return null;
}
// This holds the indices into the arguments array for
// arguments that need to be passed to the callee via the stack
var stackIndices = new ArrayList<Integer>();
if (e.arguments.length > 0) {
// Mapping arguments index -> xmm registers index
@@ -659,9 +693,6 @@ public class GenASM implements Visitor<Void> {
int[] rIdxs = new int[this.registers.length];
int ri = -1;
// Mapping arguments index -> stack
ArrayList<Integer> stackIdxs = new ArrayList<Integer>();
// Go through arguments
// sort them into the memory regions they go when being passed to functions
for (int i = 0; i < e.arguments.length; i++) {
@@ -673,7 +704,7 @@ public class GenASM implements Visitor<Void> {
xmmIdxs[fi] = i;
} else {
// Float onto stack
stackIdxs.add(i);
stackIndices.add(i);
}
} else {
if (ri < this.registers.length - 1) {
@@ -682,14 +713,39 @@ public class GenASM implements Visitor<Void> {
rIdxs[ri] = i;
} else {
// bool/int onto stack
stackIdxs.add(i);
stackIndices.add(i);
}
}
}
// Welcome the arguments in order, push everything onto the stack
for (var arg : e.arguments) {
// Make sure that the stack is aligned after all arguments
// have been pushed onto the stack. Keep in mind that we
// may remove the top n elements from our stack so they form
// the base of the callee's stack.
if ((e.arguments.length - stackIndices.size()) % 2 != 0) {
asm.sub("q", "$8", "%rsp");
}
// Welcome the arguments in order, push everything onto the stack.
// We can assume that we are currently 16 byte aligned which holds
// true until after the result of the first argument is pushed onto
// the stack. After that we have to pad the stack for every argument
// to ensure that the stack is aligned before welcoming the argument.
// We get rid of the padding directly after welcome() to
// "tightly pack" the arguments on the stack. This way the usual
// 8 byte offsets are kept intact.
for (int i = 0; i < e.arguments.length; i++) {
if (i % 2 == 0) {
asm.sub("q", "$8", "%rsp");
}
var arg = e.arguments[i];
arg.welcome(this);
if (i % 2 == 0) {
asm.add("q", "$8", "%rsp");
}
if (arg.type.equals(Type.getFloatType())) {
asm.mov("q", "%xmm0", "%rax");
asm.push("q", "%rax");
@@ -712,22 +768,38 @@ public class GenASM implements Visitor<Void> {
asm.mov("q", rspOffset, "%rsp", this.registers[i]);
}
// Move everything else from a higher stack position to our stack frame start
// Reorder the remaining n arguments that did not fit in a register,
// so that the remaining n arguments occupy the last n spots on our current stack
// This does not manipulate the stack in any way. It just reorders it.
int stackStartOffset = ((e.arguments.length) * 8);
for (int i = stackIdxs.size() - 1; i >= 0; i--) {
for (int i = stackIndices.size() - 1; i >= 0; i--) {
stackStartOffset -= 8;
int indexInArguments = stackIdxs.get(i);
int indexInArguments = stackIndices.get(i);
int rspOffset = (((e.arguments.length - indexInArguments) - 1) * 8);
asm.mov("q", rspOffset, "%rsp", "%rax");
asm.mov("q", "%rax", stackStartOffset, "%rsp");
}
// Rescue RSP
asm.add("q", stackStartOffset, "%rsp");
// The top n elements of the stack prepared in the step earlier will now become
// the base of the callee's stack by shrinking the stack by the size of the
// n arguments that did not fit in registers.
// This must only be done if there were any arguments that did not fit in the registers.
if (!stackIndices.isEmpty()) {
asm.add("q", stackStartOffset, "%rsp");
}
}
// We rename a function name if it is "main"
asm.call(e.name.equals("main") ? "main_by_user": e.name);
var mainName = e.name.equals("main") ? "main_by_user": e.name;
asm.call(mainName);
// Remove the arguments that remained on the stack and the stack padding if there was any
var wasStackPadded = (e.arguments.length - stackIndices.size()) % 2 != 0;
var bytesToRemove = 8 * (e.arguments.length - stackIndices.size() + (wasStackPadded ? 1 : 0));
if (bytesToRemove > 0) {
asm.add("q", "$" + bytesToRemove, "%rsp");
}
return null;
}
@@ -815,10 +887,29 @@ public class GenASM implements Visitor<Void> {
@Override
public Void visit(ConstructorCall e) {
// Make sure the stack is aligned before calling malloc
if (e.args.length % 2 != 0) {
// an odd number of arguments means we called pushq an odd number of times
// which results in an unaligned stack. Subtract 8 from the stack pointer to
// re-align the stack
asm.sub("q", "$8", "%rsp");
}
// push arguments onto the stack
for (var arg: e.args) {
for (int i = 0; i < e.args.length; i++) {
// if you want to know why this is done go and read visit(FunctionCall)
if (i % 2 != 0) {
asm.sub("q", "$8", "%rsp");
}
var arg = e.args[i];
arg.welcome(this);
if (i % 2 != 0) {
asm.add("q", "$8", "%rsp");
}
// move float values from xmm0 to rax first
if (arg.type.equals(Type.getFloatType())) {
asm.mov("q", "%xmm0", "%rax");
@@ -827,29 +918,21 @@ public class GenASM implements Visitor<Void> {
asm.push("q", "%rax");
}
// Make sure the stack is aligned before calling malloc
if (e.args.length % 2 != 0) {
// an odd number of arguments means we called pushq an odd number of times
// which results in an unaligned stack. Subtract 8 from the stack pointer to
// re-align the stack
asm.sub("q", "$8", "%rsp");
}
// allocate heap memory by calling malloc
var structDef = this.structs.get(e.structName);
asm.mov("l", Helper.getFieldSizeBytes(structDef), "%edi");
asm.call("malloc@PLT");
// Get rid of the stack alignment if there was any
if (e.args.length % 2 != 0) {
asm.add("q", "$8", "%rsp");
}
// push args into struct memory, last arg is on top of the stack
for (int i = e.args.length - 1; i >= 0; i--) {
asm.pop("q", Helper.getFieldOffset(structDef, i) + "(%rax)");
}
// Get rid of the stack alignment if there was any
if (e.args.length % 2 != 0) {
asm.add("q", "$8", "%rsp");
}
return null;
}
@@ -923,10 +1006,17 @@ public class GenASM implements Visitor<Void> {
return true;
} else {
lhs.welcome(this);
// A single pushq brings the stack out of 16 byte alignment,
// so we need to increase the stack by another 8 bytes
asm.sub("q", "$8", "%rsp");
asm.push("q", "%rax");
rhs.welcome(this);
asm.mov("q", "%rax", "%rbx");
asm.pop("q", "%rax");
// Which we remove afterwards
asm.add("q", "$8", "%rsp");
return false;
}
}