[Lldb-commits] Need to change swig typemap for reading C strings

[Jason Molenda via lldb-commits]

Hi Zachary, reviving a problem I initially found a year ago -- in r253487 where a swig typemap was changed for string reading methods.

The problem we're seeing with this change is that SBProcess::ReadCStringFromMemory() now returns a None object when you try to read a zero-length string, and this is breaking a couple of groups' python scripts here at Apple.  (it was a low priority thing for me to fix, until some behavior changed recently and it started biting the groups a lot more frequently).

SBProcess::ReadCStringFromMemory() takes an SBError object and returns a string.  We have three cases:


1 Non-zero length string read.  SBError says it was successful, string should be returned.

2 Zero-length / empty string read.  SBError says it was successful, string should be returned (Python None object is returned right now)

3 Memory read error.  SBError says it is an error, ideally None object should be returned.


For instance,

#include <stdlib.h>
int main ()
{
    const char *empty_string = "";
    const char *one_letter_string = "1";
    const char *invalid_memory_string = (char*)0x100; // lower 4k is always PAGEZERO & unreadable on darwin
    return empty_string[0] + one_letter_string[0]; // breakpoint here
}

we'll see:

  (lldb) br s -p breakpoint
  (lldb) r
  (lldb) p empty_string
  (const char *) $0 = 0x0000000100000fae ""
  (lldb) p one_letter_string
  (const char *) $1 = 0x0000000100000faf "1"
  (lldb) p invalid_memory_string
  (const char *) $2 = 0x0000000000000100 ""
  (lldb) scri
  >>> err = lldb.SBError()

  >>> print lldb.process.ReadCStringFromMemory(0x0000000100000fae, 2048, err)
  None
  >>> print err
  success

  >>> print lldb.process.ReadCStringFromMemory(0x0000000100000faf, 2048, err)
  1
  >>> print err
  success

  >>> print lldb.process.ReadCStringFromMemory(0x0000000000000100, 2048, err)
  None
  >>> print err
  error: memory read failed for 0x0
  >>> print err.Success()
  False
  >>> 


Before r253487, the read of a zero-length string and the read of an invalid address would both return a zero length python string (and the latter would set the SBError).  After the change, both of these return a python None object (and the latter sets the SBError).


I haven't worked with the typemaps before -- I can restore the previous behavior where an empty Python string is returned for both the zero-length string and for the unreadable address.  I don't see how I can access the SBError object used earlier in these methods.


diff --git i/scripts/Python/python-typemaps.swig w/scripts/Python/python-typemaps.swig
index df16a6d27b3..29e5d9b156d 100644
--- i/scripts/Python/python-typemaps.swig
+++ w/scripts/Python/python-typemaps.swig
@@ -102,7 +102,8 @@
 %typemap(argout) (char *dst, size_t dst_len) {
    Py_XDECREF($result);   /* Blow away any previous result */
    if (result == 0) {
-      $result = Py_None;
+      lldb_private::PythonString string("");
+      $result = string.release();
       Py_INCREF($result);
    } else {
       llvm::StringRef ref(static_cast<const char*>($1), result);



This does cause one test in the testsuite to fail --

======================================================================
FAIL: test_snapshot_minidump (TestMiniDumpNew.MiniDumpNewTestCase)
   Test that if we load a snapshot minidump file (meaning the process
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Volumes/newwork/github/stable/lldb/packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py", line 88, in test_snapshot_minidump
    self.assertEqual(stop_description, None)
AssertionError: '' != None
Config=x86_64-/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang
----------------------------------------------------------------------


which is doing


        thread = self.process.GetThreadAtIndex(0)
        self.assertEqual(thread.GetStopReason(), lldb.eStopReasonNone)
        stop_description = thread.GetStopDescription(256)
        self.assertEqual(stop_description, None)


SBThread::GetStopDescription doesn't have an SBError object to indicate that there is no stop description for eStopReasonNone.  I don't think this will be a problem if eStopReasonNone is returning an empty python string for the StopDescription.


I'm not wedded to my current patch, but we do have to come up with something that can return a zero length python string for a method like SBProcess::ReadCStringFromMemory.