[Lldb-commits] [lldb] r230787 - Fix FileSpec::GetPath to return null-terminated strings

Ilia K ki.stfu at gmail.com
Fri Feb 27 11:43:08 PST 2015


Author: ki.stfu
Date: Fri Feb 27 13:43:08 2015
New Revision: 230787

URL: http://llvm.org/viewvc/llvm-project?rev=230787&view=rev
Log:
Fix FileSpec::GetPath to return null-terminated strings

Summary:
Before this fix the FileSpec::GetPath() returned string which might be without '\0' at the end.
It could have happened if the size of buffer for path was less than actual path.

Test case:
```
FileSpec test("/path/to/file", false);
char buf[]="!!!!!!";
test.GetPath(buf, 3);
```

Before fix:
```
   233          FileSpec test("/path/to/file", false);
   234          char buf[]="!!!!!!";
   235          test.GetPath(buf, 3);
   236
-> 237          if (core_file)
   238          {
   239              if (!core_file.Exists())
   240              {
(lldb) print buf
(char [7]) $0 = "/pa!!!"
```

After fix:
```
   233          FileSpec test("/path/to/file", false);
   234          char buf[]="!!!!!!";
   235          test.GetPath(buf, 3);
   236
-> 237          if (core_file)
   238          {
   239              if (!core_file.Exists())
   240              {
(lldb) print buf
(char [7]) $0 = "/p"
```

Reviewers: zturner, abidh, clayborg

Reviewed By: abidh, clayborg

Subscribers: tberghammer, vharron, lldb-commits, clayborg, zturner, abidh

Differential Revision: http://reviews.llvm.org/D7553

Modified:
    lldb/trunk/source/API/SBFileSpec.cpp
    lldb/trunk/source/Host/common/FileSpec.cpp
    lldb/trunk/source/Host/common/SocketAddress.cpp
    lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
    lldb/trunk/source/Host/posix/HostInfoPosix.cpp
    lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
    lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
    lldb/trunk/source/lldb.cpp

Modified: lldb/trunk/source/API/SBFileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBFileSpec.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/API/SBFileSpec.cpp (original)
+++ lldb/trunk/source/API/SBFileSpec.cpp Fri Feb 27 13:43:08 2015
@@ -93,9 +93,8 @@ SBFileSpec::ResolvePath (const char *src
 {
     llvm::SmallString<64> result(src_path);
     lldb_private::FileSpec::Resolve (result);
-    size_t result_length = std::min(dst_len-1, result.size());
-    ::strncpy(dst_path, result.c_str(), result_length + 1);
-    return result_length;
+    ::snprintf(dst_path, dst_len, "%s", result.c_str());
+    return std::min(dst_len-1, result.size());
 }
 
 const char *

Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Fri Feb 27 13:43:08 2015
@@ -793,10 +793,8 @@ FileSpec::GetPath(char *path, size_t pat
         return 0;
 
     std::string result = GetPath(denormalize);
-
-    size_t result_length = std::min(path_max_len-1, result.length());
-    ::strncpy(path, result.c_str(), result_length + 1);
-    return result_length;
+    ::snprintf(path, path_max_len, "%s", result.c_str());
+    return std::min(path_max_len-1, result.length());
 }
 
 std::string

Modified: lldb/trunk/source/Host/common/SocketAddress.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/SocketAddress.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/SocketAddress.cpp (original)
+++ lldb/trunk/source/Host/common/SocketAddress.cpp Fri Feb 27 13:43:08 2015
@@ -48,8 +48,7 @@ const char* inet_ntop(int af, const void
                 const char* formatted = inet_ntoa(*static_cast<const in_addr*>(src));
                 if (formatted && strlen(formatted) < size)
                 {
-                    strncpy(dst, formatted, size);
-                    return dst;
+                    return ::strcpy(dst, formatted);
                 }
             }
             return nullptr;
@@ -64,8 +63,7 @@ const char* inet_ntop(int af, const void
                                           );
                 if (full_size < static_cast<int>(size))
                 {
-                    strncpy(dst,tmp,size);
-                    return dst;
+                    return ::strcpy(dst, tmp);
                 }
                 return nullptr;
             }

Modified: lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm (original)
+++ lldb/trunk/source/Host/macosx/HostInfoMacOSX.mm Fri Feb 27 13:43:08 2015
@@ -134,22 +134,23 @@ HostInfoMacOSX::ComputeSupportExeDirecto
     FileSpec lldb_file_spec;
     if (!GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
         return false;
-    char raw_path[PATH_MAX];
-    lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
 
-    char *framework_pos = ::strstr(raw_path, "LLDB.framework");
-    if (framework_pos)
+    std::string raw_path = lldb_file_spec.GetPath();
+
+    size_t framework_pos = raw_path.find("LLDB.framework");
+    if (framework_pos != std::string::npos)
     {
         framework_pos += strlen("LLDB.framework");
 #if defined(__arm__) || defined(__arm64__) || defined(__aarch64__)
         // Shallow bundle
-        *framework_pos = '\0';
+        raw_path.resize(framework_pos);
 #else
         // Normal bundle
-        ::strncpy(framework_pos, "/Resources", PATH_MAX - (framework_pos - raw_path));
+        raw_path.resize(framework_pos);
+        raw_path.append("/Resources");
 #endif
     }
-    file_spec.GetDirectory().SetCString(raw_path);
+    file_spec.GetDirectory().SetString(llvm::StringRef(raw_path.c_str(), raw_path.size()));
     return (bool)file_spec.GetDirectory();
 }
 
@@ -160,16 +161,16 @@ HostInfoMacOSX::ComputeHeaderDirectory(F
     if (!HostInfo::GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
         return false;
 
-    char raw_path[PATH_MAX];
-    lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
+    std::string raw_path = lldb_file_spec.GetPath();
 
-    char *framework_pos = ::strstr(raw_path, "LLDB.framework");
-    if (framework_pos)
+    size_t framework_pos = raw_path.find("LLDB.framework");
+    if (framework_pos != std::string::npos)
     {
         framework_pos += strlen("LLDB.framework");
-        ::strncpy(framework_pos, "/Headers", PATH_MAX - (framework_pos - raw_path));
+        raw_path.resize(framework_pos);
+        raw_path.append("/Headers");
     }
-    file_spec.GetDirectory().SetCString(raw_path);
+    file_spec.GetDirectory().SetString(llvm::StringRef(raw_path.c_str(), raw_path.size()));
     return true;
 }
 
@@ -181,14 +182,14 @@ HostInfoMacOSX::ComputePythonDirectory(F
     if (!GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
         return false;
 
-    char raw_path[PATH_MAX];
-    lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
+    std::string raw_path = lldb_file_spec.GetPath();
 
-    char *framework_pos = ::strstr(raw_path, "LLDB.framework");
-    if (framework_pos)
+    size_t framework_pos = raw_path.find("LLDB.framework");
+    if (framework_pos != std::string::npos)
     {
         framework_pos += strlen("LLDB.framework");
-        ::strncpy(framework_pos, "/Resources/Python", PATH_MAX - (framework_pos - raw_path));
+        raw_path.resize(framework_pos);
+        raw_path.append("/Resources/Python");
     }
     else
     {
@@ -198,9 +199,9 @@ HostInfoMacOSX::ComputePythonDirectory(F
         os.flush();
 
         // We may get our string truncated. Should we protect this with an assert?
-        ::strncat(raw_path, python_version_dir.c_str(), sizeof(raw_path) - strlen(raw_path) - 1);
+        raw_path.append(python_version_dir.c_str());
     }
-    file_spec.GetDirectory().SetCString(raw_path);
+    file_spec.GetDirectory().SetString(llvm::StringRef(raw_path.c_str(), raw_path.size()));
     return true;
 #else
     return false;
@@ -214,16 +215,16 @@ HostInfoMacOSX::ComputeClangDirectory(Fi
     if (!GetLLDBPath (lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
         return false;
     
-    char raw_path[PATH_MAX];
-    lldb_file_spec.GetPath (raw_path, sizeof (raw_path));
+    std::string raw_path = lldb_file_spec.GetPath();
     
-    char *framework_pos = ::strstr (raw_path, "LLDB.framework");
-    if (framework_pos)
+    size_t framework_pos = raw_path.find("LLDB.framework");
+    if (framework_pos != std::string::npos)
     {
         framework_pos += strlen("LLDB.framework");
-        ::strncpy (framework_pos, "/Resources/Clang", PATH_MAX - (framework_pos - raw_path));
+        raw_path.resize(framework_pos);
+        raw_path.append("/Resources/Clang");
     }
-    file_spec.SetFile (raw_path, true);
+    file_spec.SetFile (raw_path.c_str(), true);
     return true;
 }
 
@@ -233,16 +234,17 @@ HostInfoMacOSX::ComputeSystemPluginsDire
     FileSpec lldb_file_spec;
     if (!GetLLDBPath(lldb::ePathTypeLLDBShlibDir, lldb_file_spec))
         return false;
-    char raw_path[PATH_MAX];
-    lldb_file_spec.GetPath(raw_path, sizeof(raw_path));
 
-    char *framework_pos = ::strstr(raw_path, "LLDB.framework");
-    if (!framework_pos)
+    std::string raw_path = lldb_file_spec.GetPath();
+
+    size_t framework_pos = raw_path.find("LLDB.framework");
+    if (framework_pos == std::string::npos)
         return false;
 
     framework_pos += strlen("LLDB.framework");
-    ::strncpy(framework_pos, "/Resources/PlugIns", PATH_MAX - (framework_pos - raw_path));
-    file_spec.GetDirectory().SetCString(raw_path);
+    raw_path.resize(framework_pos);
+    raw_path.append("/Resources/PlugIns");
+    file_spec.GetDirectory().SetString(llvm::StringRef(raw_path.c_str(), raw_path.size()));
     return true;
 }
 

Modified: lldb/trunk/source/Host/posix/HostInfoPosix.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/HostInfoPosix.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/HostInfoPosix.cpp (original)
+++ lldb/trunk/source/Host/posix/HostInfoPosix.cpp Fri Feb 27 13:43:08 2015
@@ -153,11 +153,8 @@ HostInfoPosix::ComputeSupportExeDirector
     char *lib_pos = ::strstr(raw_path, "/lib");
     if (lib_pos != nullptr)
     {
-        // First terminate the raw path at the start of lib.
-        *lib_pos = '\0';
-
         // Now write in bin in place of lib.
-        ::strncpy(lib_pos, "/bin", PATH_MAX - (lib_pos - raw_path));
+        ::snprintf(lib_pos, PATH_MAX - (lib_pos - raw_path), "/bin");
 
         if (log)
             log->Printf("Host::%s() derived the bin path as: %s", __FUNCTION__, raw_path);

Modified: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (original)
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp Fri Feb 27 13:43:08 2015
@@ -3639,7 +3639,7 @@ GDBRemoteCommunicationClient::SaveRegist
             if (thread_suffix_supported)
                 ::snprintf (packet, sizeof(packet), "QSaveRegisterState;thread:%4.4" PRIx64 ";", tid);
             else
-                ::strncpy (packet, "QSaveRegisterState", sizeof(packet));
+                ::snprintf(packet, sizeof(packet), "QSaveRegisterState");
             
             StringExtractorGDBRemote response;
 

Modified: lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp Fri Feb 27 13:43:08 2015
@@ -231,7 +231,7 @@ SymbolVendorMacOSX::CreateInstance (cons
                                                                                 const char *node_content = (const char *)::xmlNodeGetContent(value_node);
                                                                                 if (node_content)
                                                                                 {
-                                                                                    strncpy(DBGBuildSourcePath, node_content, sizeof(DBGBuildSourcePath));
+                                                                                    ::snprintf(DBGBuildSourcePath, sizeof(DBGBuildSourcePath), "%s", node_content);
                                                                                     xmlFree((void *) node_content);
                                                                                 }
                                                                             }

Modified: lldb/trunk/source/lldb.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/lldb.cpp?rev=230787&r1=230786&r2=230787&view=diff
==============================================================================
--- lldb/trunk/source/lldb.cpp (original)
+++ lldb/trunk/source/lldb.cpp Fri Feb 27 13:43:08 2015
@@ -364,13 +364,13 @@ lldb_private::GetVersion ()
         
         const char *newline_loc = strchr(version_string, '\n');
         
-        size_t version_len = sizeof(g_version_string);
+        size_t version_len = sizeof(g_version_string) - 1;
         
         if (newline_loc &&
             (newline_loc - version_string < static_cast<ptrdiff_t>(version_len)))
             version_len = newline_loc - version_string;
         
-        ::strncpy(g_version_string, version_string, version_len);
+        ::snprintf(g_version_string, version_len + 1, "%s", version_string);
     }
 
     return g_version_string;





More information about the lldb-commits mailing list