include/rtl/stringutils.hxx | 4 ++++
include/rtl/ustrbuf.hxx | 6 +++++-
sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx | 5 +++++
3 files changed, 14 insertions(+), 1 deletion(-)
New commits:
commit b66387574ef9c83cbfff622468496b6f0ac4d571
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Thu Mar 31 10:03:39 2022 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Thu Mar 31 16:58:39 2022 +0200
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 <sberg...@redhat.com>
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;
}
diff --git a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
index 10f378c1acf9..da5a4634e94b 100644
--- a/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
+++ b/sal/qa/rtl/oustringbuffer/test_oustringbuffer_assign.cxx
@@ -13,6 +13,7 @@
#include <cppunit/TestAssert.h>
#include <cppunit/extensions/HelperMacros.h>
+#include <o3tl/cppunittraitshelper.hxx>
#include <rtl/ustrbuf.hxx>
#include <rtl/ustring.hxx>
@@ -66,6 +67,10 @@ private:
b4 = OUStringLiteral(u"1") + "234567890123456";
CPPUNIT_ASSERT_EQUAL(s3, b4.toString());
CPPUNIT_ASSERT_EQUAL(sal_Int32(32), b4.getCapacity());
+ b4 = OUStringChar('a');
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(1), b4.getLength());
+ CPPUNIT_ASSERT_EQUAL(u'a', b4.getStr()[0]);
+ CPPUNIT_ASSERT_EQUAL(u'\0', b4.getStr()[1]);
}
CPPUNIT_TEST_SUITE(Test);
