[Libreoffice-bugs] [Bug 102616] EDITING: Compare documents on near-identical files flags 99.9% of the contents as different

bugzilla-daemon at bugs.documentfoundation.org bugzilla-daemon at bugs.documentfoundation.org
Mon Apr 6 16:01:03 UTC 2020


https://bugs.documentfoundation.org/show_bug.cgi?id=102616

Stephan Bergmann <sbergman at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |caolanm at redhat.com,
                   |                            |michael.stahl at cib.de,
                   |                            |vmiklos at collabora.com

--- Comment #16 from Stephan Bergmann <sbergman at redhat.com> ---
(In reply to Julien Nabet from comment #14)
> Created attachment 159359 [details]
> bt with debug symbols
> 
> On pc Debian x86-64 with master sources updated today, I got an assert.

I do not know whether the assert from comment 14 and my below UBSan+ASan
findings are related to the original issue.  However, I'll keep their
discussion in this issue for now instead of spawning a new issue.

So
<https://git.libreoffice.org/core/+diff/aef7feb3e695ecf6d411f0777196dcc4281e201a%5E!>
"New loplugin:unsignedcompare", which introduced the firing assert, was a kind
of a gamble.  It assumed that those casts from `long` to `unsigned long` would
only be done with non-negative values (to silence warnings about mixed
signed/unsigned comparisons), rather than to wrap around negative values to
large positive values on purpose.  The commit may well have guessed wrongly, of
course.

However, locally reverting the commit with

> diff --git a/compilerplugins/clang/unsignedcompare.cxx b/compilerplugins/clang/unsignedcompare.cxx
> index d9b8f144ca77..a929d219f205 100644
> --- a/compilerplugins/clang/unsignedcompare.cxx
> +++ b/compilerplugins/clang/unsignedcompare.cxx
> @@ -225,7 +225,7 @@ private:
>      }
>  };
>  
> -loplugin::Plugin::Registration<UnsignedCompare> unsignedcompare("unsignedcompare");
> +loplugin::Plugin::Registration<UnsignedCompare> unsignedcompare("unsignedcompare", false);
>  }
>  
>  /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
> diff --git a/sw/source/core/doc/doccomp.cxx b/sw/source/core/doc/doccomp.cxx
> index 21a79453985e..5566beeb48ff 100644
> --- a/sw/source/core/doc/doccomp.cxx
> +++ b/sw/source/core/doc/doccomp.cxx
> @@ -19,7 +19,6 @@
>  
>  #include <sal/config.h>
>  
> -#include <o3tl/safeint.hxx>
>  #include <osl/diagnose.h>
>  #include <rtl/character.hxx>
>  #include <swmodule.hxx>
> @@ -882,7 +881,7 @@ sal_uLong Compare::CompareSequence::CheckDiag( sal_uLong nStt1, sal_uLong nEnd1,
>              else
>                  x = thi;
>              y = x - d;
> -            while( o3tl::make_unsigned(x) < nEnd1 && o3tl::make_unsigned(y) < nEnd2 &&
> +            while( sal_uLong(x) < nEnd1 && sal_uLong(y) < nEnd2 &&
>                  m_rMoved1.GetIndex( x ) == m_rMoved2.GetIndex( y ))
>              {
>                  ++x;
> @@ -914,7 +913,7 @@ sal_uLong Compare::CompareSequence::CheckDiag( sal_uLong nStt1, sal_uLong nEnd1,
>              else
>                  x = thi - 1;
>              y = x - d;
> -            while( o3tl::make_unsigned(x) > nStt1 && o3tl::make_unsigned(y) > nStt2 &&
> +            while( sal_uLong(x) > nStt1 && sal_uLong(y) > nStt2 &&
>                  m_rMoved1.GetIndex( x - 1 ) == m_rMoved2.GetIndex( y - 1 ))
>              {
>                  --x;

makes the scenario from comment 0 then fail with (an undefined pointer addition
and a resulting) heap-buffer-overflow in my ASan+UBSan build, which appears to
be a strong indicator that the above guess may have been well-founded after
all, and the assert was just another legitimate symptom of whatever underlying
issue.

> include/c++/10.0.1/bits/unique_ptr.h:656:9: runtime error: addition of unsigned offset to 0x610000272440 overflowed to 0x610000272430
>  #0 in std::unique_ptr<unsigned long [], std::default_delete<unsigned long []> >::operator[](unsigned long) const at include/c++/10.0.1/bits/unique_ptr.h:656:9
>  #1 in (anonymous namespace)::Compare::MovedData::GetIndex(unsigned long) const at sw/source/core/doc/doccomp.cxx:214:58
>  #2 in (anonymous namespace)::Compare::CompareSequence::CheckDiag(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long*) at sw/source/core/doc/doccomp.cxx:917:27
>  #3 in (anonymous namespace)::Compare::CompareSequence::Compare(unsigned long, unsigned long, unsigned long, unsigned long) at sw/source/core/doc/doccomp.cxx:828:13
>  #4 in (anonymous namespace)::Compare::CompareSequence::CompareSequence((anonymous namespace)::CompareData&, (anonymous namespace)::CompareData&, (anonymous namespace)::Compare::MovedData const&, (anonymous namespace)::Compare::MovedData const&) at sw/source/core/doc/doccomp.cxx:791:5
>  #5 in (anonymous namespace)::Compare::Compare(unsigned long, (anonymous namespace)::CompareData&, (anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:602:25
>  #6 in (anonymous namespace)::CompareData::CompareLines((anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:437:17
>  #7 in SwDoc::CompareDoc(SwDoc const&) at sw/source/core/doc/doccomp.cxx:1866:13
>  #8 in SwEditShell::CompareDoc(SwDoc const&) at sw/source/core/edit/editsh.cxx:876:27
>  #9 in SwView::InsertMedium(unsigned short, std::unique_ptr<SfxMedium, std::default_delete<SfxMedium> >, short) at sw/source/uibase/uiview/view2.cxx:2300:39
>  #10 in SwView::DialogClosedHdl(sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2491:19
>  #11 in SwView::LinkStubDialogClosedHdl(void*, sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2481:1
>  #12 in Link<sfx2::FileDialogHelper*, void>::Call(sfx2::FileDialogHelper*) const at include/tools/link.hxx:111:45
>  #13 in sfx2::DocumentInserter::DialogClosedHdl(sfx2::FileDialogHelper*) at sfx2/source/doc/docinsert.cxx:285:25
>  #14 in sfx2::DocumentInserter::LinkStubDialogClosedHdl(void*, sfx2::FileDialogHelper*) at sfx2/source/doc/docinsert.cxx:184:1
>  #15 in Link<sfx2::FileDialogHelper*, void>::Call(sfx2::FileDialogHelper*) const at include/tools/link.hxx:111:45
>  #16 in sfx2::FileDialogHelper::ExecuteSystemFilePicker(void*) at sfx2/source/dialog/filedlghelper.cxx:2356:25
>  #17 in sfx2::FileDialogHelper::LinkStubExecuteSystemFilePicker(void*, void*) at sfx2/source/dialog/filedlghelper.cxx:2353:1
>  #18 in Link<void*, void>::Call(void*) const at include/tools/link.hxx:111:45
>  #19 in ImplHandleUserEvent(ImplSVEvent*) at vcl/source/window/winproc.cxx:2009:30
>  #20 in ImplWindowFrameProc(vcl::Window*, SalEvent, void const*) at vcl/source/window/winproc.cxx:2562:13
>  #21 in SalFrame::CallCallback(SalEvent, void const*) const at vcl/inc/salframe.hxx:306:29
>  #22 in SalGenericDisplay::ProcessEvent(SalUserEventList::SalUserEvent) at vcl/unx/generic/app/gendisp.cxx:66:22
>  #23 in SalUserEventList::DispatchUserEvents(bool) at vcl/source/app/salusereventlist.cxx:108:17
>  #24 in SalGenericDisplay::DispatchInternalEvent(bool) at vcl/unx/generic/app/gendisp.cxx:51:12
>  #25 in call_userEventFn(void*) at vcl/unx/gtk3/gtk3gtkdata.cxx:707:27
>  #26  at <null> (/lib64/libglib-2.0.so.0 +0x4de8a)
>  #27 in g_main_context_dispatch at <null> (/lib64/libglib-2.0.so.0 +0x5156f)
>  #28  at <null> (/lib64/libglib-2.0.so.0 +0x518ff)
>  #29 in g_main_context_iteration at <null> (/lib64/libglib-2.0.so.0 +0x519a2)
>  #30 in GtkSalData::Yield(bool, bool) at vcl/unx/gtk3/gtk3gtkdata.cxx:382:31
>  #31 in GtkInstance::DoYield(bool, bool) at vcl/unx/gtk3/gtk3gtkinst.cxx:384:29
>  #32 in ImplYield(bool, bool) at vcl/source/app/svapp.cxx:454:48
>  #33 in Application::Yield() at vcl/source/app/svapp.cxx:518:5
>  #34 in Application::Execute() at vcl/source/app/svapp.cxx:433:9
>  #35 in desktop::Desktop::Main() at desktop/source/app/app.cxx:1602:17
>  #36 in ImplSVMain() at vcl/source/app/svmain.cxx:196:35
>  #37 in SVMain() at vcl/source/app/svmain.cxx:228:12
>  #38 in soffice_main at desktop/source/app/sofficemain.cxx:107:12
>  #39 in sal_main at desktop/source/app/main.c:48:15
>  #40 in main at desktop/source/app/main.c:47:1
>  #41 in __libc_start_main at /usr/src/debug/glibc-2.30-34-g994e529a37/csu/../csu/libc-start.c:308:16
>  #42 in _start at <null>
> 
> SUMMARY: UndefinedBehaviorSanitizer: pointer-overflow include/c++/10.0.1/bits/unique_ptr.h:656:9 in 
> =================================================================
> ==2406221==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x610000272430 at pc 0x7f0db2c2790d bp 0x7ffcfd1c0870 sp 0x7ffcfd1c0868
> READ of size 8 at 0x610000272430 thread T0
>  #0 in (anonymous namespace)::Compare::MovedData::GetIndex(unsigned long) const at sw/source/core/doc/doccomp.cxx:214:58
>  #1 in (anonymous namespace)::Compare::CompareSequence::CheckDiag(unsigned long, unsigned long, unsigned long, unsigned long, unsigned long*) at sw/source/core/doc/doccomp.cxx:917:27
>  #2 in (anonymous namespace)::Compare::CompareSequence::Compare(unsigned long, unsigned long, unsigned long, unsigned long) at sw/source/core/doc/doccomp.cxx:828:13
>  #3 in (anonymous namespace)::Compare::CompareSequence::CompareSequence((anonymous namespace)::CompareData&, (anonymous namespace)::CompareData&, (anonymous namespace)::Compare::MovedData const&, (anonymous namespace)::Compare::MovedData const&) at sw/source/core/doc/doccomp.cxx:791:5
>  #4 in (anonymous namespace)::Compare::Compare(unsigned long, (anonymous namespace)::CompareData&, (anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:602:25
>  #5 in (anonymous namespace)::CompareData::CompareLines((anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:437:17
>  #6 in SwDoc::CompareDoc(SwDoc const&) at sw/source/core/doc/doccomp.cxx:1866:13
>  #7 in SwEditShell::CompareDoc(SwDoc const&) at sw/source/core/edit/editsh.cxx:876:27
>  #8 in SwView::InsertMedium(unsigned short, std::unique_ptr<SfxMedium, std::default_delete<SfxMedium> >, short) at sw/source/uibase/uiview/view2.cxx:2300:39
>  #9 in SwView::DialogClosedHdl(sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2491:19
>  #10 in SwView::LinkStubDialogClosedHdl(void*, sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2481:1
>  #11 in Link<sfx2::FileDialogHelper*, void>::Call(sfx2::FileDialogHelper*) const at include/tools/link.hxx:111:45
>  #12 in sfx2::DocumentInserter::DialogClosedHdl(sfx2::FileDialogHelper*) at sfx2/source/doc/docinsert.cxx:285:25
>  #13 in sfx2::DocumentInserter::LinkStubDialogClosedHdl(void*, sfx2::FileDialogHelper*) at sfx2/source/doc/docinsert.cxx:184:1
>  #14 in Link<sfx2::FileDialogHelper*, void>::Call(sfx2::FileDialogHelper*) const at include/tools/link.hxx:111:45
>  #15 in sfx2::FileDialogHelper::ExecuteSystemFilePicker(void*) at sfx2/source/dialog/filedlghelper.cxx:2356:25
>  #16 in sfx2::FileDialogHelper::LinkStubExecuteSystemFilePicker(void*, void*) at sfx2/source/dialog/filedlghelper.cxx:2353:1
>  #17 in Link<void*, void>::Call(void*) const at include/tools/link.hxx:111:45
>  #18 in ImplHandleUserEvent(ImplSVEvent*) at vcl/source/window/winproc.cxx:2009:30
>  #19 in ImplWindowFrameProc(vcl::Window*, SalEvent, void const*) at vcl/source/window/winproc.cxx:2562:13
>  #20 in SalFrame::CallCallback(SalEvent, void const*) const at vcl/inc/salframe.hxx:306:29
>  #21 in SalGenericDisplay::ProcessEvent(SalUserEventList::SalUserEvent) at vcl/unx/generic/app/gendisp.cxx:66:22
>  #22 in SalUserEventList::DispatchUserEvents(bool) at vcl/source/app/salusereventlist.cxx:108:17
>  #23 in SalGenericDisplay::DispatchInternalEvent(bool) at vcl/unx/generic/app/gendisp.cxx:51:12
>  #24 in call_userEventFn(void*) at vcl/unx/gtk3/gtk3gtkdata.cxx:707:27
>  #25  at <null> (/lib64/libglib-2.0.so.0 +0x4de8a)
>  #26 in g_main_context_dispatch at <null> (/lib64/libglib-2.0.so.0 +0x5156f)
>  #27  at <null> (/lib64/libglib-2.0.so.0 +0x518ff)
>  #28 in g_main_context_iteration at <null> (/lib64/libglib-2.0.so.0 +0x519a2)
>  #29 in GtkSalData::Yield(bool, bool) at vcl/unx/gtk3/gtk3gtkdata.cxx:382:31
>  #30 in GtkInstance::DoYield(bool, bool) at vcl/unx/gtk3/gtk3gtkinst.cxx:384:29
>  #31 in ImplYield(bool, bool) at vcl/source/app/svapp.cxx:454:48
>  #32 in Application::Yield() at vcl/source/app/svapp.cxx:518:5
>  #33 in Application::Execute() at vcl/source/app/svapp.cxx:433:9
>  #34 in desktop::Desktop::Main() at desktop/source/app/app.cxx:1602:17
>  #35 in ImplSVMain() at vcl/source/app/svmain.cxx:196:35
>  #36 in SVMain() at vcl/source/app/svmain.cxx:228:12
>  #37 in soffice_main at desktop/source/app/sofficemain.cxx:107:12
>  #38 in sal_main at desktop/source/app/main.c:48:15
>  #39 in main at desktop/source/app/main.c:47:1
>  #40 in __libc_start_main at /usr/src/debug/glibc-2.30-34-g994e529a37/csu/../csu/libc-start.c:308:16
>  #41 in _start at <null>
> 
> 0x610000272430 is located 16 bytes to the left of 192-byte region [0x610000272440,0x610000272500)
> allocated by thread T0 here:
>  #0 in operator new[](unsigned long) at compiler-rt/lib/asan/asan_new_delete.cpp:102:3
>  #1 in (anonymous namespace)::Compare::MovedData::MovedData((anonymous namespace)::CompareData&, char const*) at sw/source/core/doc/doccomp.cxx:768:25
>  #2 in (anonymous namespace)::Compare::Compare(unsigned long, (anonymous namespace)::CompareData&, (anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:597:24
>  #3 in (anonymous namespace)::CompareData::CompareLines((anonymous namespace)::CompareData&) at sw/source/core/doc/doccomp.cxx:437:17
>  #4 in SwDoc::CompareDoc(SwDoc const&) at sw/source/core/doc/doccomp.cxx:1866:13
>  #5 in SwEditShell::CompareDoc(SwDoc const&) at sw/source/core/edit/editsh.cxx:876:27
>  #6 in SwView::InsertMedium(unsigned short, std::unique_ptr<SfxMedium, std::default_delete<SfxMedium> >, short) at sw/source/uibase/uiview/view2.cxx:2300:39
>  #7 in SwView::DialogClosedHdl(sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2491:19
>  #8 in SwView::LinkStubDialogClosedHdl(void*, sfx2::FileDialogHelper*) at sw/source/uibase/uiview/view2.cxx:2481:1
>  #9 in Link<sfx2::FileDialogHelper*, void>::Call(sfx2::FileDialogHelper*) const at include/tools/link.hxx:111:45
> 
> SUMMARY: AddressSanitizer: heap-buffer-overflow sw/source/core/doc/doccomp.cxx:214:58 in (anonymous namespace)::Compare::MovedData::GetIndex(unsigned long) const
> Shadow bytes around the buggy address:
>   0x0c2080046430: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c2080046440: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c2080046450: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
>   0x0c2080046460: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
>   0x0c2080046470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> =>0x0c2080046480: fa fa fa fa fa fa[fa]fa 00 00 00 00 00 00 00 00
>   0x0c2080046490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>   0x0c20800464a0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>   0x0c20800464b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
>   0x0c20800464c0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
>   0x0c20800464d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> Shadow byte legend (one shadow byte represents 8 application bytes):
>   Addressable:           00
>   Partially addressable: 01 02 03 04 05 06 07 
>   Heap left redzone:       fa
>   Freed heap region:       fd
>   Stack left redzone:      f1
>   Stack mid redzone:       f2
>   Stack right redzone:     f3
>   Stack after return:      f5
>   Stack use after scope:   f8
>   Global redzone:          f9
>   Global init order:       f6
>   Poisoned by user:        f7
>   Container overflow:      fc
>   Array cookie:            ac
>   Intra object redzone:    bb
>   ASan internal:           fe
>   Left alloca redzone:     ca
>   Right alloca redzone:    cb
>   Shadow gap:              cc
> ==2406221==ABORTING

I've put on CC some people who know the Writer code in general and have touched
sw/source/core/doc/doccomp.cxx in the past, in the hope that one of them might
have an idea what's going on here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/libreoffice-bugs/attachments/20200406/e4d886da/attachment-0001.htm>


More information about the Libreoffice-bugs mailing list