Fix JsonDocument::clear() does not clear the KEY_IS_OWNED flag in VariantData's _flags variable

[DanielEbert]

If JsonDocument::clear() is called, the KEY_IS_OWNED flag of JsonDocument's _data variable is not cleared.

Currently, JsonDocument::clear() calls _data.setNull, which calls setType(VALUE_IS_NULL), and setType preserves the KEY_IS_OWNED flag due to the '_flags &= KEY_IS_OWNED;' statement.

In this pull request, the _data.init() call was added to the JsonDocument::clear() method so that the KEY_IS_OWNED flag is cleared.

[coveralls]

Coverage increased (+0.0003%) to 99.039% when pulling c3ebea9 on DanielEbert:6.x into d8a1d1a on bblanchon:6.x.

[DanielEbert]

This might be related to the (fixed) issue from #1217 (comment)

[bblanchon]

Hi @DanielEbert,

Thank you very much for this Pull Request.
Why did you need to make this change in the first place?

Best Regards,
Benoit

[DanielEbert]

TLDR: You can close this PR. I forgot that I had modified my local ArduinoJson source code. This PR fixes a problem that I have introduced with my own (local only) changes -- this problem does not exist with the unmodified latest ArduinoJson version.

---

Long Story:

MemorySanitizer complained due to a use-of-uninitialized-value in JsonDocument::clear().
I'll post this just in case it is a problem. I don't think it is a problem because MemorySanitizer does not complain when the unmodified ArduinoJson version is used.

To learn the internals of ArduinoJson I added a bunch of printf statement, including one in clear():

  void clear() {
    _pool.clear();
    //_data.init();         // <- this is my PR
    _data.setNull();
    if (_data._flags != 0) {             // <- these are my local-only modifications
      printf("%u", _data._flags);        // <- these are my local-only modifications
    }                                    // <- these are my local-only modifications
  }

And I made VariantData's _flags variable public:

class VariantData {
...
public:
  uint8_t _flags;
...

This is my test source code c.cpp:

#include "ArduinoJson-v6.17.3.h"

int main(){
  char s[] = "{}";
  DynamicJsonDocument doc(256);
  DeserializationError ret = deserializeJson(doc, s, strlen(s));
  return 0;
}

Compile & Run (Ubuntu 20.04 64bit, clang 10.0, ArduinoJson-v6.17.3):

➜  clang++ c.cpp -fsanitize=memory && ./a.out
==6754==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4990a8 in ArduinoJson6173_71::JsonDocument::clear() (/tmp/1/a.out+0x4990a8)
    #1 0x498c05 in ArduinoJson6173_71::DeserializationError ArduinoJson6173_71::deserialize<ArduinoJson6173_71::JsonDeserializer, char, ArduinoJson6173_71::AllowAllFilter>(ArduinoJson6173_71::JsonDocument&, char*, unsigned long, ArduinoJson6173_71::NestingLimit, ArduinoJson6173_71::AllowAllFilter) (/tmp/1/a.out+0x498c05)
    #2 0x497967 in ArduinoJson6173_71::DeserializationError ArduinoJson6173_71::deserializeJson<char>(ArduinoJson6173_71::JsonDocument&, char*, unsigned long, ArduinoJson6173_71::NestingLimit) (/tmp/1/a.out+0x497967)
    #3 0x497541 in main (/tmp/1/a.out+0x497541)
    #4 0x7feb1f97e0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #5 0x41c2ad in _start (/tmp/1/a.out+0x41c2ad)

SUMMARY: MemorySanitizer: use-of-uninitialized-value (/tmp/1/a.out+0x4990a8) in ArduinoJson6173_71::JsonDocument::clear()
Exiting

Again, I don't think it is a problem because MemorySanitizer does not complain when the unmodified ArduinoJson version is used. I think I have this problem because my if statement accesses the 'KEY_IS_OWNED' bit of the _flags byte and this bit is not initialized. I assume VariantData does not access this 'KEY_IS_OWNED' bit and for this reason MemorySanitizer does not complain when the unmodified ArduinoJson version is used.

[bblanchon]

Indeed, KEY_IS_OWNED is not relevant for the root variant because, by definition, it doesn't have a key.
Anyway, you made me realize that I run the address, undefined, and leak sanitizers, but I forgot the memory sanitizer.

[bblanchon]

I tried to run the test suite with -fsanitize=memory, but I get use-of-uninitialized-value errors in the standard library, even when I use -stdlib=libc++:

==127==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x4ef2e9 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::size() const /usr/lib/llvm-10/bin/../include/c++/v1/string:947:17
    #1 0x540114 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::empty() const /usr/lib/llvm-10/bin/../include/c++/v1/string:967:42
    #2 0x51a957 in Catch::startsWith(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, char) /arduinojson/extras/tests/catch/catch.hpp:8712:19
    #3 0x53de5f in Catch::extractClassName(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) /arduinojson/extras/tests/catch/catch.hpp:7427:13
    #4 0x50e3ce in Catch::registerTestCase(Catch::ITestCase*, char const*, Catch::NameAndDesc const&, Catch::SourceLineInfo const&) /arduinojson/extras/tests/catch/catch.hpp:7447:21
    #5 0x50fc9b in Catch::registerTestCaseFunction(void (*)(), Catch::SourceLineInfo const&, Catch::NameAndDesc const&) /arduinojson/extras/tests/catch/catch.hpp:7456:9
    #6 0x50fdb9 in Catch::AutoReg::AutoReg(void (*)(), Catch::SourceLineInfo const&, Catch::NameAndDesc const&) /arduinojson/extras/tests/catch/catch.hpp:7465:9   
    #7 0x469a89 in __cxx_global_var_init /arduinojson/extras/tests/TextFormatter/writeFloat.cpp:26:1
    #8 0x469c48 in _GLOBAL__sub_I_writeFloat.cpp /arduinojson/extras/tests/TextFormatter/writeFloat.cpp
    #9 0x6e1b9c in __libc_csu_init (/tmp/extras/tests/TextFormatter/TextFormatterTests+0x6e1b9c)
    #10 0x7fb4e889e03f in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:264:6
    #11 0x46abdd in _start (/tmp/extras/tests/TextFormatter/TextFormatterTests+0x46abdd)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /usr/lib/llvm-10/bin/../include/c++/v1/string:947:17 in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >::size() const
Exiting

This was with clang-10 and libc++-10.
When I try with clang-11 and libc++-11, I get the error fatal error: 'sstream' file not found at compile-time

Do you know how I could work around these problems?

[DanielEbert]

I get the same error messages. I don't know why that is the case.

An alternative would be to use another uninitialized memory sanitizer, such as valgrind.

[bblanchon]

Done! I added a Valgrind job to the continuous integration.