google / flatbuffers

FlatBuffers: Memory Efficient Serialization Library
https://flatbuffers.dev/
Apache License 2.0
23.28k stars 3.25k forks source link

Clang analyzer reports potential memory leak with uint64 type #5224

Closed JerryJoyce closed 5 years ago

JerryJoyce commented 5 years ago

Language: c++ Compiler: clang version 6.0.0-1ubuntu2 OS: Ubuntu 18.04 FBVersion: 1.10

In my schema I have an element of type uint64. When I call the method to create an instance of this schema, the compiler thinks there is a potential memory leak. I really don't want to disable this compiler check.

Here is the clang analysis:

/go/src/RaaS.PerceptionEngine.Core/libArchon/Archon/src/api.cpp:159:22: note: Calling 'CreateArchonMessage' auto archonMsg = CreateArchonMessage(builder, ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/build/io/gencpp/archonmessage_generated.h:157:3: note: Calling 'ArchonMessageBuilder::addTimeStamp' builder.add_TimeStamp(TimeStamp); ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/build/io/gencpp/archonmessagegenerated.h:117:5: note: Calling 'FlatBufferBuilder::AddElement' fbb.AddElement(ArchonMessage::VT_TIMESTAMP, TimeStamp, 0); ^ /usr/local/include/flatbuffers/flatbuffers.h:1032:9: note: Left side of '&&' is true if (e == def && !forcedefaults) return; ^ /usr/local/include/flatbuffers/flatbuffers.h:1032:21: note: Assuming the condition is false if (e == def && !forcedefaults) return; ^ /usr/local/include/flatbuffers/flatbuffers.h:1032:5: note: Taking false branch if (e == def && !forcedefaults) return; ^ /usr/local/include/flatbuffers/flatbuffers.h:1034:5: note: Calling 'FlatBufferBuilder::TrackField' TrackField(field, off); ^ /usr/local/include/flatbuffers/flatbuffers.h:1024:5: note: Calling 'vector_downward::scratch_pushsmall' buf.scratch_push_small(fl); ^ /usr/local/include/flatbuffers/flatbuffers.h:719:5: note: Calling 'vector_downward::ensure_space' ensure_space(sizeof(T)); ^ /usr/local/include/flatbuffers/flatbuffers.h:665:5: note: Taking true branch if (len > static_cast(cur - scratch)) { reallocate(len); } ^ /usr/local/include/flatbuffers/flatbuffers.h:665:55: note: Calling 'vector_downward::reallocate' if (len > static_cast(cur - scratch)) { reallocate(len); } ^ /usr/local/include/flatbuffers/flatbuffers.h:776:29: note: Assuming 'old_reserved' is 0 old_reserved ? old_reserved / 2 : initialsize); ^ /usr/local/include/flatbuffers/flatbuffers.h:776:29: note: '?' condition is false /usr/local/include/flatbuffers/flatbuffers.h:778:9: note: Assuming the condition is false if (buf) { ^ /usr/local/include/flatbuffers/flatbuffers.h:778:5: note: Taking false branch if (buf) { ^ /usr/local/include/flatbuffers/flatbuffers.h:782:14: note: Calling 'Allocate' buf = Allocate(allocator, reserved_); ^ /usr/local/include/flatbuffers/flatbuffers.h:431:10: note: Assuming 'allocator' is null return allocator ? allocator->allocate(size) ^ /usr/local/include/flatbuffers/flatbuffers.h:431:10: note: '?' condition is false /usr/local/include/flatbuffers/flatbuffers.h:432:22: note: Calling 'DefaultAllocator::allocate' : DefaultAllocator().allocate(size); ^ /usr/local/include/flatbuffers/flatbuffers.h:418:12: note: Memory is allocated return new uint8t[size]; ^ /usr/local/include/flatbuffers/flatbuffers.h:432:22: note: Returned allocated memory : DefaultAllocator().allocate(size); ^ /usr/local/include/flatbuffers/flatbuffers.h:782:14: note: Returned allocated memory buf = Allocate(allocator, reserved); ^ /usr/local/include/flatbuffers/flatbuffers.h:665:55: note: Returned allocated memory if (len > static_cast(cur - scratch)) { reallocate(len); } ^ /usr/local/include/flatbuffers/flatbuffers.h:719:5: note: Returned allocated memory ensurespace(sizeof(T)); ^ /usr/local/include/flatbuffers/flatbuffers.h:1024:5: note: Returned allocated memory buf.scratch_push_small(fl); ^ /usr/local/include/flatbuffers/flatbuffers.h:1034:5: note: Returned allocated memory TrackField(field, off); ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/build/io/gencpp/archonmessagegenerated.h:117:5: note: Returned allocated memory fbb.AddElement(ArchonMessage::VT_TIMESTAMP, TimeStamp, 0); ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/build/io/gencpp/archonmessagegenerated.h:157:3: note: Returned allocated memory builder.add_TimeStamp(TimeStamp); ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/Archon/src/api.cpp:159:22: note: Returned allocated memory auto archonMsg = CreateArchonMessage(builder, ^ /go/src/RaaS.PerceptionEngine.Core/libArchon/Archon/src/api.cpp:171:5: note: Calling 'FlatBufferBuilder::Finish' builder.Finish(archonMsg); ^ /usr/local/include/flatbuffers/flatbuffers.h:1700:5: note: Calling 'FlatBufferBuilder::Finish' Finish(root.o, file_identifier, false); ^ /usr/local/include/flatbuffers/flatbuffers.h:1725:15: note: '?' condition is false PreAlign((size_prefix ? sizeof(uoffset_t) : 0) + sizeof(uoffset_t) + ^ /usr/local/include/flatbuffers/flatbuffers.h:1726:19: note: '?' condition is false (file_identifier ? kFileIdentifierLength : 0), ^ /usr/local/include/flatbuffers/flatbuffers.h:1725:5: note: Calling 'FlatBufferBuilder::PreAlign' PreAlign((size_prefix ? sizeof(uoffset_t) : 0) + sizeof(uoffset_t) + ^ /usr/local/include/flatbuffers/flatbuffers.h:1179:5: note: Calling 'vectordownward::fill' buf.fill(PaddingBytes(GetSize() + len, alignment)); ^ /usr/local/include/flatbuffers/flatbuffers.h:727:5: note: Calling 'vector_downward::make_space' make_space(zero_pad_bytes); ^ /usr/local/include/flatbuffers/flatbuffers.h:673:20: note: Calling 'vector_downward::ensure_space' size_t space = ensure_space(len); ^ /usr/local/include/flatbuffers/flatbuffers.h:665:5: note: Taking true branch if (len > static_cast(cur - scratch)) { reallocate(len); } ^ /usr/local/include/flatbuffers/flatbuffers.h:665:55: note: Calling 'vector_downward::reallocate' if (len > static_cast(cur - scratch)) { reallocate(len); } ^ /usr/local/include/flatbuffers/flatbuffers.h:776:29: note: '?' condition is true old_reserved ? old_reserved / 2 : initialsize); ^ /usr/local/include/flatbuffers/flatbuffers.h:778:5: note: Taking true branch if (buf_) { ^ /usr/local/include/flatbuffers/flatbuffers.h:786:3: note: Potential memory leak } ^

vglavnyy commented 5 years ago

Thank you for the report. This can be false positive due to hidden mutually exclusive conditions.

Can you reproduce this conditional flow in run-time as an isolated unit-test in the test.cpp? Flatbuffers cmake script has the ability to enable ASAN and UBSAN for the flattests target using -DFLATBUFFERS_CODE_SANITIZE=ON flag.

JerryJoyce commented 5 years ago

I suspect you are correct about this being a false positive. I've used NOLINT comments to calm down the noise.