[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 added a comment.

ping


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

https://reviews.llvm.org/D126464

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

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 added a comment.

In D126464#3628819 , @thakis wrote:

> What `--author` flag should I use for you when landing this?

Hi! Could you please use --author="Kaining Zhong 
"? Thanks!


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

https://reviews.llvm.org/D126464

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

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 created this revision.
PRESIDENT810 added reviewers: clayborg, jingham, jasonmolenda.
Herald added a project: All.
PRESIDENT810 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This fixes https://github.com/llvm/llvm-project/issues/50114 where lldb/mac 
can't load object files from thin archives.
This patch allows lldb to identify thin archives, and load object files 
contained in them.

I'm a novice at llvm not sure whether I'm fixing this right, so please tell me 
if anything wrong with this fix, thank you guys!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126464

Files:
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h

Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
@@ -15,12 +15,16 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 
+#include "llvm/Object/Archive.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Path.h"
 
 #include 
 #include 
 #include 
 
+enum class ArchiveType { Invalid, Archive, ThinArchive };
+
 class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
 public:
   ObjectContainerBSDArchive(const lldb::ModuleSP &module_sp,
@@ -54,7 +58,7 @@
 lldb::offset_t length,
 lldb_private::ModuleSpecList &specs);
 
-  static bool MagicBytesMatch(const lldb_private::DataExtractor &data);
+  static ArchiveType MagicBytesMatch(const lldb_private::DataExtractor &data);
 
   // Member Functions
   bool ParseHeader() override;
@@ -78,6 +82,10 @@
 
 void Clear();
 
+lldb::offset_t ExtractFromThin(const lldb_private::DataExtractor &data,
+   lldb::offset_t offset,
+   llvm::StringRef stringTable);
+
 lldb::offset_t Extract(const lldb_private::DataExtractor &data,
lldb::offset_t offset);
 /// Object name in the archive.
@@ -156,6 +164,8 @@
 
 lldb_private::DataExtractor &GetData() { return m_data; }
 
+ArchiveType GetArchiveType() { return m_archive_type; }
+
   protected:
 typedef lldb_private::UniqueCStringMap ObjectNameToIndexMap;
 // Member Variables
@@ -167,11 +177,14 @@
 lldb_private::DataExtractor m_data; ///< The data for this object container
 ///so we don't lose data if the .a files
 ///gets modified
+ArchiveType m_archive_type;
   };
 
   void SetArchive(Archive::shared_ptr &archive_sp);
 
   Archive::shared_ptr m_archive_sp;
+
+  ArchiveType m_archive_type;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_OBJECTCONTAINER_BSD_ARCHIVE_OBJECTCONTAINERBSDARCHIVE_H
Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -40,6 +40,8 @@
 using namespace lldb;
 using namespace lldb_private;
 
+using namespace llvm::object;
+
 LLDB_PLUGIN_DEFINE(ObjectContainerBSDArchive)
 
 ObjectContainerBSDArchive::Object::Object() : ar_name() {}
@@ -55,6 +57,74 @@
   file_size = 0;
 }
 
+lldb::offset_t ObjectContainerBSDArchive::Object::ExtractFromThin(
+const DataExtractor &data, lldb::offset_t offset,
+llvm::StringRef stringTable) {
+  size_t ar_name_len = 0;
+  std::string str;
+  char *err;
+
+  // File header
+  //
+  // The common format is as follows.
+  //
+  //  Offset  Length	NameFormat
+  //  0   16  File name   ASCII right padded with spaces (no spaces
+  //  allowed in file name)
+  //  16  12  File modDecimal as cstring right padded with
+  //  spaces
+  //  28  6   Owner IDDecimal as cstring right padded with
+  //  spaces
+  //  34  6   Group IDDecimal as cstring right padded with
+  //  spaces
+  //  40  8   File mode   Octal   as cstring right padded with
+  //  spaces
+  //  48  10  File byte size  Decimal as cstring right padded with
+  //  spaces
+  //  58  2   File magic  0x60 0x0A
+
+  // Make sure there is enough data for the file header and bail if not
+  if (!data.ValidOffsetForDataOfSize(offset, 60))
+return LLDB_INVALID_OFFSET;
+
+  str.assign((const char *)data.GetData(&offset, 16), 16);
+  if (!(llvm::StringRef(str).startswith("//") || stringTable.empty())) {
+// Strip off any trailing spaces.
+const size_t last_pos =

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 updated this revision to Diff 432857.
PRESIDENT810 added a comment.

I have refactored my code so it should looks cleaner now, but I'm not sure how 
to add a test. It seems that adding a test for thin archive on macOS platforms 
can be not so straightforward. I see that in 
lldb/test/API/functionalities/archives/Makefile, the test suite is using 
libtool instead of ar to create archives on macOS platforms, and I don't know 
whether I can produce a thin archive with that. Also, ld on macOS platforms 
seems to be unable to identify thin archives (I'm using ld64.lld when testing 
my code locally).

I would really appreciate it if someone can provide some advice about how to 
implement such a test case here, thank you!


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

https://reviews.llvm.org/D126464

Files:
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h

Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
@@ -15,19 +15,24 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 
+#include "llvm/Object/Archive.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Path.h"
 
 #include 
 #include 
 #include 
 
+enum class ArchiveType { Invalid, Archive, ThinArchive };
+
 class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
 public:
   ObjectContainerBSDArchive(const lldb::ModuleSP &module_sp,
 lldb::DataBufferSP &data_sp,
 lldb::offset_t data_offset,
 const lldb_private::FileSpec *file,
-lldb::offset_t offset, lldb::offset_t length);
+lldb::offset_t offset, lldb::offset_t length,
+ArchiveType archive_type);
 
   ~ObjectContainerBSDArchive() override;
 
@@ -54,7 +59,7 @@
 lldb::offset_t length,
 lldb_private::ModuleSpecList &specs);
 
-  static bool MagicBytesMatch(const lldb_private::DataExtractor &data);
+  static ArchiveType MagicBytesMatch(const lldb_private::DataExtractor &data);
 
   // Member Functions
   bool ParseHeader() override;
@@ -78,6 +83,10 @@
 
 void Clear();
 
+lldb::offset_t ExtractFromThin(const lldb_private::DataExtractor &data,
+   lldb::offset_t offset,
+   llvm::StringRef stringTable);
+
 lldb::offset_t Extract(const lldb_private::DataExtractor &data,
lldb::offset_t offset);
 /// Object name in the archive.
@@ -156,6 +165,8 @@
 
 lldb_private::DataExtractor &GetData() { return m_data; }
 
+ArchiveType GetArchiveType() { return m_archive_type; }
+
   protected:
 typedef lldb_private::UniqueCStringMap ObjectNameToIndexMap;
 // Member Variables
@@ -167,11 +178,14 @@
 lldb_private::DataExtractor m_data; ///< The data for this object container
 ///so we don't lose data if the .a files
 ///gets modified
+ArchiveType m_archive_type;
   };
 
   void SetArchive(Archive::shared_ptr &archive_sp);
 
   Archive::shared_ptr m_archive_sp;
+
+  ArchiveType m_archive_type;
 };
 
 #endif // LLDB_SOURCE_PLUGINS_OBJECTCONTAINER_BSD_ARCHIVE_OBJECTCONTAINERBSDARCHIVE_H
Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
@@ -40,6 +40,8 @@
 using namespace lldb;
 using namespace lldb_private;
 
+using namespace llvm::object;
+
 LLDB_PLUGIN_DEFINE(ObjectContainerBSDArchive)
 
 ObjectContainerBSDArchive::Object::Object() : ar_name() {}
@@ -55,6 +57,74 @@
   file_size = 0;
 }
 
+lldb::offset_t ObjectContainerBSDArchive::Object::ExtractFromThin(
+const DataExtractor &data, lldb::offset_t offset,
+llvm::StringRef stringTable) {
+  size_t ar_name_len = 0;
+  std::string str;
+  char *err;
+
+  // File header
+  //
+  // The common format is as follows.
+  //
+  //  Offset  Length	NameFormat
+  //  0   16  File name   ASCII right padded with spaces (no spaces
+  //  allowed in file name)
+  //  16  12  File modDecimal as cstring right padded with
+  //  spaces
+  //  28  6   Owner IDDecimal as cstring right padded with
+  //  spaces
+  //  34  6   Group IDDecimal as cstring right padded wi

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 updated this revision to Diff 434391.
PRESIDENT810 added a comment.
Herald added a subscriber: mgorny.

Add tests by introducing llvm-ar to produce thin archives


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

https://reviews.llvm.org/D126464

Files:
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
  lldb/test/API/functionalities/archives/Makefile
  lldb/test/API/functionalities/archives/TestBSDArchives.py
  lldb/test/API/functionalities/archives/c.c
  lldb/test/API/lit.cfg.py
  lldb/test/CMakeLists.txt

Index: lldb/test/CMakeLists.txt
===
--- lldb/test/CMakeLists.txt
+++ lldb/test/CMakeLists.txt
@@ -148,6 +148,7 @@
   llvm-objcopy
   llvm-pdbutil
   llvm-readobj
+  llvm-ar
   )
 
 if(TARGET lld)
Index: lldb/test/API/lit.cfg.py
===
--- lldb/test/API/lit.cfg.py
+++ lldb/test/API/lit.cfg.py
@@ -162,6 +162,10 @@
 if is_configured('llvm_libs_dir'):
   dotest_cmd += ['--env', 'LLVM_LIBS_DIR=' + config.llvm_libs_dir]
 
+# This path may be needed to locate required llvm tools
+if is_configured('llvm_tools_dir'):
+  dotest_cmd += ['--env', 'LLVM_TOOLS_DIR=' + config.llvm_tools_dir]
+
 # Forward ASan-specific environment variables to tests, as a test may load an
 # ASan-ified dylib.
 for env_var in ('ASAN_OPTIONS', 'DYLD_INSERT_LIBRARIES'):
Index: lldb/test/API/functionalities/archives/c.c
===
--- /dev/null
+++ lldb/test/API/functionalities/archives/c.c
@@ -0,0 +1,11 @@
+static int __c_global = 3;
+
+int c(int arg) {
+  int result = arg + __c_global;
+  return result;
+}
+
+int cc(int arg1) {
+  int result3 = arg1 - __c_global;
+  return result3;
+}
Index: lldb/test/API/functionalities/archives/TestBSDArchives.py
===
--- lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -60,3 +60,10 @@
 self.expect("frame variable", VARIABLES_DISPLAYED_CORRECTLY,
 substrs=['(int) arg = 2'])
 self.expect_var_path("__b_global", type="int", value="2")
+
+# Test loading thin archives
+archive_path = self.getBuildArtifact("libbar.a")
+module_specs = lldb.SBModuleSpecList.GetModuleSpecifications(archive_path)
+num_specs = module_specs.GetSize()
+self.assertEqual(num_specs, 1)
+self.assertEqual(module_specs.GetSpecAtIndex(0).GetObjectName(), "c.o")
Index: lldb/test/API/functionalities/archives/Makefile
===
--- lldb/test/API/functionalities/archives/Makefile
+++ lldb/test/API/functionalities/archives/Makefile
@@ -1,8 +1,8 @@
-C_SOURCES := main.c a.c b.c
+C_SOURCES := main.c a.c b.c c.c
 EXE :=  # Define a.out explicitly
 MAKE_DSYM := NO
 
-all: a.out
+all: a.out libbar.a
 
 a.out: main.o libfoo.a
 	$(LD) $(LDFLAGS) $^ -o $@
@@ -11,4 +11,11 @@
 	$(AR) $(ARFLAGS) $@ $^
 	$(RM) $^
 
+# This tests whether lldb can load a thin archive
+libbar.a: c.o
+	$(eval LLVM_AR := $(LLVM_TOOLS_DIR)/llvm-ar)
+	$(eval LLVM_ARFLAGS := -rcsDT)
+	$(LLVM_AR) $(LLVM_ARFLAGS) $@ $^
+	# Note for thin archive case, we cannot remove c.o
+
 include Makefile.rules
Index: lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
===
--- lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
+++ lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
@@ -15,19 +15,24 @@
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/FileSpec.h"
 
+#include "llvm/Object/Archive.h"
 #include "llvm/Support/Chrono.h"
+#include "llvm/Support/Path.h"
 
 #include 
 #include 
 #include 
 
+enum class ArchiveType { Invalid, Archive, ThinArchive };
+
 class ObjectContainerBSDArchive : public lldb_private::ObjectContainer {
 public:
   ObjectContainerBSDArchive(const lldb::ModuleSP &module_sp,
 lldb::DataBufferSP &data_sp,
 lldb::offset_t data_offset,
 const lldb_private::FileSpec *file,
-lldb::offset_t offset, lldb::offset_t length);
+lldb::offset_t offset, lldb::offset_t length,
+ArchiveType archive_type);
 
   ~ObjectContainerBSDArchive() override;
 
@@ -54,7 +59,7 @@
 lldb::offset_t length,
 lldb_private::ModuleSpecList &specs);
 
-  static bool MagicBytesMatch(const lldb_private::DataExtractor &data);
+  static ArchiveType MagicBytesMatch(const lldb_private::DataExtractor &data);
 
   // Member Functions
 

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 added a comment.
Herald added a subscriber: Michael137.

ping


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

https://reviews.llvm.org/D126464

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

[Lldb-commits] [PATCH] D126464: [lldb] Add support to load object files from thin archives

[Kaining Zhong via Phabricator via lldb-commits]

PRESIDENT810 added a comment.

Hi, sorry to interrupt but I just forgot to mention that I didn't have access 
to commit. Anyone can please help me land this one if it looks ok?


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

https://reviews.llvm.org/D126464

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