Update a Nested String/Vector/Object problem using Reflection

[Ranger]

Dear FlatBuffers's author,

I'm using FlatBuffers to store data in C++ storage and I wanna use REFLECTION instead of generating classes code, 'case I wanna create, use, and change data model at run time.

I have a problem of updating a String/Vector or Object in a complicated nested structure.

For example,

I have the Object structure like below:

table Person {
   jobs: [Job];
}

table Job {
   company: string;
   wage: float;
   companyInfo: CompanyInfo;
}

table CompanyInfo {
   name: string;
   address: string;
   numOfStaffs: int;
}

 

Now, after create process, I have one Person record (flatbuf). I would like to use REFLECTION to change only "Person.jobs[1].address" to new value: "NEW ADDRESS!!!".

I saw the project's reflection test code (in tests.cpp - ReflectionTest) and follow it as follow:

std::vector<uint8_t> resizingbuf(flatbuf, flatbuf +flatbuf.size()); // With flatbuf is the whole FlatBuffer data of the Person record.
auto rroot = flatbuffers::piv(tableDataPtr, resizingbuf); // with tableDataPtr is a flatbuffers::Table* that was calculated as "Person.jobs[1]" previously. I can make sure that tableDataPtr is retrieve correctly, because I can update a scalar field correctly.
SetString(schema, "NEW ADDRESS!!!", GetFieldS(**rroot, fieldNeedToEdit), &resizingbuf, root_table); // with fieldNeedToEdit is field "address" inside table "CompanyInfo", root_table is table "CompanyInfo".

But Exception (AddressSantilizer) happens in SetString code. Can you point out what did I do wrong:

- Did I set the wrong flatBuf for resizingbuf in the 1st line?

- Did I set value for rroot correctly? The GetFieldS(**rroot, fieldNeedToEdit) code in line 3 return the correct value as I set previously.

Please help me through out this situation.

-------

Many thanks!

Quan Nguyen Hoang

[Wouter van Oortmerssen]

Your code looks correct at first glance.

Can you get some more information? When Address Sanitizer fails, the address at which it fails, where is that relative to the buffer start and end? Can you see what is in memory there (is it near the beginning or end of the new or old string?

What if you run it without Address Sanitizer, are you able to read back the string correctly? If not what does it look like?

--
You received this message because you are subscribed to the Google Groups "FlatBuffers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to flatbuffers...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Ranger]

Dear Wouter van Oortmerssen,

I check it back and the case I mentioned happened in DagCheck() function (called when ResizeTable). When the error happened, the dang_idx is too big and it cause the Address Sanitizer error.

In my code, I trace the input field from root table like below:

std::vector<uint8_t> resizingbuf(itemPtr->data.data(), itemPtr->data.data()+itemPtr->data.size()); // resizingbuf is now the WHOLE Flatbuf of Root table

// trace Field from outer to inner (Person --> Job --> CompanyInfo --> address)
if(field is Object) {
   rroot = schema::piv(flatbuffer::GetFieldT(**rroot, *fieldToSelect), resizingbuf);
   // update the field
} else if(field is Vector of Object) {
   const flatbuffers::Table* tableVectorItem = schema::GetFieldV<flatbuffers::Offset<schema::Table>>(**rroot, *fieldToSelect)->Get(fieldIndex);
   rroot = schema::piv(const_cast<flatbuffers::Table*>(tableVectorItem), resizingbuf); // I HAVE to cast to remove const qualifier here. AM I WRONG HERE????
   // update the field
}

Finally, I have the pointer **rroot points to table CompanyInfo and a field of "address". I did SetString as code above.

Here is the JSON data I used to create data (the field data goes in the same order as they are defined):

{
  "custId": -3479823489239,
  "custName": "Hello, it's me",
  "interestingNumbers": [
    132,-3422,111111,-4235,32,32,4321
  ],
  "interestingQuotes": [
    "Never forget what you are, the rest of the world will not. Wear it like armor, and it can never be used to hurt you",
    "I am that I am",
    "Hello, bye!"
  ],
  "address": {
    "home_number": 12341242,
    "street": "Love boulevard"
  },
  "jobs": [
    {
      "company": "Viettel",
      "wage": 50000000.123,
      "title": "Department manager",
      "companyInfo": {
        "name": "Viettel",
        "slogan": "Say it your way",
        "stockPrice": 123434523,
        "departments": [
          {
            "name": "Hoa Lac Palace",
            "numOfStaffs": 2345
          },
          {
            "name": "Giang Van Minh",
            "numOfStaffs": 500
          },
          {
            "name": "Viettel Head Quarter",
            "numOfStaffs": 456
          }
        ],
        "headQuarter": {
          "address": "No.1 Nguyen Huu Duc",
          "managers": [
            {
              "name": "Nguyen Hoang Quan",
              "age": 40,
              "area": "General Mananager"
            },
            {
              "name": "Nguyen Manh Hung",
              "age": 77,
              "area": "Associate"
            },
            {
              "name": "Tong Viet Trung",
              "age": 78,
              "area": "Finance"
            }
          ]
        }
      }
    },
    {
      "company": "Freelancer",
      "wage": 407840784078.23,
      "title": "No title"
    }
  ],
  "inner1": {
    "inner2": {
      "inner3_arr": [
        {
          "inner4": "AAA"
        },
        {
          "inner4": "BBB"
        },
        {
          "inner4": "CCC"
        }
      ],
      "inner3": {
        "inner4": "EEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEEE"
      }
    }
  }
}

*** Case: Update field "jobs[0].company", the result is:

"custName": NOK
"address.home_number": NOK
"address.street": NOK
"jobs[0].company": OK
"jobs[0].title": OK
"jobs[0].companyInfo.headQuarter.managers[1].age": OK
"jobs[0].companyInfo.headQuarter.address": OK
"jobs[0].companyInfo.headQuarter.managers[0].name": OK
"inner1.inner2.inner3.inner4": OK

*** Case: Update field "jobs[0].title", the result is:

"custName": NOK
"address.home_number": NOK
"jobs[0].company": NOK
"address.street": NOK
"jobs[0].title": OK
"jobs[0].companyInfo.headQuarter.managers[1].age": OK
"jobs[0].companyInfo.headQuarter.address": OK
"jobs[0].companyInfo.headQuarter.managers[0].name": OK
"inner1.inner2.inner3.inner4": OK

It seems to be like that after Setting String, some offsets are changed incorrectly somehow. Is it true that after Setting String, all offsets (including offsets of above layer tables/vectors) are re-set?

------

Regards,

QUAN Nguyen Hoang

[Ranger]

After rechecking the code, I debugged and found that the vec->size() returns wrong value (reference code at: reflection.cpp::ResizeTable(), line 247. I don't know why the vec->size() is bigger than the real vec->size that I inserted, so the code DagCheck for each element location (location of String element, Object element) is wrong (In one of my test, vec->size() returns 10 and in another test, vec->size() return 14) when the item index is larger than the vector real size.

Can you re-check the code for me. (I'm using FlatBuffers v1.3.0).

----

Many thanks,

QUAN Nguyen Hoang

[Wouter van Oortmerssen]

Yes, the point of ResizeTable is to ensure all offsets get updated correctly. If it doesn't do that, it's either a bug in my code, or yours, or corrupt data being passed to it.

You say you parse the data from JSON, so I presume it gets passed correctly from the FlatBufferBuilder to resizingbuf. It would be worth calling the Verifier on the data in resizingbuf both just before you call SetString and after (if possible), and see what it says.

When you say vec->size() is incorrect, I presume this is the vector for jobs, which should only have 2 elements?

as for the const cast.. you chose yourself to make that variable const?

[Ranger]

I re-checked my code and all the Verify tests passed. Up to now, I found the problem with updating String.

At the beginning of this discussion (question). I have code like bellow:

SetString(schema, "NEW ADDRESS!!!", GetFieldS(**rroot, fieldNeedToEdit), &resizingbuf, root_table); // with fieldNeedToEdit is field "address" inside table "CompanyInfo", root_table is table "CompanyInfo".

It is FAILED when I passed in the inner table ("CompanyInfo") as root_table (the last variable), but it ran CORRECTLY when I change root_table to be FlatBuffers root table ("Person").

But in the instruction of SetString(), you wrote: 

// If your FlatBuffer's root table is not the schema's root table, you should
// pass in your root_table type as well.

So, does it mean that I should pass "Person" or "CompanyInfo" as root_table? (Please look back at the beginning of this question).

I'm trying to change inner Vector data after finishing work with String. So, I think I will ask more questions about this.

Thank you!

[Wouter van Oortmerssen]

Yes, it needs to be Person. The reason you pass the root table is because it needs to traverse the entire buffer to fix offsets when you change the string length.

Apologies I did not spot in your first message you passed CompanyInfo as the root.

[Ranger]

Dear Wouter,

I found one thing in your code that I think it is a bug (maybe - version 1.3.0). I had a trouble with Resizing String for days:

In line 208 and line 212 of reflection.cpp::ResizeTable(const reflection::Object &objectdef, Table *table), you do change value of "table" (Straddle code). It means that, after this code, the start of "value that point to start of vtable" changes. So, after that, we cannot read vtable and all offsets inside correctly.

My fix: I think that we have to change all offset values before changing value of "value that point to start of vtable".

I moved 2 Straddle lines of code to below the code that change ALL field offsets (line 273) and my code run correctly after all.

Please consider this situation. Maybe, it is a bug.

[Wouter van Oortmerssen]

You are entirely correct! Wow, that's a bad bug :(

Do you want to make a PR for it, or shall I fix it?

--

[Ranger]

I think it's better that you will make a small fix for that code.

Thanks for your instant response.

-----

Regards,

QUAN Nguyen Hoang

[Ranger]

Dear Wouter,

In your reflection test, I saw that you can update a String by 2 steps:

1. Create a new buffer and point to it (use string_ptr).

2. Change offset of field String to string_ptr: 

SetFieldT(*rroot, name_field, string_ptr);

Can I do that with Vector/Object. For example, with vector:

flatbuffers::FlatBufferBuilder fbb;
fbb.Finish(fbb.CreateVector<uint32_t>(elements, vecSize)); // vector of uint32_t
auto newVecPtr = schema::AddFlatBuffer(resizingbuf, fbb.GetBufferPointer(), fbb.GetSize());
schema::SetFieldT(*rroot, fieldNeedToEdit, newVecPtr);

I ask this question, because after setting these things, I had some errors.

[Ranger]

Dear Wouter,

I think that (maybe) there is also a problem here:

1. In reflection.h::SetFieldT (line 404): It calls table->SetPointer

2. In flatbuffers.h::SetPointer (line 1263): It calls: 

WriteScalar(data_ + field_offset, val - (data_ + field_offset));

If I'm not wrong, this code is used to change offset pointer of String/Table/Vector, so, the Scalar value must be 4-byte-type (uint32_t, int32_t). But, as I debug, val - (data_ + field_offset) has a type of 8 bytes (I use sizeof() function to  measure it).

So, it writes the wrong value to the wrong place, causes a wrong buffer.

My fix: Cast the second parameter to uint32_t as below:

WriteScalar(data_ + field_offset, (uint32_t)(val - (data_ + field_offset)));

---------------
Regards,

QUAN Nguyen Hoang

[Wouter van Oortmerssen]

I'll make a fix, thanks for your help :)

I think you've found another bug.. the case you were using was never tested, only MutateOffset, but that also has a potential problem on 64bit machines.

Both are fixed in: https://github.com/google/flatbuffers/commit/1bba4fd9ea3e06e5d6aa712dbce3e2e7c326321e