prosyslab-classroom / is593-language-based-security

28 stars 6 forks source link

[Homework5] grading question #36

Closed KunJeong closed 4 years ago

KunJeong commented 4 years ago

Hi. Right now my debloater generates all results correctly, except for the "predecessor block" info that llvm automatically generates. For instance, dune test gives me:

-if.then:        ;preds = %entry
+if.then:       ;preds = %entry, %entry

Is this okay? I'm using the given delete function, so I can't think of a way to update that info as well.

Also, is it okay to leave in debug code, since hw5 does not grade the program output but instead grades the debloated .ll file? I experienced some behavior changes when adding/removing print code (weirdly enough, sometimes adding a print saved me from seg fault..!) so I would like to leave it this way if possible. Thank you.

KihongHeo commented 4 years ago

Hi. I am happy to hear that you complete homework.

Is that the only different line? I don't understand why you have if.then: ;preds = %entry, %entry. Make sure you replace conditional branches to unconditional branch that contains only one target, not conditional branches with two same destinations.

But anyway, if it is not the case, TA will handle such issues when grading.

KunJeong commented 4 years ago

Is that the only different line? I don't understand why you have if.then: ;preds = %entry, %entry.

I have if.then: ;preds = %entry, %entry and if.else: ;preds = %entry, %entry for hw3, and that is the only difference.

Make sure you replace conditional branches to unconditional branch that contains only one target, not conditional branches with two same destinations.

Thanks for the tip. I'm unsure if I'm understanding the replace function correctly. I have to specify the "conditional branch instruction" and the "target block" in that order, correct? If so, I don't see how I can be mistaken in the replacement of the target instruction, as the unconditional branch instruction is not created by me.

KihongHeo commented 4 years ago

Yes. I just wanted to double check.

KunJeong commented 4 years ago

Yes, thank you.