Bug 46886 – [GTK] Crash on testatk.c unittest because of a call to ASSERT_NOT_REACHED

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Mario Sanchez Prada]

It looks that the following crash has been happening in the Debug build of the GTK port since the patch for bug 45381 was applied (see http://trac.webkit.org/changeset/68415):

Program terminated with signal 11, Segmentation fault.
#0  0x55fb8d13 in listMarkerSuffix (type=WebCore::Disc, value=1)
    at ../../WebCore/rendering/RenderListMarker.cpp:507
507	        ASSERT_NOT_REACHED();

This is caused due to the new function WebCore::RenderListMarker::suffix() added along with that patch, which uses the internal function listMarkerSuffix() to retrieve the marker's suffix and return its value.

Adding Daniel Bates to CC as its name is in commit revision 53452 (bug 33089), where the original code was introduced.

[Mario Sanchez Prada]

Created attachment 69325 [details]
Patch proposal

Attaching a patch proposal that just gets rid of the ASSERT_NOT_REACHED() statement, as it's my understanding that's no longer correct. As explained in the ChangeLog entry:

        So far, this assertion made sense because it was not possible that
        a call to listMarkerSuffix() happened when the style of the list
        item was one of the following: Asterisks, Circle, Discm Footnotes,
        NoneListStyle or Square (it's easy to figure this out by checking
        the functions where listMarkerSuffix() was called).

        However, since revision 68415 (about bug 45381), there's a new
        place where listMarkerSuffix() is being called (the suffix()
        public method), regardless of the style of the item being or not
        one of those pointed out, so the aforementioned assertion in
        listMarkerSuffix() would no longer be correct, as now it's ok to
        reach those cases in the switch statement.


Daniel, would you mind taking a look to this. I guess you're probably one of the best persons to say whether this approach is correct or not.

Thanks in advance

[Daniel Bates]

Comment on attachment 69325 [details]
Patch proposal

This seems reasonable to me.

[Martin Robinson]

Comment on attachment 69325 [details]
Patch proposal

Setting the cq bit, since Mario is still waiting on his committer access.

[WebKit Commit Bot]

Comment on attachment 69325 [details]
Patch proposal

Clearing flags on attachment: 69325

Committed r68858: <http://trac.webkit.org/changeset/68858>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.