[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack added a comment.

@jasonmolenda thank you for looking into this!
I was finally able to repro the test failure on arm64 macOS with this diff (the 
version with `ASSERT_EQ(a_file_spec.GetPath(), b_file_spec.GetPath()`) and this 
CMake config.

  cmake \
  ../llvm \
  -G Ninja \
  -DCMAKE_BUILD_TYPE=RelWithDebInfo \
  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
  -DLLVM_TARGETS_TO_BUILD=X86;ARM;AArch64 \
  -DLLDB_TEST_USER_ARGS="--build-dir;$(pwd);-t;--env;TERM=vt100" \
  -DLLDB_ENFORCE_STRICT_TEST_REQUIREMENTS=Off \
  -DLLDB_ENABLE_PYTHON=On \
  -DLLVM_ENABLE_ASSERTIONS:BOOL=TRUE \
  -DLLVM_ENABLE_MODULES=On \
  -DLLVM_ENABLE_PROJECTS="clang;lld;lldb;cross-project-tests" \
  -DLLVM_LIT_ARGS="-v --time-tests --shuffle 
--xunit-xml-output=$(pwd)/results.xml -v -j 8" \
  -DLLVM_VERSION_PATCH=99 \
  -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;compiler-rt" \
  -DPython3_EXECUTABLE=/usr/bin/python3 \
  -DSWIG_EXECUTABLE=/opt/brew/Cellar/swig@3/3.0.12/bin/swig



   TEST 'lldb-unit :: Target/./TargetTests/6/11' FAILED 

  Script(shard):
  --
  
GTEST_OUTPUT=json:/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests-lldb-unit-45687-6-11.json
 GTEST_SHUFFLE=1 GTEST_TOTAL_SHARDS=11 GTEST_SHARD_INDEX=6 
GTEST_RANDOM_SEED=20766 
/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests
  --
  
  Script:
  --
  
/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/./TargetTests
 --gtest_filter=LocateModuleCallbackTest.GetOrCreateModuleWithCachedModule
  --
  
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:152:
 Failure
  Expected equality of these values:
a_file_spec.GetPath()
  Which is: 
"/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/Inputs/AndroidModule.unstripped.so"
b_file_spec.GetPath()
  Which is: 
"/var/folders/hv/pgk33lbd2snf__0bd0v30yycgn/T/lit-tmp-qrocj9y_/lldb/45872/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModule/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC/AndroidModule.so"
  
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:280:
 Failure
  Value of: module_sp->GetSymbolFileFileSpec()
Actual: true
  Expected: false
  
  
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:152
  Expected equality of these values:
a_file_spec.GetPath()
  Which is: 
"/Users/user/Sources/llvm-project/build/tools/lldb/unittests/Target/Inputs/AndroidModule.unstripped.so"
b_file_spec.GetPath()
  Which is: 
"/var/folders/hv/pgk33lbd2snf__0bd0v30yycgn/T/lit-tmp-qrocj9y_/lldb/45872/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModule/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC/AndroidModule.so"
  
/Users/user/Sources/llvm-project/lldb/unittests/Target/LocateModuleCallbackTest.cpp:280
  Value of: module_sp->GetSymbolFileFileSpec()
Actual: true
  Expected: false

The root cause is that the module is cached in this specific test, 
`GetOrCreateModuleWithCachedModule`.
Because `llvm-lit` runs multiple tests in a same process by this CMake config.
Most runtime data is surely initialized by gtest `SetUp` but `ModuleList` 
global instance lives beyond the test lifespan.
Fixed it by tear down the cached module.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-15 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack updated this revision to Diff 540648.
splhack added a comment.

Tear down the module cache.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

Files:
  lldb/unittests/Target/LocateModuleCallbackTest.cpp

Index: lldb/unittests/Target/LocateModuleCallbackTest.cpp
===
--- lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -233,6 +233,11 @@
 m_module_spec = GetTestModuleSpec();
   }
 
+  void TearDown() override {
+if (m_module_sp)
+  ModuleList::RemoveSharedModule(m_module_sp);
+  }
+
   void CheckNoCallback() {
 EXPECT_FALSE(m_platform_sp->GetLocateModuleCallback());
 EXPECT_EQ(m_callback_call_count, 0);
@@ -257,51 +262,39 @@
   TargetSP m_target_sp;
   ProcessSP m_process_sp;
   ModuleSpec m_module_spec;
+  ModuleSP m_module_sp;
   int m_callback_call_count = 0;
 };
 
 } // namespace
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModule) {
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   // The module file is cached, and the locate module callback is not set.
   // GetOrCreateModule should succeed to return the module from the cache.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
-  CheckStrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModuleAndSymbol) {
   // The module and symbol files are cached, and the locate module callback is
   // not set. GetOrCreateModule should succeed to return the module from the
   // cache with the symbol.
-
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   FileSpec uuid_view = BuildCacheDirWithSymbol(m_test_dir);
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
-  CheckUnstrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckUnstrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest,
@@ -313,12 +306,11 @@
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
-  CheckUnstrippedSymbol(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckUnstrippedSymbol(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleFailure) {
@@ -329,9 +321,8 @@
 
   CheckNoCallback();
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureNoCache) {
@@ -347,9 +338,8 @@
 return Status("The locate module callback failed");
   });
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  ASSERT_FALSE(module_sp);
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_FALSE(m_module_sp);
 }
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureCached) {
@@ -365,12 +355,11 @@
 return Status("The locate module callback failed");
   });
 
-  ModuleSP module_sp =
-  m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
-  CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

FWIW when I was testing this locally, I was seeing paths like

  (lldb) p uuid_view
  (lldb_private::FileSpec) $0 = file: "AndroidModule.so" dir: 
"/var/folders/1b/976z5_5513l64wvrj15vkt80gn/T/lldb/49966/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModuleAndSymbol/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"
  
  (lldb) p module_sp->GetFileSpec()
  (const lldb_private::FileSpec) $1 = file: "AndroidModule.so" dir: 
"/var/folders/1b/976z5_5513l64wvrj15vkt80gn/T/lldb/49966/LocateModuleCallbackTest-GetOrCreateModuleWithCachedModuleAndSymbol/remote-android/.cache/80008338-82A0-51E5-5922-C905D23890DA-BDDEFECC"

and I'm betting the real issue is that the arm64 bot account has something 
funky about TMPRDIR, so one of these is realpath'ed to a different filepath and 
the comparison failed.  It would be a lot easier to understand what was 
actually happening on that bot if we could see the paths, but I don't think 
we're addressing the actual issue here.  (we need to fix the printing first, 
then we can see what is going on with the bot)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

Like reading third-party/unittest/googletest/include/gtest/gtest-printers.h, I 
thought I could add a static PrintTo method?  but

  index 6eb5b805d9d..93d98baa5be 100644
  --- a/lldb/include/lldb/Utility/FileSpec.h
  +++ b/lldb/include/lldb/Utility/FileSpec.h
  @@ -12,4 +12,5 @@
   #include 
   #include 
  +#include 
   #include 
   
  @@ -198,4 +199,8 @@ public:
 static

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

My first thought was, but what about all the other unit tests doing this, and 
git grepping around, I think there's one in Interpreter/TestOptionValue.cpp and 
one in Interpreter/TestOptionValueFileColonLine.cpp.   I'd like to see what 
@JDevlieghere thinks, this patch is fine to me but it is an unfortunate - 
surely there is some method we could implement in FileSpec to get the gtest 
stuff to print the path when there's a failure, but I didn't see it when I 
tried to read through the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack updated this revision to Diff 540594.
splhack added a comment.

Use GetPath() instead of Compare().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

Files:
  lldb/unittests/Target/LocateModuleCallbackTest.cpp

Index: lldb/unittests/Target/LocateModuleCallbackTest.cpp
===
--- lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -148,11 +148,15 @@
   return module_spec;
 }
 
+void CheckFileSpecs(const FileSpec _file_spec, const FileSpec _file_spec) {
+  ASSERT_EQ(a_file_spec.GetPath(), b_file_spec.GetPath());
+}
+
 void CheckModule(const ModuleSP _sp) {
   ASSERT_TRUE(module_sp);
   ASSERT_EQ(module_sp->GetUUID().GetAsString(), k_module_uuid);
   ASSERT_EQ(module_sp->GetObjectOffset(), 0U);
-  ASSERT_EQ(module_sp->GetPlatformFileSpec(), GetRemotePath());
+  CheckFileSpecs(module_sp->GetPlatformFileSpec(), GetRemotePath());
 }
 
 SymbolContextList FindFunctions(const ModuleSP _sp,
@@ -263,11 +267,6 @@
 } // namespace
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModule) {
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   // The module file is cached, and the locate module callback is not set.
   // GetOrCreateModule should succeed to return the module from the cache.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
@@ -277,7 +276,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -286,12 +285,6 @@
   // The module and symbol files are cached, and the locate module callback is
   // not set. GetOrCreateModule should succeed to return the module from the
   // cache with the symbol.
-
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   FileSpec uuid_view = BuildCacheDirWithSymbol(m_test_dir);
 
   CheckNoCallback();
@@ -299,8 +292,8 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
   CheckUnstrippedSymbol(module_sp);
 }
 
@@ -316,8 +309,8 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
   CheckUnstrippedSymbol(module_sp);
 }
 
@@ -368,7 +361,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -391,7 +384,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -413,7 +406,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -438,7 +431,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_TRUE(module_sp->GetSymbolFileFileSpec().GetPath().empty());
   CheckStrippedSymbol(module_sp);
 }
@@ -459,8 +452,8 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(),
-FileSpec(GetInputFilePath(k_module_file)));
+

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack added a comment.

thanks @JDevlieghere @jasonmolenda, yeah `GetPath()` makes sense it could show 
the mismatch in human readable form.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added a comment.

I ran the TargetTests under lldb to see (this test doesn't fail on my arm64 mac 
desktop ftr).   ASSERT_EQ calls `template  
AssertionResult CmpHelperEQ()` and that calls `FileSpec::operator==` on my 
desktop.  Jonas suggested that we get the binary data dump of the FileSpec 
objects because there wasn't a method implemented in FileSpec that gtest knew 
to call to print the objects.  I originally thought ASSERT_EQ might be doing a 
binary comparison of the object bytes when I saw the failing msg on the arm64 
macos bot.

Would it be better to pass the `GetPath()` method on both of the FileSpec's we 
send to ASSERT_EQ so it is working with std::string's which it will know how to 
print if there is a comparison mismatch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere requested changes to this revision.
JDevlieghere added a comment.
This revision now requires changes to proceed.

`ASSERT_EQ` should call `operator==` under the hood. Why would that be flakey 
on arm64? Can we achieve this by changing the implementation to reimplement 
this with `FileSpec::Compare`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155333

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

[Lldb-commits] [PATCH] D155333: [lldb][LocateModuleCallback] Fix FileSpec compare

2023-07-14 Thread Kazuki Sakamoto via Phabricator via lldb-commits 
splhack created this revision.
splhack added reviewers: clayborg, jingham, bulbazord, jasonmolenda, 
JDevlieghere, rastogishubham.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
splhack requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

ASSERT_EQ FileSpec is flaky on arm64. Use FileSpec::Compare instead.

- 
https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/2260/testReport/junit/lldb-unit/Target___TargetTests_LocateModuleCallbackTest/GetOrCreateModuleWithCachedModule/
- 
https://green.lab.llvm.org/green/view/LLDB/job/as-lldb-cmake/2260/testReport/junit/lldb-unit/Target___TargetTests_LocateModuleCallbackTest/GetOrCreateModuleWithCachedModuleAndSymbol/


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155333

Files:
  lldb/unittests/Target/LocateModuleCallbackTest.cpp

Index: lldb/unittests/Target/LocateModuleCallbackTest.cpp
===
--- lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -148,11 +148,15 @@
   return module_spec;
 }
 
+void CheckFileSpecs(const FileSpec _file_spec, const FileSpec _file_spec) {
+  ASSERT_EQ(FileSpec::Compare(a_file_spec, b_file_spec, /*full=*/true), 0);
+}
+
 void CheckModule(const ModuleSP _sp) {
   ASSERT_TRUE(module_sp);
   ASSERT_EQ(module_sp->GetUUID().GetAsString(), k_module_uuid);
   ASSERT_EQ(module_sp->GetObjectOffset(), 0U);
-  ASSERT_EQ(module_sp->GetPlatformFileSpec(), GetRemotePath());
+  CheckFileSpecs(module_sp->GetPlatformFileSpec(), GetRemotePath());
 }
 
 SymbolContextList FindFunctions(const ModuleSP _sp,
@@ -263,11 +267,6 @@
 } // namespace
 
 TEST_F(LocateModuleCallbackTest, GetOrCreateModuleWithCachedModule) {
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   // The module file is cached, and the locate module callback is not set.
   // GetOrCreateModule should succeed to return the module from the cache.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
@@ -277,7 +276,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -286,12 +285,6 @@
   // The module and symbol files are cached, and the locate module callback is
   // not set. GetOrCreateModule should succeed to return the module from the
   // cache with the symbol.
-
-  // Disable test on arm64 because of failures in the lldb incremental arm64
-  // bot.
-#if defined(__arm64__) || defined(__aarch64__) || defined(_M_ARM64)
-  GTEST_SKIP() << "broken on arm64.";
-#endif
   FileSpec uuid_view = BuildCacheDirWithSymbol(m_test_dir);
 
   CheckNoCallback();
@@ -299,8 +292,8 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
   CheckUnstrippedSymbol(module_sp);
 }
 
@@ -316,8 +309,8 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
-  ASSERT_EQ(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetSymbolFileFileSpec(), GetSymFileSpec(uuid_view));
   CheckUnstrippedSymbol(module_sp);
 }
 
@@ -368,7 +361,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -391,7 +384,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -413,7 +406,7 @@
   ModuleSP module_sp =
   m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
   CheckModule(module_sp);
-  ASSERT_EQ(module_sp->GetFileSpec(), uuid_view);
+  CheckFileSpecs(module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(module_sp->GetSymbolFileFileSpec());
   CheckStrippedSymbol(module_sp);
 }
@@ -438,7