webkit_layout_tests failing on chromium.webkit/WebKit Linux Trusty ASAN Builders failed on: - WebKit Linux Trusty ASAN: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20ASAN Failures: * editing/undo/undo-delete-boundary.html * editing/undo/undo-delete.html
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20ASAN/builds/3964 ==1:1==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7f925efeee40 at pc 0x0000004d6815 bp 0x7ffd629cb690 sp 0x7ffd629cae40 READ of size 16 at 0x7f925efeee40 thread T0 (content_shell) #0 0x4d6814 in __asan_memcpy ??:0:0 #1 0x9833d06 in blink::SelectionForUndoStep::operator=(blink::SelectionForUndoStep const&) third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp:37:9 #2 0x984cd7e in blink::UndoStep::SetStartingSelection(blink::SelectionForUndoStep const&) third_party/WebKit/Source/core/editing/commands/UndoStep.cpp:93:23 #3 0x97c7d6a in blink::CompositeEditCommand::SetStartingSelection(blink::SelectionForUndoStep const&) third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp:1910:18 #4 0x9848933 in blink::TypingCommand::DeleteKeyPressedInternal(blink::VisibleSelectionTemplate<blink::EditingAlgorithm<blink::NodeTraversal> > const&, blink::SelectionForUndoStep const&, bool, blink::EditingState*) third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp:876:5 #5 0x98419a2 in blink::TypingCommand::DeleteKeyPressed(blink::TextGranularity, bool, blink::EditingState*) third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp:847:3 #6 0x98408f2 in blink::TypingCommand::DeleteKeyPressed(blink::Document&, unsigned int, blink::TextGranularity) third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp:210:30 #7 0x97e22df in blink::ExecuteDelete(blink::LocalFrame&, blink::Event*, blink::EditorCommandSource, WTF::String const&) third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp:725:7 #8 0x97db38f in blink::Document::execCommand(WTF::String const&, bool, WTF::String const&, blink::ExceptionState&) third_party/WebKit/Source/core/editing/commands/DocumentExecCommand.cpp:92:25 #9 0x84911d2 in execCommandMethod ./out/Release/gen/blink/bindings/core/v8/V8Document.cpp:3705:23 #10 0x84911d2 in blink::V8Document::execCommandMethodCallback(v8::FunctionCallbackInfo<v8::Value> const&) ./out/Release/gen/blink/bindings/core/v8/V8Document.cpp:5878:0 #11 0x158e4ad in v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) v8/src/api-arguments.cc:25:3 #12 0x1775745 in v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) v8/src/builtins/builtins-api.cc:112:36
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d8242f55d3861d817d70a12d249e1bb8357340fe commit d8242f55d3861d817d70a12d249e1bb8357340fe Author: Tsuyoshi Horo <horo@chromium.org> Date: Thu Aug 03 08:56:04 2017 Revert "Introduce SelectionForUndoStep" This reverts commit 44e59041eab4efb2d9cae5e3e7e5623963dd131a. Reason for revert: Caused LayoutTest failures on ASAN bot. BUG:752013 Original change's description: > Introduce SelectionForUndoStep > > This patch introduces |SelectionForUndoStep| to hold selection for undo/redo > instead of |VisibleSelection| and avoid dirty |VisibleSelection|[1] for > improving code health. > > Before this patch, we hold selection as |VisibleSelection| in |UndoStep| and > recompute "visible selection" by |CorrectedSelectionAfterCommand()| at undo/redo > time, DOM tree can be different when we save |VisibleSelection| in |UndoStep|. > > After this patch, we use |SelectionForUndoStep| to hold selection at command > execution and change |EndingVisibleSelection()| to compute visible selection > before we used. > > This patch looks large, but most of them are replacing > |EndingVisibleSelection()| to |EndingSelection()| to use |SelectionInUndoStep| > instead of updated |VisibleSelection|. > > Essential part of this change is > * "CompositeEditCommand.{cpp,h}": holding starting and ending selection in > |SelectionInUndoStep| > > * "TypginCommand.{cpp,h}": Computing |SelectionInUndoStep| other than > |FrameSelection|. > * "SelectionInUndoStep.{cpp.h}": The implementation > * "UndoStep.{cpp,h}": holding staring and ending seleciton in > |SelectionInUndoStep| > > Just for reference, there is another attempt in the patch: > - http://crrev.com/2457523004: Introduce SelectionForUndoStep > - http://crrev.com/c/582724 Introduce SelectionForUndoStep > > [1] Dirty VisibleSelection: |VisibleSelection| holds canonicalized base and > extent positions by using layout tree. Once layout tree is updated, e.g. by > DOM change or style change, |VisibleSelection| may not be canonicalized. In > other words, once we get |VisibleSelection|, it is valid only until layout tree > updated. > > Bug: 747795 > Change-Id: I2da864e03fbd2e06f5150072eafe133430dcbfcb > Reviewed-on: https://chromium-review.googlesource.com/588874 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491667} TBR=yosin@chromium.org,yoichio@chromium.org,xiaochengh@chromium.org Change-Id: I84188673f225161fc65da6ad1693a1721d2d8f5a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 747795 Reviewed-on: https://chromium-review.googlesource.com/599667 Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Commit-Queue: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#491683} [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [delete] https://crrev.com/75f445688a893bf2da56ba664e1155628f5601ce/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [delete] https://crrev.com/75f445688a893bf2da56ba664e1155628f5601ce/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UndoStep.h [modify] https://crrev.com/d8242f55d3861d817d70a12d249e1bb8357340fe/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp
Public link of bot failures: https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20ASAN/builds/3964
It seems this is clang's bug. Generated codes deallocates |const SelectionForUndoStep&|. 840 const SelectionForUndoStep& selection_after_undo = 841 SelectionForUndoStep::Builder() 842 .SetBaseAndExtentAsBackwardSelection( 843 StartingSelection().End(), 844 CreateVisiblePosition(selection_to_delete.Extent()) 845 .DeepEquivalent()) 846 .Build(); 847 DeleteKeyPressedInternal(selection_to_delete, selection_after_undo, kill_ring, 848 editing_state);
In review: https://chromium-review.googlesource.com/c/600007 Change |const SelectionForUndoStep&| to |const SelectionForUndoStep| works.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f471810d59597fda437adc2db434a2f5f1e58cfc commit f471810d59597fda437adc2db434a2f5f1e58cfc Author: Yoshifumi Inoue <yosin@chromium.org> Date: Thu Aug 03 13:01:50 2017 Reland "Introduce SelectionForUndoStep" This is a reland of 44e59041eab4efb2d9cae5e3e7e5623963dd131a Changes of original is replace |const SelectionForUndo&| to |const SelectionForUndo| at end of DeleteKeyPressed() and ForwardDeleteKeyPressed() as work around of clang's bug which doesn't handle extent of |const T&| as end of scope. Original change's description: > Introduce SelectionForUndoStep > > This patch introduces |SelectionForUndoStep| to hold selection for undo/redo > instead of |VisibleSelection| and avoid dirty |VisibleSelection|[1] for > improving code health. > > Before this patch, we hold selection as |VisibleSelection| in |UndoStep| and > recompute "visible selection" by |CorrectedSelectionAfterCommand()| at undo/redo > time, DOM tree can be different when we save |VisibleSelection| in |UndoStep|. > > After this patch, we use |SelectionForUndoStep| to hold selection at command > execution and change |EndingVisibleSelection()| to compute visible selection > before we used. > > This patch looks large, but most of them are replacing > |EndingVisibleSelection()| to |EndingSelection()| to use |SelectionInUndoStep| > instead of updated |VisibleSelection|. > > Essential part of this change is > * "CompositeEditCommand.{cpp,h}": holding starting and ending selection in > |SelectionInUndoStep| > > * "TypginCommand.{cpp,h}": Computing |SelectionInUndoStep| other than > |FrameSelection|. > * "SelectionInUndoStep.{cpp.h}": The implementation > * "UndoStep.{cpp,h}": holding staring and ending seleciton in > |SelectionInUndoStep| > > Just for reference, there is another attempt in the patch: > - http://crrev.com/2457523004: Introduce SelectionForUndoStep > - http://crrev.com/c/582724 Introduce SelectionForUndoStep > > [1] Dirty VisibleSelection: |VisibleSelection| holds canonicalized base and > extent positions by using layout tree. Once layout tree is updated, e.g. by > DOM change or style change, |VisibleSelection| may not be canonicalized. In > other words, once we get |VisibleSelection|, it is valid only until layout tree > updated. > > Bug: 747795 > Change-Id: I2da864e03fbd2e06f5150072eafe133430dcbfcb > Reviewed-on: https://chromium-review.googlesource.com/588874 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#491667} TBR=xiaochengh@chromium.org Bug: 747795 , 752013 Change-Id: I3135599416fea2af25aeba1ce76348dbf3b36628 Reviewed-on: https://chromium-review.googlesource.com/600007 Reviewed-by: Yoshifumi Inoue <yosin@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#491715} [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/BUILD.gn [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/BreakBlockquoteCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/CreateLinkCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/DeleteSelectionCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertIncrementalTextCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertLineBreakCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/InsertTextCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp [add] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.cpp [add] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/SelectionForUndoStep.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/TypingCommand.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UndoStep.cpp [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UndoStep.h [modify] https://crrev.com/f471810d59597fda437adc2db434a2f5f1e58cfc/third_party/WebKit/Source/core/editing/commands/UnlinkCommand.cpp
Comment 1 by horo@chromium.org
, Aug 3 2017