Hi vharron, abidh,

This patch fixes a memory allocating inside signal handler.

This bug was found by @vharron:
> Hi all,
> 
> I noticed these thread sanitizer warnings while running lldb-mi tests on
> Linux.
> 
> WARNING: ThreadSanitizer: signal-unsafe call inside of a signal (pid=39721)
> 
>     #0 operator new(unsigned long) <null>:0 (lldb-mi-3.7.0+0x000000092b9d)
> 
>     #1 std::string::_Rep::_S_create(unsigned long, unsigned long,
> std::allocator<char> const&) <null>:0 (libstdc++.so.6+0x0000000ba3b8)
> 
>     #2 CMICmnResources::GetStringFromResource(unsigned int, CMIUtilString&)
> const
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MICmnResources.cpp:434
> (lldb-mi-3.7.0+0x00000014ddd8)
> 
>     #3 CMICmnResources::GetString(unsigned int) const
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MICmnResources.cpp:371
> (lldb-mi-3.7.0+0x00000014db81)
> 
>     #4 sigwinch_handler(int)
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriverMain.cpp:99
> (lldb-mi-3.7.0+0x0000001589ff)
> 
>     #5 __tsan::CallUserSignalHandler(__tsan::ThreadState*, bool, bool, int,
> my_siginfo_t*, void*) tsan_interceptors.o:0 (lldb-mi-3.7.0+0x00000009f25f)
> 
>     #6 void std::this_thread::sleep_for<long, std::ratio<1l, 1000l>
> >(std::chrono::duration<long, std::ratio<1l, 1000l> > const&)
> /usr/bin/../lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/thread:279
> (lldb-mi-3.7.0+0x00000013243e)
> 
>     #7 CMIDriver::ReadStdinLineQueue()
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriver.cpp:710
> (lldb-mi-3.7.0+0x000000155e62)
> 
>     #8 CMIDriver::DoMainLoop()
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriver.cpp:631
> (lldb-mi-3.7.0+0x000000155d37)
> 
>     #9 non-virtual thunk to CMIDriver::DoMainLoop()
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriver.cpp:648
> (lldb-mi-3.7.0+0x000000155fbd)
> 
>     #10 CMIDriverMgr::DriverMainLoop()
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriverMgr.cpp:340
> (lldb-mi-3.7.0+0x000000159ee6)
> 
>     #11 main
> /usr/local/google/home/vharron/ll/tot/llvm/tools/lldb/tools/lldb-mi/MIDriverMain.cpp:364
> (lldb-mi-3.7.0+0x000000158f60)

http://reviews.llvm.org/D8256

Files:
  tools/lldb-mi/MICmnResources.cpp
  tools/lldb-mi/MICmnResources.h

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
Index: tools/lldb-mi/MICmnResources.cpp
===================================================================
--- tools/lldb-mi/MICmnResources.cpp
+++ tools/lldb-mi/MICmnResources.cpp
@@ -42,7 +42,7 @@
     {IDS_UTIL_FILE_ERR_OPENING_FILE_UNKNOWN, "File Handler. Unknown error opening '%s'"},
     {IDE_UTIL_FILE_ERR_WRITING_FILE, "File Handler. Error %s writing '%s'"},
     {IDE_UTIL_FILE_ERR_WRITING_NOTOPEN, "File Handler. File '%s' not open for write"},
-    {IDS_RESOURCES_ERR_STRING_NOT_FOUND, "Resources. String (%d) not found in resources"},
+    {IDS_RESOURCES_ERR_STRING_NOT_FOUND, "Resources. String not found in resources"},
     {IDS_RESOURCES_ERR_STRING_TABLE_INVALID, "Resources. String resource table is not set up"},
     {IDS_MI_CLIENT_MSG, "Client message: \"%s\""},
     {IDS_LOG_MSG_CREATION_DATE, "Creation date %s time %s%s"},
@@ -368,12 +368,25 @@
 CMIUtilString
 CMICmnResources::GetString(const MIuint vResourceId) const
 {
-    CMIUtilString str;
-    const bool bFound = GetStringFromResource(vResourceId, str);
+    return CMIUtilString(GetCString(vResourceId));
+}
+
+//++ ------------------------------------------------------------------------------------
+// Details: Retrieve the corresponding text assigned to the resource ID.
+// Type:    Method.
+// Args:    vResourceId - (R) MI resource ID.
+// Return:  const MIchar * - Resource text.
+// Throws:  None.
+//--
+const MIchar *
+CMICmnResources::GetCString(const MIuint vResourceId) const
+{
+    const MIchar *pStr;
+    const bool bFound = GetCStringFromResource(vResourceId, pStr);
     MIunused(bFound);
     assert(bFound);
-
-    return str;
+    assert(pStr != nullptr);
+    return pStr;
 }
 
 //++ ------------------------------------------------------------------------------------
@@ -387,8 +400,8 @@
 bool
 CMICmnResources::HasString(const MIuint vResourceId) const
 {
-    CMIUtilString str;
-    return GetStringFromResource(vResourceId, str);
+    const MIchar *pStr;
+    return GetCStringFromResource(vResourceId, pStr);
 }
 
 //++ ------------------------------------------------------------------------------------
@@ -403,7 +416,7 @@
 // Throws:  None.
 //--
 bool
-CMICmnResources::GetStringFromResource(const MIuint vResourceId, CMIUtilString &vrwResourceString) const
+CMICmnResources::GetCStringFromResource(const MIuint vResourceId, const MIchar *&vrwpResourceString) const
 {
     MapRscrIdToTextData_t::const_iterator it = m_mapRscrIdToTextData.find(vResourceId);
     if (it == m_mapRscrIdToTextData.end())
@@ -415,14 +428,14 @@
             it = m_mapRscrIdToTextData.find(vResourceId);
             if (it == m_mapRscrIdToTextData.end())
             {
-                vrwResourceString = MIRSRC(IDS_RESOURCES_ERR_STRING_TABLE_INVALID);
+                vrwpResourceString = MIRSRC(IDS_RESOURCES_ERR_STRING_TABLE_INVALID);
                 return MIstatus::failure;
             }
         }
 
         if (it == m_mapRscrIdToTextData.end())
         {
-            vrwResourceString = CMIUtilString::Format(MIRSRC(IDS_RESOURCES_ERR_STRING_NOT_FOUND), vResourceId);
+            vrwpResourceString = MIRSRC(IDS_RESOURCES_ERR_STRING_NOT_FOUND);
             return MIstatus::failure;
         }
     }
@@ -432,7 +445,7 @@
     const MIchar *pRsrcData((*it).second);
 
     // Return result
-    vrwResourceString = pRsrcData;
+    vrwpResourceString = pRsrcData;
 
     return MIstatus::success;
 }
Index: tools/lldb-mi/MICmnResources.h
===================================================================
--- tools/lldb-mi/MICmnResources.h
+++ tools/lldb-mi/MICmnResources.h
@@ -300,6 +300,7 @@
     bool Shutdown(void);
 
     CMIUtilString GetString(const MIuint vResourceId) const;
+    const MIchar * GetCString(const MIuint vResourceId) const;
     bool HasString(const MIuint vResourceId) const;
 
     // Typedef:
@@ -328,7 +329,7 @@
     /* ctor */ CMICmnResources(const CMICmnResources &);
     void operator=(const CMICmnResources &);
 
-    bool GetStringFromResource(const MIuint vResourceId, CMIUtilString &vrwResourceString) const;
+    bool GetCStringFromResource(const MIuint vResourceId, const MIchar *&vrwpResourceString) const;
     bool ReadResourceStringData(void);
 
     // Overridden:
@@ -347,4 +348,4 @@
 //++ =========================================================================
 // Details: Macro short cut for retrieving a text data resource
 //--
-#define MIRSRC(x) CMICmnResources::Instance().GetString(x).c_str()
+#define MIRSRC(x) CMICmnResources::Instance().GetCString(x)
_______________________________________________
lldb-commits mailing list
lldb-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits

Reply via email to