Get Version SWIG wrapper should fill the list it makes

[jingham]

Summary

The "uint32_t *versions, uint32_t num_versions" wrapper makes a list with the number of elements that SBModule::GetVersion said it returns, but if one of those elements is UINT32_MAX it aborts filling the list. If you then iterate over that returned list in Python, you will get a crash. This patch will fill all the elements GetVersion said it had.

This comes up if you have a library with a version 0.0.0. That returns the (to me) not very sensical result of 2 elements, the first of which is UINT32_MAX.

This patch is just to handle the crash that that result causes in Python. I asked on lldb-dev why GetVersion does this - maybe there is some good reason I don't know. But anyway, I want to treat that as a separate issue.

[labath]

The fix looks correct, but I am wondering if it wouldn't be possible to test this in a more platform-independent manner. Please see inline comment below.

packages/Python/lldbsuite/test/macosx/version_zero/TestGetVersionZeroVersion.py

2–50

Would it be possible to generate this binary via yaml2obj (e.g. compile it manually, like you did above, and then yamlize it)? Then, if you avoid running the target and just load the module by creating the SBModule object directly from a ModuleSpec, you should get a test that is able to run on all platforms, and not just darwin.

[jingham]

Use yaml2obj to create the dylib with version 0.0.0

packages/Python/lldbsuite/test/macosx/version_zero/TestGetVersionZeroVersion.py

+"""
+Read in a library with a version number of 0.0.0, make sure we produce a good version.
+"""
+
+from __future__ import print_function
+
+
+import os
+import time
+import re
+import lldb
+from lldbsuite.test import decorators
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestGetVersionForZero(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    # If your test case doesn't stress debug info, the
+    # set this to true.  That way it won't be run once for
+    # each debug info format.
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test_get_version_zero(self):
+        """Read in a library with a version of 0.0.0.  Test SBModule::GetVersion"""
+        self.yaml2obj("libDylib.dylib.yaml", self.getBuildArtifact("libDylib.dylib"))
+        self.do_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def do_test(self):
+        lib_name = "libDylib.dylib"
+        target = lldbutil.run_to_breakpoint_make_target(self, exe_name=lib_name)
+        module = target.FindModule(lldb.SBFileSpec(lib_name))
+        self.assertTrue(module.IsValid(), "Didn't find the libDylib.dylib module")
+        # For now the actual version numbers are wrong for a library of 0.0.0
+        # but the previous code would crash iterating over the resultant
+        # list.  So we are testing that that doesn't happen.
+        did_iterate = False
+        for elem in module.GetVersion():
+            did_iterate = True
+        self.assertTrue(did_iterate, "Didn't get into the GetVersion loop")
+

packages/Python/lldbsuite/test/macosx/version_zero/libDylib.dylib.yaml

+--- !mach-o
+FileHeader:      
+  magic:           0xFEEDFACF
+  cputype:         0x01000007
+  cpusubtype:      0x00000003
+  filetype:        0x00000006
+  ncmds:           12
+  sizeofcmds:      672
+  flags:           0x00100085
+  reserved:        0x00000000
+LoadCommands:    
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         232
+    segname:         __TEXT
+    vmaddr:          0
+    vmsize:          4096
+    fileoff:         0
+    filesize:        4096
+    maxprot:         5
+    initprot:        5
+    nsects:          2
+    flags:           0
+    Sections:        
+      - sectname:        __text
+        segname:         __TEXT
+        addr:            0x0000000000000FA0
+        size:            11
+        offset:          0x00000FA0
+        align:           4
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x80000400
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+      - sectname:        __unwind_info
+        segname:         __TEXT
+        addr:            0x0000000000000FAC
+        size:            72
+        offset:          0x00000FAC
+        align:           2
+        reloff:          0x00000000
+        nreloc:          0
+        flags:           0x00000000
+        reserved1:       0x00000000
+        reserved2:       0x00000000
+        reserved3:       0x00000000
+  - cmd:             LC_SEGMENT_64
+    cmdsize:         72
+    segname:         __LINKEDIT
+    vmaddr:          4096
+    vmsize:          4096
+    fileoff:         4096
+    filesize:        528
+    maxprot:         1
+    initprot:        1
+    nsects:          0
+    flags:           0
+  - cmd:             LC_ID_DYLIB
+    cmdsize:         56
+    dylib:           
+      name:            24
+      timestamp:       1
+      current_version: 0
+      compatibility_version: 0
+    PayloadString:   '@executable_path/libDylib.dylib'
+    ZeroPadBytes:    1
+  - cmd:             LC_DYLD_INFO_ONLY
+    cmdsize:         48
+    rebase_off:      0
+    rebase_size:     0
+    bind_off:        0
+    bind_size:       0
+    weak_bind_off:   0
+    weak_bind_size:  0
+    lazy_bind_off:   0
+    lazy_bind_size:  0
+    export_off:      4096
+    export_size:     16
+  - cmd:             LC_SYMTAB
+    cmdsize:         24
+    symoff:          4120
+    nsyms:           10
+    stroff:          4280
+    strsize:         344
+  - cmd:             LC_DYSYMTAB
+    cmdsize:         80
+    ilocalsym:       0
+    nlocalsym:       8
+    iextdefsym:      8
+    nextdefsym:      1
+    iundefsym:       9
+    nundefsym:       1
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
+    nmodtab:         0
+    extrefsymoff:    0
+    nextrefsyms:     0
+    indirectsymoff:  0
+    nindirectsyms:   0
+    extreloff:       0
+    nextrel:         0
+    locreloff:       0
+    nlocrel:         0
+  - cmd:             LC_UUID
+    cmdsize:         24
+    uuid:            5F76D8E3-7EA5-3092-8A9D-0D0E36429550
+  - cmd:             LC_BUILD_VERSION
+    cmdsize:         32
+    platform:        1
+    minos:           659200
+    sdk:             659200
+    ntools:          1
+    Tools:           
+      - tool:            3
+        version:         33227776
+  - cmd:             LC_SOURCE_VERSION
+    cmdsize:         16
+    version:         0
+  - cmd:             LC_LOAD_DYLIB
+    cmdsize:         56
+    dylib:           
+      name:            24
+      timestamp:       2
+      current_version: 83427328
+      compatibility_version: 65536
+    PayloadString:   '/usr/lib/libSystem.B.dylib'
+    ZeroPadBytes:    6
+  - cmd:             LC_FUNCTION_STARTS
+    cmdsize:         16
+    dataoff:         4112
+    datasize:        8
+  - cmd:             LC_DATA_IN_CODE
+    cmdsize:         16
+    dataoff:         4120
+    datasize:        0
+LinkEditData:    
+  ExportTrie:      
+    TerminalSize:    0
+    NodeOffset:      0
+    Name:            ''
+    Flags:           0x0000000000000000
+    Address:         0x0000000000000000
+    Other:           0x0000000000000000
+    ImportName:      ''
+    Children:        
+      - TerminalSize:    3
+        NodeOffset:      9
+        Name:            _func
+        Flags:           0x0000000000000000
+        Address:         0x0000000000000FA0
+        Other:           0x0000000000000000
+        ImportName:      ''
+  NameList:        
+    - n_strx:          25
+      n_type:          0x64
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+    - n_strx:          148
+      n_type:          0x64
+      n_sect:          0
+      n_desc:          0
+      n_value:         0
+    - n_strx:          156
+      n_type:          0x66
+      n_sect:          3
+      n_desc:          1
+      n_value:         1553735575
+    - n_strx:          1
+      n_type:          0x2E
+      n_sect:          1
+      n_desc:          0
+      n_value:         4000
+    - n_strx:          332
+      n_type:          0x24
+      n_sect:          1
+      n_desc:          0
+      n_value:         4000
+    - n_strx:          1
+      n_type:          0x24
+      n_sect:          0
+      n_desc:          0
+      n_value:         11
+    - n_strx:          1
+      n_type:          0x4E
+      n_sect:          1
+      n_desc:          0
+      n_value:         11
+    - n_strx:          1
+      n_type:          0x64
+      n_sect:          1
+      n_desc:          0
+      n_value:         0
+    - n_strx:          2
+      n_type:          0x0F
+      n_sect:          1
+      n_desc:          0
+      n_value:         4000
+    - n_strx:          8
+      n_type:          0x01
+      n_sect:          0
+      n_desc:          256
+      n_value:         0
+  StringTable:     
+    - ' '
+    - _func
+    - dyld_stub_binder
+    - '/Volumes/ThePlayground/Users/jingham/Work/LLDB/llvm-dot-org/lldb-clean/packages/Python/lldbsuite/test/macosx/version_zero/'
+    - dylib.c
+    - '/Volumes/ThePlayground/Users/jingham/Work/LLDB/llvm-dot-org/lldb-clean/test/lldb-test-build.noindex/macosx/version_zero/TestGetVersionZeroVersion.test_get_version_zero/dylib.o'
+    - _func
+    - ''
+    - ''
+    - ''
+    - ''
+    - ''
+    - ''
+...

scripts/Python/python-typemaps.swig

 
 %typemap(argout) (uint32_t *versions, uint32_t num_versions) {
     uint32_t count = result;
     if (count >= $2)
         count = $2;
     PyObject* list = PyList_New(count);
     for (uint32_t j = 0; j < count; j++)
     {
-        if ($1[j] < UINT32_MAX)
-        {
-            PyObject* item = PyInt_FromLong($1[j]);
-            int ok = PyList_SetItem(list,j,item);
-            if (ok != 0)
-            {
-                $result = Py_None;
-                break;
-            }
-        }
-        else
-            break;
-    }
     $result = list;
 }
 
 %typemap(freearg) (uint32_t *versions) {
     free($1);
 }