diff options
author | Stephan Bergmann <sbergman@redhat.com> | 2019-09-03 11:26:44 +0200 |
---|---|---|
committer | Stephan Bergmann <sbergman@redhat.com> | 2019-09-03 15:54:08 +0200 |
commit | cf2696efa0d0ea55c950d7b2898e7b7aab74812e (patch) | |
tree | daa0b007384286b281eb93db81939a649ddac7b9 /cppu | |
parent | f1ec3c675f1fcb04de564861c24f26b2bac4244c (diff) |
Silence -fsanitize=object-size in --enable-optimized builds
<http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks>
(as of recent LLVM trunk towards LLVM 10) states:
"-fsanitize=object-size: An attempt to potentially use bytes which the optimizer
can determine are not part of the object being accessed. This will also detect
some types of undefined behavior that may not directly access memory, but are
provably incorrect given the size of the objects involved, such as invalid
downcasts and calling methods on invalid pointers. These checks are made in
terms of __builtin_object_size, and consequently may be able to detect more
problems at higher optimization levels."
A `make check screenshot` with --enabled-optimized runs into two such issues
that were not diagnosed with --disable-optimized, in both cases because a struct
with an "idiomatic trailing dynamic array member" (statically declared to be an
array of size 1) is allocated without any space for that array member, as the
dynamic array size is 0 in the specific case:
* For
> [CUT] sc_ucalc
> cppu/source/uno/copy.hxx:46:19: runtime error: member access within address 0x6020001aaa70 with insufficient space for an object of type 'uno_Sequence' (aka '_sal_Sequence')
> 0x6020001aaa70: note: pointer points here
> 2b 00 80 6a be be be be be be be be 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ^
> #0 in cppu::allocSeq(int, int) at cppu/source/uno/copy.hxx:46:19 (instdir/program/libuno_cppu.so.3 +0x41f03f)
> #1 in uno_type_sequence_reference2One at cppu/source/uno/sequence.cxx:813:20 (instdir/program/libuno_cppu.so.3 +0x41f03f)
[...]
the call to
pNew = allocSeq( 0, 0 );
in uno_type_sequence_reference2One is inlined, so
sal_uInt32 nSize = calcSeqMemSize( nElementSize, nElements );
in allocSeq is known to be too small.
* For
> testSignatureLineODF::TestBody finished in: 2044ms
> engine-gpg.c:302:6: runtime error: member access within address 0x604001f24f10 with insufficient space for an object of type 'struct arg_and_data_s'
> 0x604001f24f10: note: pointer points here
> 6e 01 00 26 be be be be be be be be be be be be be be be be be be be be be be be be be be be be
> ^
> #0 in add_data at workdir/UnpackedTarball/gpgmepp/src/engine-gpg.c:302:6 (instdir/program/libgpgme.so.11 +0x120c2b)
[...]
> make[1]: *** [solenv/gbuild/CppunitTest.mk:114: workdir/CppunitTest/xmlsecurity_signing.test] Error 1
the
a = malloc (sizeof *a - 1);
is apparently detected to be too small only with optimization enabled.
In both cases, the solution is to operate on the too-small dynamically allocated
object via a reinterpret_cast'ed pointer to some newly introduced "struct
prefix" type.
Change-Id: Ie814db1d00a439bb9189474b91d729e538e81984
Reviewed-on: https://gerrit.libreoffice.org/78548
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
Diffstat (limited to 'cppu')
-rw-r--r-- | cppu/source/uno/copy.hxx | 26 |
1 files changed, 23 insertions, 3 deletions
diff --git a/cppu/source/uno/copy.hxx b/cppu/source/uno/copy.hxx index b0811b5fab10..b24e0e337bf5 100644 --- a/cppu/source/uno/copy.hxx +++ b/cppu/source/uno/copy.hxx @@ -22,7 +22,9 @@ #include "prim.hxx" #include "constr.hxx" #include <cassert> +#include <cstddef> #include <cstdlib> +#include <type_traits> namespace cppu { @@ -30,6 +32,22 @@ namespace cppu //#### copy construction ########################################################################### +namespace { + +// The non-dynamic prefix of sal_Sequence (aka uno_Sequence): +struct SequencePrefix { + sal_Int32 nRefCount; + sal_Int32 nElements; +}; +static_assert(sizeof (SequencePrefix) < sizeof (uno_Sequence)); +static_assert(offsetof(SequencePrefix, nRefCount) == offsetof(uno_Sequence, nRefCount)); +static_assert( + std::is_same_v<decltype(SequencePrefix::nRefCount), decltype(uno_Sequence::nRefCount)>); +static_assert(offsetof(SequencePrefix, nElements) == offsetof(uno_Sequence, nElements)); +static_assert( + std::is_same_v<decltype(SequencePrefix::nElements), decltype(uno_Sequence::nElements)>); + +} inline uno_Sequence * allocSeq( sal_Int32 nElementSize, sal_Int32 nElements ) @@ -42,9 +60,11 @@ inline uno_Sequence * allocSeq( pSeq = static_cast<uno_Sequence *>(std::malloc( nSize )); if (pSeq != nullptr) { - // header init - pSeq->nRefCount = 1; - pSeq->nElements = nElements; + // header init, going via SequencePrefix to avoid UBSan insufficient-object-size + // warnings when `nElements == 0` and thus `nSize < sizeof (uno_Sequence)`: + auto const header = reinterpret_cast<SequencePrefix *>(pSeq); + header->nRefCount = 1; + header->nElements = nElements; } } return pSeq; |