From 5c347c49f8b4817cb2a1bd4c4c690e8539ab2b13 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <var-const@users.noreply.github.com>
Date: Thu, 29 Aug 2019 14:14:09 -0400
Subject: [PATCH] Fix accessing a destroyed object (#3721)

This was caused by running under Asan with XCode 11. Apparently, previous versions of XCode cannot catch it (which is why Travis is passing, apparently).

Also:
* enable "Detect use of stack after return" option for Asan. AFAIU, it can only be enabled in the scheme. It's already enabled for unit tests, this PR additionally enables it for integration tests;
* make Asan failures fail Travis build. They were originally not considered blocking due to some sporadic failures. I haven't seen those for some time, and worst case scenario, we can revert this change.
---
 .travis.yml                                   |  4 ---
 .../Firestore_IntegrationTests_iOS.xcscheme   | 25 ++++++++-----------
 .../core/src/firebase/firestore/core/view.cc  |  3 ++-
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 9b024e6a526..12c9198d3ee 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -447,12 +447,8 @@ jobs:
     # need to make them fatal for the purposes of the test run.
 
   # TODO(varconst): disallow sanitizers to fail once we fix all existing issues.
-    - env:
-      - PROJECT=Firestore PLATFORM=macOS METHOD=cmake SANITIZERS=asan
     - env:
       - PROJECT=Firestore PLATFORM=macOS METHOD=cmake SANITIZERS=tsan
-    - env:
-      - PROJECT=Firestore PLATFORM=iOS METHOD=xcodebuild SANITIZERS=asan
     - env:
       - PROJECT=Firestore PLATFORM=iOS METHOD=xcodebuild SANITIZERS=tsan
     - env:
diff --git a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme
index 03e9133eefe..f4896bc192e 100644
--- a/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme
+++ b/Firestore/Example/Firestore.xcodeproj/xcshareddata/xcschemes/Firestore_IntegrationTests_iOS.xcscheme
@@ -26,7 +26,17 @@
       buildConfiguration = "Debug"
       selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
       selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
-      shouldUseLaunchSchemeArgsEnv = "YES">
+      shouldUseLaunchSchemeArgsEnv = "YES"
+      enableASanStackUseAfterReturn = "YES">
+      <MacroExpansion>
+         <BuildableReference
+            BuildableIdentifier = "primary"
+            BlueprintIdentifier = "DE03B2941F2149D600A30B9C"
+            BuildableName = "Firestore_IntegrationTests_iOS.xctest"
+            BlueprintName = "Firestore_IntegrationTests_iOS"
+            ReferencedContainer = "container:Firestore.xcodeproj">
+         </BuildableReference>
+      </MacroExpansion>
       <Testables>
          <TestableReference
             skipped = "NO">
@@ -39,17 +49,6 @@
             </BuildableReference>
          </TestableReference>
       </Testables>
-      <MacroExpansion>
-         <BuildableReference
-            BuildableIdentifier = "primary"
-            BlueprintIdentifier = "DE03B2941F2149D600A30B9C"
-            BuildableName = "Firestore_IntegrationTests_iOS.xctest"
-            BlueprintName = "Firestore_IntegrationTests_iOS"
-            ReferencedContainer = "container:Firestore.xcodeproj">
-         </BuildableReference>
-      </MacroExpansion>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </TestAction>
    <LaunchAction
       buildConfiguration = "Debug"
@@ -70,8 +69,6 @@
             ReferencedContainer = "container:Firestore.xcodeproj">
          </BuildableReference>
       </MacroExpansion>
-      <AdditionalOptions>
-      </AdditionalOptions>
    </LaunchAction>
    <ProfileAction
       buildConfiguration = "Release"
diff --git a/Firestore/core/src/firebase/firestore/core/view.cc b/Firestore/core/src/firebase/firestore/core/view.cc
index 14f8399c548..6ec2fa5e1bc 100644
--- a/Firestore/core/src/firebase/firestore/core/view.cc
+++ b/Firestore/core/src/firebase/firestore/core/view.cc
@@ -212,7 +212,8 @@ ViewDocumentChanges View::ComputeDocumentChanges(
   if (limit != Query::kNoLimit &&
       new_document_set.size() > static_cast<size_t>(limit)) {
     for (size_t i = new_document_set.size() - limit; i > 0; --i) {
-      const Document& old_doc = *new_document_set.GetLastDocument();
+      absl::optional<Document> found = new_document_set.GetLastDocument();
+      const Document& old_doc = *found;
       new_document_set = new_document_set.erase(old_doc.key());
       new_mutated_keys = new_mutated_keys.erase(old_doc.key());
       change_set.AddChange(