compare-transform-pass breaks original control flows

[vhertz]

Description

laf-intel's compare-transform-pass breaks original control flows. A PoC of the bug:

#include <stdio.h>
#include <stdlib.h>
#include <libxml/xmlstring.h>

int main() {
    char *msg = "foo";

    if (!xmlStrEqual(msg, "foo")) {
        fprintf(stderr, "msg is not foo!\n");
        abort();
    }
    fprintf(stdout, "msg is foo!\n");

    return 0;
}

xmlStrEqual() returns 1 if the two strings are equal. This PoC should exit successfully because the strings are both "foo".

$ ./a.out
msg is foo!

However, with transform-compare-pass, the PoC aborts.

$ ./a.out
msg is not foo!
Aborted (core dumped)

This result shows that transform-compare-pass has broken the original control flow. Perhaps the pass is designed for functions that return 0 when two arguments are equal, such as strcmp() or memcmp().

How to reproduce

Steps to reproduce the behavior

1. Install libxml2 (I've used 2.9.13, but any version will be acceptable as long as xmlStrEqual() is defined.).
2. Build compare-transform-pass.so (build AFL++ by LLVM_CONFIG=llvm-config-14 make all).
3. Build the PoC (as poc.c) with the pass and without the pass.
4. Run both the executables.

How to build the PoC without the pass

$ clang-14 -I /usr/include/libxml2 poc.c -lxml2

How to build the PoC with the pass

$ clang-14 -I /usr/include/libxml2 -fpass-plugin=../AFLplusplus/compare-transform-pass.so poc.c -lxml2

Expected behavior

Both the executables have the same output.

$ ./a.out
msg is foo!

Screen output/Screenshots

None.

Additional context

Some envirnomental info

• AFL++ commit hash (dev branch's head): 1604351
• OS: Linux 6.2.0 (Ubuntu 22.04)
• GCC: 11.4.0
• Clang/LLVM: 14.0.0

[vanhauser-thc]

This happens too with llvm 16, and with O3 and O0, so yeah this is a bug in our llvm module. these are a pain to debug and fix :-( ...
thanks for reporting

[vanhauser-thc]

I tried a fix in dad56ab - please test

[tokatoka]

I guess laf crash in #1808 has the same root cause

[vhertz]

At least for the PoC, the pass no longer seems to break the control flow. :)

I have two questions about dad56ab:

• !FuncName.compere("g_strcmp0") duplicates in isStrcmp, and !FuncName.compare("xmlStrncmp") duplicates in isStrncmp. Why?
• Why is success_is_one flag true even for g_strcmp0 and xmlStrncmp?

[vanhauser-thc]

I guess laf crash in #1808 has the same root cause

no that is different

[vanhauser-thc]

At least for the PoC, the pass no longer seems to break the control flow. :)

I have two questions about dad56ab:

• !FuncName.compere("g_strcmp0") duplicates in isStrcmp, and !FuncName.compare("xmlStrncmp") duplicates in isStrncmp. Why?
• Why is success_is_one flag true even for g_strcmp0 and xmlStrncmp?

because I was tired and had little time :) fixed.
thanks for reviewing!