[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f820fa4feda: [lldb] Fix SourceManager::SourceFileCache 
insertion (authored by emrekultursay, committed by labath).

Changed prior to commit:
  https://reviews.llvm.org/D76805?vs=252881&id=258737#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

In D76805#1949642 , @labath wrote:

> Ok, that makes kind of sense, though I am left wondering if we really need 
> this feature, given that we have survived so long without noticing it is 
> missing...
>
> Am I understanding it correctly that without this patch, we would only cache 
> the most recently accessed file (via `m_last_file_sp` member), and would 
> always reload when switching to a new file?


Yes, without this patch, only the most recently accessed file is cached inside 
that member, and switching to a new file replaces that entry. I guess this was 
designed for optimizing the case where the user hits "next line" while staying 
in the same file. Yet, if a breakpoint on a different file is hit during that 
"next line" command, we trigger a reload (twice).

This patch will increase memory usage, as mapped source files will stay in the 
cache, and thus, never be unmapped. The increase will be and proportional to 
the size of source files loaded by LLDB on-demand.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D76805#1944198 , @emrekultursay 
wrote:

> > Does this actually depend on the other patch? It looks like an independent 
> > fix we could commit separately.
>
> This bug seems to have existed forever. Fixing it means we will have source 
> file cache enabled for the first time. If it causes any unforeseen issues, 
> I'd like users to have the ability to disable the cache, which is why I made 
> this change depend on the other change.


Ok, that makes kind of sense, though I am left wondering if we really need this 
feature, given that we have survived so long without noticing it is missing...

Am I understanding it correctly that without this patch, we would only cache 
the most recently accessed file (via `m_last_file_sp` member), and would always 
reload when switching to a new file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252881.
emrekultursay marked 3 inline comments as done.
emrekultursay added a comment.

Applied labath@'s suggestions on test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, 
nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,48 @@
+//===-- SourceManagerTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo, expect found.
+  FileSpec another_foo_file_spec("foo");
+  ASSERT_EQ(cache.FindSourceFile(another_foo_file_spec), foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar, expect not found.
+  FileSpec bar_file_spec("bar");
+  ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  Sou

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay added a comment.

> Does this actually depend on the other patch? It looks like an independent 
> fix we could commit separately.

This bug seems to have existed forever. Fixing it means we will have source 
file cache enabled for the first time. If it causes any unforeseen issues, I'd 
like users to have the ability to disable the cache, which is why I made this 
change depend on the other change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath marked an inline comment as done.
labath added a comment.
This revision is now accepted and ready to land.

Looks good. I'm just picking some nits in the test. I'm assuming that 
@jingham's comment refers to the other patch (and it more-or-less matches what 
I wrote there already).

Does this actually depend on the other patch? It looks like an independent fix 
we could commit separately.




Comment at: lldb/source/Core/SourceManager.cpp:699
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
   FileCache::iterator pos = m_file_cache.find(file_spec);

Woops :)



Comment at: lldb/unittests/Core/SourceManagerTest.cpp:11
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/RegularExpression.h"
+

This doesn't appear to be needed.



Comment at: lldb/unittests/Core/SourceManagerTest.cpp:24-31
+TEST_F(SourceFileCache, AddSourceFile) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec file_spec("foo");
+  auto file_sp = std::make_shared(file_spec, nullptr);
+  cache.AddSourceFile(file_sp);

A test without any assertion is weird. I'd recommend just deleting this since 
insertion is already tested in the other tests.



Comment at: lldb/unittests/Core/SourceManagerTest.cpp:60-63
+  SourceManager::FileSP result = cache.FindSourceFile(bar_file_spec);
+
+  // Expect not found.
+  ASSERT_EQ(result, nullptr);

I'd recommend folding these two statements into one 
(ASSERT_EQ(cache.FindSourceFile(...), nullptr)). That way we'll have at least a 
semi-reasonable error message when this fails instead of a "0xdeadbeef != 0"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252734.
emrekultursay added a comment.
Herald added a subscriber: mgorny.

Add unit tests for SourceManager::SourceFileCache


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/SourceManagerTest.cpp


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,64 @@
+//===-- SourceManagerTest.cpp 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/RegularExpression.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, AddSourceFile) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec file_spec("foo");
+  auto file_sp = std::make_shared(file_spec, nullptr);
+  cache.AddSourceFile(file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, 
nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo
+  FileSpec another_foo_file_spec("foo");
+  SourceManager::FileSP result = cache.FindSourceFile(another_foo_file_spec);
+
+  // Expect found.
+  ASSERT_EQ(result, foo_file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileNotFound) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp =
+  std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+   
+  // Query: bar
+  FileSpec bar_file_spec("bar");
+  SourceManager::FileSP result = cache.FindSourceFile(bar_file_spec);
+
+  // Expect not found.
+  ASSERT_EQ(result, nullptr);
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
+  SourceManagerTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/unittests/Core/SourceManagerTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/SourceManagerTest.cpp
@@ -0,0 +1,64 @@
+//===-- SourceManagerTest.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/SourceManager.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Utility/RegularExpression.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+class SourceFileCache : public ::testing::Test {
+public:
+  void SetUp() override { FileSystem::Initialize(); }
+  void TearDown() override { FileSystem::Terminate(); }
+};
+
+TEST_F(SourceFileCache, AddSourceFile) {
+  SourceManager::SourceFileCache cache;
+
+  // Insert: foo
+  FileSpec file_spec("foo");
+  auto file_sp = std::make_shared(file_spec, nullptr);
+  cache.AddSourceFile(file_sp);
+}
+
+TEST_F(SourceFileCache, FindSourceFileFound) {
+  SourceManager::SourceFileCache cache;  
+  
+  // Insert: foo
+  FileSpec foo_file_spec("foo");
+  auto foo_file_sp = std::make_shared(foo_file_spec, nullptr);
+  cache.AddSourceFile(foo_file_sp);
+
+  // Query: foo
+  FileSpec another_foo_file_spec("foo");
+  SourceManager::FileSP result = cache.FindSourceFile(another_foo_file_spec);
+
+  // Expect found.
+  ASSERT_EQ(result, foo_file_sp);

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

IIUC, you should be able to test the actual effect you are trying to achieve by 
having a large source file, running "source list" to get it into the File 
Manager, then try to open the source file for writing in a process in a 
subshell that won't have the same mmap.  That should fail without the patch (or 
with the setting turned to cache-on) and succeed with it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

Could we add a unit test for this?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay updated this revision to Diff 252702.
emrekultursay added a comment.

fix link warning


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76805/new/

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D76805: Fix SourceManager::SourceFileCache insertion

[Emre Kultursay via Phabricator via lldb-commits]

emrekultursay created this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
emrekultursay added a child revision: D76806: Remove m_last_file_sp from 
SourceManager.

Lookup and subsequent insert was done using uninitialized
FileSpec object, which caused the cache to be a no-op.

Depends on D76804 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76805

Files:
  lldb/source/Core/SourceManager.cpp


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();;
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;


Index: lldb/source/Core/SourceManager.cpp
===
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -696,7 +696,7 @@
 }
 
 void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) {
-  FileSpec file_spec;
+  FileSpec file_spec = file_sp->GetFileSpec();;
   FileCache::iterator pos = m_file_cache.find(file_spec);
   if (pos == m_file_cache.end())
 m_file_cache[file_spec] = file_sp;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits