summaryrefslogtreecommitdiff
path: root/include
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2022-03-31 10:03:39 +0200
committerStephan Bergmann <sbergman@redhat.com>2022-03-31 16:58:39 +0200
commitb66387574ef9c83cbfff622468496b6f0ac4d571 (patch)
tree6052d794f4d6400cdf3595e78e7a60c8d1dca3a0 /include
parentd940013e1726eba4d043110d5a01367c8d3f0a05 (diff)
Fix -Werror=array-bounds
> In file included from svtools/source/svrtf/parrtf.cxx:28: > In member function ‘typename rtl::libreoffice_internal::ConstCharArrayDetector<T, rtl::OUStringBuffer&>::TypeUtf16 rtl::OUStringBuffer::operator=(T&) [with T = const rtl::OUStringChar_]’, > inlined from ‘virtual int SvRTFParser::GetNextToken_()’ at svtools/source/svrtf/parrtf.cxx:183:94: > include/rtl/ustrbuf.hxx:352:20: error: array subscript ‘unsigned int[0]’ is partly outside array bounds of ‘rtl::OUStringChar [1]’ {aka ‘const rtl::OUStringChar_ [1]’} [-Werror=array-bounds] > 352 | std::memcpy( > | ~~~~~~~~~~~^ > 353 | pData->buffer, > | ~~~~~~~~~~~~~~ > 354 | libreoffice_internal::ConstCharArrayDetector<T>::toPointer(literal), > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > 355 | (n + 1) * sizeof (sal_Unicode)); //TODO: check for overflow > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > svtools/source/svrtf/parrtf.cxx: In member function ‘virtual int SvRTFParser::GetNextToken_()’: > svtools/source/svrtf/parrtf.cxx:183:94: note: object ‘<anonymous>’ of size 2 > 183 | aToken = OUStringChar( static_cast<sal_Unicode>(nTokenValue) ); > | ^ as seen with recent GCC 12 trunk in an --enable-optimized build. And add a test, even though it is relatively likely that the OUStringChar temporary is followed by null bytes, which would make the test happen to erroneously succeed. But at least tools like ASan or Valgrind could catch that. (For the corresponding OStringChar and OStringBuffer scenario, this issue does not arise, as OStringChar is not covered by ConstCharArrayDetector, so the correpsonding OStringBuffer assignment operator is OK memcpy'ing n+1 elements. There /are/ similar issues with string_view assignment operators for both O[U]StringBuffer, which will be addressed in a later commit.) Change-Id: Ia131d763aa5f8df45b9625f296408cc935df96ac Reviewed-on: https://gerrit.libreoffice.org/c/core/+/132354 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'include')
-rw-r--r--include/rtl/stringutils.hxx4
-rw-r--r--include/rtl/ustrbuf.hxx6
2 files changed, 9 insertions, 1 deletions
diff --git a/include/rtl/stringutils.hxx b/include/rtl/stringutils.hxx
index f97b7917f266..be56408c52a8 100644
--- a/include/rtl/stringutils.hxx
+++ b/include/rtl/stringutils.hxx
@@ -118,6 +118,10 @@ There are 2 cases:
would be automatically converted to the const variant, which is not wanted (not a string literal
with known size of the content). In this case ConstCharArrayDetector is used to ensure the function
is called only with const char[N] arguments. There's no other plain C string type overload.
+ (Note that OUStringChar is also covered by ConstCharArrayDetector's TypeUtf16 check, but
+ provides a pointer to a string that is not NUL-termianted, unlike the char16_t const[N] arrays
+ normally covered by that check, and which are assumed to represent NUL-terminated string
+ literals.)
2) All plain C string types are wanted, and const char[N] needs to be handled differently.
In this case const char[N] would match const char* argument type (not exactly sure why, but it's
consistent in all of gcc, clang and msvc). Using a template with a reference to const of the type
diff --git a/include/rtl/ustrbuf.hxx b/include/rtl/ustrbuf.hxx
index d35200643e01..fef71f98b329 100644
--- a/include/rtl/ustrbuf.hxx
+++ b/include/rtl/ustrbuf.hxx
@@ -349,10 +349,14 @@ public:
if (n >= nCapacity) {
ensureCapacity(n + 16); //TODO: check for overflow
}
+ // For OUStringChar, which is covered by this template's ConstCharArrayDetector TypeUtf16
+ // check, toPointer does not return a NUL-terminated string, so we can't just memcpy n+1
+ // elements but rather need to add the terminating NUL manually:
std::memcpy(
pData->buffer,
libreoffice_internal::ConstCharArrayDetector<T>::toPointer(literal),
- (n + 1) * sizeof (sal_Unicode)); //TODO: check for overflow
+ n * sizeof (sal_Unicode));
+ pData->buffer[n] = '\0';
pData->length = n;
return *this;
}