jfjlaros / simpleRPC

Simple RPC implementation for Arduino.
MIT License
50 stars 17 forks source link

int vec_test3(Vector<uint8_t> &v, int flags) --> CRASH #5

Closed ghost closed 4 years ago

ghost commented 4 years ago

Calling the following function via RPC results in a crash (corrupt heap detected) when the function returns.

int vec_test3(Vector<uint8_t> &v, int flags)
{
    return flags+1;
}

Without an int after the Vector, there is no issue.

Vector<uint8_t> vec_test1(Vector<uint8_t> &v)
{
    Vector<uint8_t> res = Vector<uint8_t>(v.size);
    for (int i=0;  i<v.size;  i++)
        res[v.size-i-1] = v[i];
    return res; 
}

Vector<uint8_t> vec_test2(int sz)
{
    auto res = Vector<uint8_t>(sz);
    for (int i=0;  i<sz;  i++) res[i] = 'A' + i;
    return res;
}
jfjlaros commented 4 years ago

Can you try it now?

bboser commented 4 years ago

Wow, thanks for the quick fix! Yes, no more problems.

With hindsight I see why this should help, but I never would have found the problem.

Thanks, Bernhard

On Sat, Jul 20, 2019 at 3:20 AM Jeroen F.J. Laros notifications@github.com wrote:

Can you try it now?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jfjlaros/simpleRPC/issues/5?email_source=notifications&email_token=AAW5XJDD3YFRX6E3XBPJPVLQALRHVA5CNFSM4IFMZHCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NLMBA#issuecomment-513455620, or mute the thread https://github.com/notifications/unsubscribe-auth/AAW5XJAMYPL5PNNJTS6I5M3QALRHVANCNFSM4IFMZHCA .

jfjlaros commented 4 years ago

I got quite some help from Valgrind, which reported an invalid free(). This suggests there was a problem like the one we have seen in #3.

Thank you for raising this issue.

bboser commented 4 years ago

Hi Jeroen,

How do you use Valgrind with an Arduino?

Do you test it on a host? Would be nice to be able to check memory leaks on the esp32.

Regards, Bernhard

On Sat, Jul 20, 2019 at 12:15 Jeroen F.J. Laros notifications@github.com wrote:

I got quite some help from Valgrind, which reported an invalid free(). This suggests there was a problem like the one we have seen in #3 https://github.com/jfjlaros/simpleRPC/issues/3.

Thank you for raising this issue.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/jfjlaros/simpleRPC/issues/5?email_source=notifications&email_token=AAW5XJGCLJK36UAP37SK2CTQANP5NA5CNFSM4IFMZHCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2NUKXY#issuecomment-513492319, or mute the thread https://github.com/notifications/unsubscribe-auth/AAW5XJGLIDARMWTBGPHFD5LQANP5NANCNFSM4IFMZHCA .

jfjlaros commented 4 years ago

Well, I cheat a bit I suppose.

I wrote a small fixture library to simulate a serial interface. This library is used in the unit tests which in turn use the Catch2 unit testing framework. These unit tests are then executed using Valgrind.

This approach is not 100% full proof, for one there are slight differences between the C++ version of Arduino and C++11 (which is enforced for the tests). It does allow for easy CI though.

ghost commented 4 years ago

Thanks, I have some serious learning to do.

Bernhard

On Tue, Jul 23, 2019 at 7:19 AM Jeroen F.J. Laros notifications@github.com wrote:

Well, I cheat a bit I suppose.

I wrote a small fixture https://github.com/jfjlaros/arduino-serial-fixture library to simulate a serial interface. This library is used in the unit tests https://github.com/jfjlaros/simpleRPC/tree/master/tests which in turn use the Catch2 https://github.com/catchorg/Catch2 unit testing framework. These unit tests are then executed https://github.com/jfjlaros/simpleRPC/blob/master/tests/Makefile using Valgrind.

This approach is not 100% full proof, for one there are slight differences between the C++ version of Arduino and C++11 (which is enforced for the tests). It does allow for easy CI https://travis-ci.org/jfjlaros/simpleRPC though.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jfjlaros/simpleRPC/issues/5?email_source=notifications&email_token=AL426CZPYOB5ITLTGCDHYA3QA4HPHA5CNFSM4IFMZHCKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2TISMY#issuecomment-514230579, or mute the thread https://github.com/notifications/unsubscribe-auth/AL426CY4Z6W3YH2TU53YFPTQA4HPHANCNFSM4IFMZHCA .