[Lldb-commits] [PATCH] D61233: Refactor ObjectFile::GetSDKVersion

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: aprantl, jasonmolenda.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This patch modernizes the GetSDKVersion API and hopefully prevents problems such as the ones discovered in D61218 <https://reviews.llvm.org/D61218>.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D61233

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp


Index: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp
@@ -163,8 +163,8 @@
       std::string xcode_contents_path;
       std::string default_xcode_sdk;
       FileSpec fspec;
-      uint32_t versions[2];
-      if (objfile->GetSDKVersion(versions, sizeof(versions))) {
+      llvm::Optional<std::vector<uint32_t>> versions = objfile->GetSDKVersion();
+      if (versions && versions->size() >= 2) {
         fspec = HostInfo::GetShlibDir();
         if (fspec) {
           std::string path;
@@ -208,8 +208,8 @@
           StreamString sdk_path;
           sdk_path.Printf("%sDeveloper/Platforms/MacOSX.platform/Developer/"
                           "SDKs/MacOSX%u.%u.sdk",
-                          xcode_contents_path.c_str(), versions[0],
-                          versions[1]);
+                          xcode_contents_path.c_str(), versions->at(0),
+                          versions->at(1));
           fspec.SetFile(sdk_path.GetString(), FileSpec::Style::native);
           if (FileSystem::Instance().Exists(fspec))
             return ConstString(sdk_path.GetString());
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
@@ -116,7 +116,7 @@
 
   llvm::VersionTuple GetMinimumOSVersion() override;
 
-  uint32_t GetSDKVersion(uint32_t *versions, uint32_t num_versions) override;
+  llvm::Optional<std::vector<uint32_t>> GetSDKVersion() override;
 
   bool GetIsDynamicLinkEditor() override;
 
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5846,8 +5846,7 @@
   return *m_min_os_version;
 }
 
-uint32_t ObjectFileMachO::GetSDKVersion(uint32_t *versions,
-                                        uint32_t num_versions) {
+llvm::Optional<std::vector<uint32_t>> ObjectFileMachO::GetSDKVersion() {
   if (m_sdk_versions.empty()) {
     lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
     bool success = false;
@@ -5931,19 +5930,10 @@
   // on to m_sdk_versions.  If we only have one value, it's
   // the sentinel value indicating that this object file
   // does not have a valid minimum os version #.
-  if (m_sdk_versions.size() > 1) {
-    if (versions != NULL && num_versions > 0) {
-      for (size_t i = 0; i < num_versions; ++i) {
-        if (i < m_sdk_versions.size())
-          versions[i] = m_sdk_versions[i];
-        else
-          versions[i] = 0;
-      }
-    }
-    return m_sdk_versions.size();
-  }
+  if (m_sdk_versions.size() > 1)
+    return m_sdk_versions;
   // Call the superclasses version that will empty out the data
-  return ObjectFile::GetSDKVersion(versions, num_versions);
+  return ObjectFile::GetSDKVersion();
 }
 
 bool ObjectFileMachO::GetIsDynamicLinkEditor() {
Index: lldb/include/lldb/Symbol/ObjectFile.h
===================================================================
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -577,15 +577,8 @@
   }
 
   /// Get the SDK OS version this object file was built with.
-  ///
-  /// The versions arguments and returns values are the same as the
-  /// GetMinimumOSVersion()
-  virtual uint32_t GetSDKVersion(uint32_t *versions, uint32_t num_versions) {
-    if (versions && num_versions) {
-      for (uint32_t i = 0; i < num_versions; ++i)
-        versions[i] = UINT32_MAX;
-    }
-    return 0;
+  virtual llvm::Optional<std::vector<uint32_t>> GetSDKVersion() {
+    return llvm::None;
   }
 
   /// Return true if this file is a dynamic link editor (dyld)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61233.196986.patch
Type: text/x-patch
Size: 4016 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190427/58c6470a/attachment.bin>