jingoro2112 / wrench

practical embedded script interpreter
MIT License
106 stars 9 forks source link

Invalid read of size 1 in wr_compile function #16

Closed dsendkowski closed 9 months ago

dsendkowski commented 9 months ago

Hi!

I'm trying to compile the following script:

function main(data) {
    scale = get_config("scale");
    offset = get_config("offset");
    x = data.value * scale;
}

by calling the wr_compile function. Unfortunately, running it with valgrind, I get the following error:

==13418== Invalid read of size 1
==13418==    at 0x4068CE: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:3387)
==13418==    by 0x416FB3: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5374)
==13418==    by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141)
==13418==    by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286)
==13418==    by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635)
==13418==    by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381)
==13418==    by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501)
==13418==    by 0x424840: WRCompilationContext::compile(char const*, int, unsigned char**, int*, char*, bool) (wrench.cpp:8043)
==13418==    by 0x42516B: wr_compile(char const*, int, unsigned char**, int*, char*, bool) (wrench.cpp:8165)
==13418==    by 0x4024A6: main (main.cpp:41)
==13418==  Address 0x4dc567f is 1 bytes before a block of size 17 alloc'd
==13418==    at 0x4844723: operator new[](unsigned long) (vg_replace_malloc.c:725)
==13418==    by 0x4043CA: WROpcodeStream::append(unsigned char const*, int) [clone .isra.0] (wrench.cpp:1731)
==13418==    by 0x406A4E: operator+= (wrench.cpp:1724)
==13418==    by 0x406A4E: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:4205)
==13418==    by 0x41641F: WRCompilationContext::addLocalSpaceLoad(WRBytecode&, WRstr&, bool) [clone .part.0] (wrench.cpp:4976)
==13418==    by 0x416AFD: addLocalSpaceLoad (wrench.cpp:4923)
==13418==    by 0x416AFD: WRCompilationContext::loadExpressionContext(WRExpression&, int, int) (wrench.cpp:5055)
==13418==    by 0x41713A: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5233)
==13418==    by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141)
==13418==    by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286)
==13418==    by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635)
==13418==    by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381)
==13418==    by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501)
==13418==    by 0x424840: WRCompilationContext::compile(char const*, int, unsigned char**, int*, char*, bool) (wrench.cpp:8043)

The wr_compile function returns zero. The problem is in this piece of code:

3386: else if ( bytecode.opcodes[o] == O_LoadFromLocal
3387:    && bytecode.opcodes[o-1] == O_LoadFromGlobal )

o-1 is negative, so the data is read 1 byte before the beginning of the allocated buffer. Any ideas what may be wrong? Or maybe I use it wrong?

jingoro2112 commented 9 months ago

looking into this right now, but the holidays are upon me so it may be a few days to sort it.

On Sat, Dec 23, 2023 at 5:51 AM Dariusz K. Sendkowski < @.***> wrote:

Hi!

I'm trying to compile the following script:

function main(data) { scale = get_config("scale"); offset = get_config("offset"); x = data.value * scale; }

by calling the wr_compile function. Unfortunately, running it with valgrind, I get the following error:

==13418== Invalid read of size 1 ==13418== at 0x4068CE: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:3387) ==13418== by 0x416FB3: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5374) ==13418== by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141) ==13418== by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286) ==13418== by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635) ==13418== by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381) ==13418== by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501) ==13418== by 0x424840: WRCompilationContext::compile(char const*, int, unsigned char*, int, char, bool) (wrench.cpp:8043) ==13418== by 0x42516B: wr_compile(char const, int, unsigned char, int, char, bool) (wrench.cpp:8165) ==13418== by 0x4024A6: main (main.cpp:41) ==13418== Address 0x4dc567f is 1 bytes before a block of size 17 alloc'd ==13418== at 0x4844723: operator new[](unsigned long) (vg_replace_malloc.c:725) ==13418== by 0x4043CA: WROpcodeStream::append(unsigned char const, int) [clone .isra.0] (wrench.cpp:1731) ==13418== by 0x406A4E: operator+= (wrench.cpp:1724) ==13418== by 0x406A4E: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:4205) ==13418== by 0x41641F: WRCompilationContext::addLocalSpaceLoad(WRBytecode&, WRstr&, bool) [clone .part.0] (wrench.cpp:4976) ==13418== by 0x416AFD: addLocalSpaceLoad (wrench.cpp:4923) ==13418== by 0x416AFD: WRCompilationContext::loadExpressionContext(WRExpression&, int, int) (wrench.cpp:5055) ==13418== by 0x41713A: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5233) ==13418== by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141) ==13418== by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286) ==13418== by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635) ==13418== by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381) ==13418== by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501) ==13418== by 0x424840: WRCompilationContext::compile(char const, int, unsigned char, int, char, bool) (wrench.cpp:8043)

The wr_compile function returns zero. The problem is in this piece of code:

3386: else if ( bytecode.opcodes[o] == O_LoadFromLocal 3387: && bytecode.opcodes[o-1] == O_LoadFromGlobal )

o-1 is negative, so the data is read 1 byte before the beginning of the allocated buffer. Any ideas what may be wrong? Or maybe I use it wrong?

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA3FWNGMWXB223JZQV3YK2ZRTAVCNFSM6AAAAABBAW5AXOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA2TINZVGEYDEOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

jingoro2112 commented 9 months ago

Found it, oversight in the keyhole optimizer! Thank you for running valgrind on it. I do this as part of my release protocol, but getting 100% code coverage is pretty difficult.

Have released 3.2.1 for download.

On Sat, Dec 23, 2023 at 1:02 PM Curt Hartung @.***> wrote:

looking into this right now, but the holidays are upon me so it may be a few days to sort it.

On Sat, Dec 23, 2023 at 5:51 AM Dariusz K. Sendkowski < @.***> wrote:

Hi!

I'm trying to compile the following script:

function main(data) { scale = get_config("scale"); offset = get_config("offset"); x = data.value * scale; }

by calling the wr_compile function. Unfortunately, running it with valgrind, I get the following error:

==13418== Invalid read of size 1 ==13418== at 0x4068CE: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:3387) ==13418== by 0x416FB3: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5374) ==13418== by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141) ==13418== by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286) ==13418== by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635) ==13418== by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381) ==13418== by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501) ==13418== by 0x424840: WRCompilationContext::compile(char const*, int, unsigned char*, int, char, bool) (wrench.cpp:8043) ==13418== by 0x42516B: wr_compile(char const, int, unsigned char, int, char, bool) (wrench.cpp:8165) ==13418== by 0x4024A6: main (main.cpp:41) ==13418== Address 0x4dc567f is 1 bytes before a block of size 17 alloc'd ==13418== at 0x4844723: operator new[](unsigned long) (vg_replace_malloc.c:725) ==13418== by 0x4043CA: WROpcodeStream::append(unsigned char const, int) [clone .isra.0] (wrench.cpp:1731) ==13418== by 0x406A4E: operator+= (wrench.cpp:1724) ==13418== by 0x406A4E: WRCompilationContext::pushOpcode(WRBytecode&, WROpcode) (wrench.cpp:4205) ==13418== by 0x41641F: WRCompilationContext::addLocalSpaceLoad(WRBytecode&, WRstr&, bool) [clone .part.0] (wrench.cpp:4976) ==13418== by 0x416AFD: addLocalSpaceLoad (wrench.cpp:4923) ==13418== by 0x416AFD: WRCompilationContext::loadExpressionContext(WRExpression&, int, int) (wrench.cpp:5055) ==13418== by 0x41713A: WRCompilationContext::resolveExpressionEx(WRExpression&, int, int) (wrench.cpp:5233) ==13418== by 0x41760C: WRCompilationContext::resolveExpression(WRExpression&) (wrench.cpp:5141) ==13418== by 0x41A5B7: WRCompilationContext::parseExpression(WRExpression&) (wrench.cpp:6286) ==13418== by 0x4208B7: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7635) ==13418== by 0x41FCBA: WRCompilationContext::parseUnit(bool, int) (wrench.cpp:6381) ==13418== by 0x41FECC: WRCompilationContext::parseStatement(int, char, bool&, WROpcode) (wrench.cpp:7501) ==13418== by 0x424840: WRCompilationContext::compile(char const, int, unsigned char, int, char, bool) (wrench.cpp:8043)

The wr_compile function returns zero. The problem is in this piece of code:

3386: else if ( bytecode.opcodes[o] == O_LoadFromLocal 3387: && bytecode.opcodes[o-1] == O_LoadFromGlobal )

o-1 is negative, so the data is read 1 byte before the beginning of the allocated buffer. Any ideas what may be wrong? Or maybe I use it wrong?

— Reply to this email directly, view it on GitHub https://github.com/jingoro2112/wrench/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALIKA3FWNGMWXB223JZQV3YK2ZRTAVCNFSM6AAAAABBAW5AXOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGA2TINZVGEYDEOI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

dsendkowski commented 9 months ago

Thanks a lot for this quick response :)