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>

3 files changed, 14 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;
     }
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);