Skip to content

Commit

Permalink
Use flatbuffers::String::str instead of c_str. (#20487)
Browse files Browse the repository at this point in the history
### Description
<!-- Describe your changes. -->
flatbuffers::String::c_str returns a pointer that may not be null
terminated.

This causes a warning when building on an A100 with gcc 11. Not clear
why other builds with gcc 11 (e.g. Ubuntu 22.04 WSL) don't generate a
warning. Either way it's safer to use str() as that constructs a
std::string with data() and size().

Unclear if this is an issue in reality as it's reading from the
flatbuffer and most likely didn't write out an empty string in order to
save space. There's no perf need to use c_str instead of str, and in
LOAD_STR_FROM_ORT_FORMAT we need to convert the return value to a
std::string anyway.

```c++
struct String : public Vector<char> {
  const char *c_str() const { return reinterpret_cast<const char *>(Data()); }
  std::string str() const { return std::string(c_str(), size()); }
```

```
    inlined from ‘onnxruntime::common::Status onnxruntime::fbs::utils::LoadAttributeOrtFormat(const onnxruntime::fbs::Attribute&, onnx::AttributeProto&, std::unique_ptr<onnxruntime::Graph>&, onnxruntime::Graph&, onnxruntime::Node&, const onnxruntime::OrtFormatLoadOptions&, const onnxruntime::logging::Logger&)’ at /frdong_data/onnxruntime/onnxruntime/core/graph/graph_flatbuffers_utils.cc:385:3:
/usr/include/c++/11/bits/char_traits.h:399:32: error: ‘long unsigned int __builtin_strlen(const char*)’ reading 1 or more bytes from a region of size 0 [-Werror=stringop-overread]
```

### Motivation and Context
<!-- - Why is this change required? What problem does it solve?
- If it fixes an open issue, please link to the issue here. -->
Fix build error on A100
  • Loading branch information
skottmckay authored Apr 27, 2024
1 parent a0775d7 commit 321c1e5
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
5 changes: 3 additions & 2 deletions onnxruntime/core/flatbuffers/flatbuffers_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,9 @@ Status SaveValueInfoOrtFormat(flatbuffers::FlatBufferBuilder& builder,
#endif // #if !defined(ORT_MINIMAL_BUILD)

void LoadStringFromOrtFormat(std::string& dst, const flatbuffers::String* fbs_string) {
if (fbs_string)
dst = fbs_string->c_str();
if (fbs_string) {
dst = fbs_string->str();
}
}

static Status LoadTypeInfoOrtFormat(const fbs::TypeInfo& fbs_type_info,
Expand Down
2 changes: 1 addition & 1 deletion onnxruntime/core/flatbuffers/flatbuffers_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void LoadStringFromOrtFormat(std::string& dst, const flatbuffers::String* fbs_st
#define LOAD_STR_FROM_ORT_FORMAT(protobuf_msg, str_field, fbs_string) \
{ \
if (fbs_string) \
protobuf_msg.set_##str_field(fbs_string->c_str()); \
protobuf_msg.set_##str_field(fbs_string->str()); \
}

onnxruntime::common::Status LoadValueInfoOrtFormat(
Expand Down

0 comments on commit 321c1e5

Please sign in to comment.