Bug 137542 - SIGSEGV on tab cycling focus to input field in a table cell
Summary: SIGSEGV on tab cycling focus to input field in a table cell
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
5.3.6.1 release
Hardware: All Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.6.0 target:7.5.3.2 target:7.4.7
Keywords: haveBacktrace
Depends on:
Blocks: Crash
  Show dependency treegraph
 
Reported: 2020-10-16 18:20 UTC by charkins
Modified: 2023-05-03 10:20 UTC (History)
3 users (show)

See Also:
Crash report or crash signature: ["SwFieldInputDlg::SwFieldInputDlg(weld::Widget*, SwWrtShell&, SwField*, bool, bool)"]


Attachments
minimum example to trigger bug (8.38 KB, application/vnd.oasis.opendocument.text)
2020-10-16 18:20 UTC, charkins
Details
backtrace from crash (12.68 KB, text/x-log)
2020-10-16 18:21 UTC, charkins
Details
bt with debug symbols (6.00 KB, text/plain)
2020-10-17 09:19 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description charkins 2020-10-16 18:20:59 UTC
Created attachment 166437 [details]
minimum example to trigger bug

Attached is a minimum working example to demonstrate the crash. This appears to be caused by an interaction between focus traversal of input fields and table cells.

To trigger the crash, it appears these conditions must exist:

1. focus is in an input field (e.g. added via CTRL-F2)
2. the focused input field is inside of a table cell
3. text is selected within the focused input field
4. the next input field that would receive focus is also in a table cell
5. the next input field has content before the input field in the table cell

For example, click in the first text field once, which should focus the first text field with the text (all spaces) highlighted. Next press TAB to cycle focus to the second text field to crash.


The crash has been confirmed on at least these libreoffice versions:

    libreoffice-writer-5.3.6.1-24.el7.x86_64
    libreoffice-writer-6.0.6.1-20.el8.x86_64
    libreoffice-writer-6.4.6.2-3.fc32.x86_64
Comment 1 charkins 2020-10-16 18:21:54 UTC
Created attachment 166439 [details]
backtrace from crash
Comment 2 Julien Nabet 2020-10-17 09:19:27 UTC
Created attachment 166457 [details]
bt with debug symbols

On pc Debian x86-64 with master sources updated today, I could reproduce this.
Comment 3 charkins 2020-10-29 20:16:34 UTC
I am not really familiar with the LO code base, but tried to at least narrow down the source of the crash. When triggering the focus change, SwTextShell::ExecField is called and triggers this case:

        case FN_GOTO_NEXT_INPUTFLD:
        case FN_GOTO_PREV_INPUTFLD:
            {
                bool bRet = false;
                SwFieldType* pField = rSh.GetFieldType( 0, SwFieldIds::Input );
                const bool bAddSetExpressionFields = !( rSh.GetViewOptions()->IsReadonly() );
                if ( pField != nullptr
                     && rSh.MoveFieldType(
                            pField,
                            FN_GOTO_NEXT_INPUTFLD == nSlot,
                            SwFieldIds::Unknown,
                            bAddSetExpressionFields ) )
                {
                    rSh.ClearMark();
                    if (!rSh.IsMultiSelection()
                        && (nullptr != dynamic_cast<const SwTextInputField*>(
                               SwCursorShell::GetTextFieldAtCursor(rSh.GetCursor(), true))))
                    {
                        rSh.SttSelect();
                        rSh.SelectText(
                            SwCursorShell::StartOfInputFieldAtPos( *(rSh.GetCursor()->Start()) ) + 1,
                            SwCursorShell::EndOfInputFieldAtPos( *(rSh.GetCursor()->Start()) ) - 1 );
                    }
                    else
                    {
                        rSh.StartInputFieldDlg(rSh.GetCurField(true), false, false, GetView().GetFrameWeld());
                    }
                    bRet = true;
                }

                rReq.SetReturnValue( SfxBoolItem( nSlot, bRet ));
            }
            break;


In the working case, SwCursorShell::GetTextFieldAtCursor(...) != nullptr and the nested if condition is true and focus changes without problem. In the broken case, SwCursorShell::GetTextFieldAtCursor(...) == nullptr, triggering the else case of the nested if, eventually leading to the segfault. Digging further into the GetTextFieldAtCursor code path, the difference arises down in lcl_GetTextAttrs in sw/source/core/txtnode/ndtxt.cxx:

static void
lcl_GetTextAttrs(
    std::vector<SwTextAttr *> *const pVector,
    SwTextAttr **const ppTextAttr,
    SwpHints const *const pSwpHints,
    sal_Int32 const nIndex, sal_uInt16 const nWhich,
    enum SwTextNode::GetTextAttrMode const eMode)

When this function is called during focus change in the working case, pSwpHints has a single hint which is assigned to *ppTextAttr, which is then used to get the GetTextFieldAtCursor return value. In the broken case (with text preceding the second input field), pSwpHints has TWO hints, with the hint from the working case at index 1 and another hint at index 0. This block of code in that function breaks early in the broken case:


        sal_Int32 const nHintStart = pHint->GetStart();
        if (nIndex < nHintStart)
            break; // hints are sorted by which&start, so we are done...

nIndex=0 and nHintStart=0 in the working case, but nIndex=0 and nHintStart=1 in the broken case. Hopefully this will hope narrow things down for someone familiar with this code. It might be a while before I can get back to it!
Comment 4 Julien Nabet 2020-10-30 21:08:58 UTC
Seeing you spent some time to investigate this bug, thought you might be interested in submitting a first version of a patch? (see https://wiki.documentfoundation.org/Development/GetInvolved for more info).
Indeed, I supposed you've already:
- downloaded the code
- built it to find the part you quoted

So you just have to submit an ad hoc license statement + create gerrit account.
You may also discuss this on dev forum or IRC
Comment 5 QA Administrators 2022-10-31 03:33:38 UTC Comment hidden (obsolete)
Comment 6 charkins 2022-11-01 16:58:13 UTC
Bug is still present.


Version: 7.3.6.2
Build ID: 30(Build:2)
CPU threads: 8; OS: Linux 5.19; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

libreoffice-writer-7.3.6.2-3.fc36.x86_64
Comment 7 charkins 2023-04-13 14:04:53 UTC
Bug is still present in libreoffice-writer-7.4.6.2-2.fc37.x86_64:

Version: 7.4.6.2
Build ID: 40(Build:2)
CPU threads: 8; OS: Linux 6.2; UI render: default; VCL: gtk3
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded
Comment 8 Commit Notification 2023-04-17 08:03:56 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5692a58cbfbd5da33b37415383f6eafb80d79177

tdf#137542 don't crash at least

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Caolán McNamara 2023-04-17 08:09:43 UTC
lets at least no crash, done in trunk, backport to 7-5 and 7-4 in gerrit
Comment 10 Commit Notification 2023-04-17 11:02:25 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-5":

https://git.libreoffice.org/core/commit/508cc73f5301abc00d1ed75e331a22eb01a65f09

tdf#137542 don't crash at least

It will be available in 7.5.4.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2023-04-17 11:19:33 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-4":

https://git.libreoffice.org/core/commit/c59204ebc40950147926cc241c3292321abbf444

tdf#137542 don't crash at least

It will be available in 7.4.7.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Stéphane Guillou (stragu) 2023-04-25 21:41:36 UTC
Crash reproduced in 7.3.7.2, report in https://crashreport.libreoffice.org/stats/crash_details/7a386ea6-7d3a-4cd9-b0a9-4f01661ce1bb
Fix verified in:

Version: 7.6.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 5cd9de202765e243e41416802f3e4486b8a96f16
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded

Could not crash when navigating with tab.
Comment 13 Commit Notification 2023-04-26 11:06:53 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-5-3":

https://git.libreoffice.org/core/commit/b4b13f0bbf95497c7addd7026c602074aa0725de

tdf#137542 don't crash at least

It will be available in 7.5.3.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.