Skip to content

Who manages STRING_ARRAY/STRING_POINTER? #472

Closed
Closed
Who manages STRING_ARRAY/STRING_POINTER?#472
Labels
question

Description

@CookStar
CookStar
opened on May 17, 2023

Who manages the char* set by Pointer.set_string_pointer? Is it impossible to set a string to a char* managed by a C++ object?

Test Code 1:

from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(4, False)
s_p2 = alloc(4, False)

s_p1.set_string_pointer(string)
s_p2.set_string_pointer(string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

s_p1.set_string_pointer(s_p2.get_string_pointer())

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

print("string1", s_p1.get_string_pointer())
print("string2", s_p2.get_string_pointer())

s_p1.get_pointer().dealloc()

Output 1:

uint1 3949688248
uint2 3949688312
uint1 3953894104
uint2 3949688312
string1 12
string2 string2
free(): invalid pointer

First, I don't know why string1 is set to 12 instead of string2 as in string2, but trying to free string1 would result in an illegal pointer deallocation. Who exactly is managing this char*?

There is also a problem with TypeManager.pointer_attribute not being able to set STRING_ARRAY.

Test Code 2:

from memory.manager import CustomType
from memory.manager import Type
from memory.manager import TypeManager

manager = TypeManager()

class Test(CustomType, metaclass=manager):
    _size = 4
    string = manager.pointer_attribute(Type.STRING_ARRAY, 0)

test = Test()
test.string = "test"

Output 2:

[Source.Python]
[SP] Caught an Exception:
Traceback (most recent call last):
  File "../addons/source-python/packages/source-python/plugins/command.py", line 164, in load_plugin
    plugin = self.manager.load(plugin_name)
  File "../addons/source-python/packages/source-python/plugins/manager.py", line 207, in load
    plugin._load()
  File "../addons/source-python/packages/source-python/plugins/instance.py", line 74, in _load
    self.module = import_module(self.import_name)
  File "../addons/source-python/plugins/test/test.py", line 4214, in <module>
    test.string = "test"
  File "../addons/source-python/packages/source-python/memory/manager.py", line 515, in fset
    instance_ptr = alloc(TYPE_SIZES[type_name.upper()])

KeyError: 'STRING_ARRAY'

STRING_ARRAY also allows buffer overrun, and it seems that STRING-related issues are somewhat problematic.

Activity

  1. jordanbriere commented on May 17, 2023

    @jordanbriere
    jordanbriere
    on May 17, 2023
    Contributor

    char * is really just a pointer to an array. Perhaps this can give you some hints:

    from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(len(string) + 1, False)
s_p2 = alloc(len(string2) + 1, False)

s_p1.set_string_array(string)
s_p2.set_string_array(string2)

# struct { char *s1;, char *s2; }
p = alloc(8, False)
p.set_pointer(s_p1)
p.set_pointer(s_p2, 4)

print("p.s1", p.get_string_pointer(), p.get_pointer().get_string_array())
print("p.s2", p.get_string_pointer(4), p.get_pointer(4).get_string_array())

p.get_pointer().dealloc()
p.get_pointer(4).dealloc()

# These are now freed...
print(s_p1.get_string_array())
print(s_p2.get_string_array())

  2. CookStar commented on May 17, 2023

    @CookStar
    CookStar
    on May 17, 2023
    ContributorAuthor

    char * is really just a pointer to an array. Perhaps this can give you some hints:

    I understand how get/set_string_array works, it is set_string_pointer I am asking about.

    Who manages the char* set by Pointer.set_string_pointer? Is it impossible to set a string to a char* managed by a C++ object?

  3. CookStar commented on May 17, 2023

    @CookStar
    CookStar
    on May 17, 2023
    ContributorAuthor

    I will change the question, who determines the lifetime of this string object? When does it end?

    from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

  4. jordanbriere commented on May 17, 2023

    @jordanbriere
    jordanbriere
    on May 17, 2023
    Contributor

    I will change the question, who determines the lifetime of this string object? When does it end?

    from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

    s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

  5. CookStar commented on May 17, 2023

    @CookStar
    CookStar
    on May 17, 2023
    ContributorAuthor

    s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

    The actual behavior is not like that, set_string_pointer clearly sets the pointer of the string object.

    Code:

    from memory import alloc

p1 = memory.alloc(4, False)
print(p1.get_uint())
print(p1.get_pointer().address)
p1.set_string_pointer("string")
print(p1.get_uint())
print(p1.get_pointer().address)
print(p1.get_string_pointer())

p2 = memory.alloc(4, False)
print(p2.get_uint())
print(p2.get_pointer().address)

p2.get_pointer().set_string_array("string")

    Output:

    0
0
4023810712
4023810712
string
0
0
[Source.Python]
[SP] Caught an Exception:
Traceback (most recent call last):
  File "../addons/source-python/packages/source-python/plugins/command.py", line 164, in load_plugin
    plugin = self.manager.load(plugin_name)
  File "../addons/source-python/packages/source-python/plugins/manager.py", line 207, in load
    plugin._load()
  File "../addons/source-python/packages/source-python/plugins/instance.py", line 74, in _load
    self.module = import_module(self.import_name)
  File "../addons/source-python/plugins/test/test.py", line 4195, in <module>
    p2.get_pointer().set_string_array("string")

ValueError: Pointer is NULL.

  6. Ayuto commented on May 17, 2023

    @Ayuto
    Ayuto
    on May 17, 2023
    Member

    I will change the question, who determines the lifetime of this string object? When does it end?

    from memory import alloc

s_p1 = alloc(4, False)
s_p1.set_string_pointer("string")

    s_p1.set_string_pointer("string") is the exact equivalence of s_p1.get_pointer().set_string_array("string"). Which obviously is undefined, because your memory is uninitialized and your pointer holds an unknown address.

    This is not quite the equivalence. Let me bring some light into this topic by adding some comments to the very first example. set_string_pointer is actually a little bit dangerous, because you always have to keep a copy of the string alive in Python. As soon as your Python string is garbage collected, the pointer that has been set is also getting invalid. set_string_array is different, because it copies the bytes of the given string into an existing pointer.

    from memory import alloc

string = "string1"
string2 = "string2"

s_p1 = alloc(4, False)
s_p2 = alloc(4, False)

# These two calls are perfectly fine, because your string is stored in a Python
# variable. When you delete the variable "string" or assign a new value,
# the pointer is invalidated and it's not clear what exactly is returned by
# get_string_pointer. But let's continue in the example...
s_p1.set_string_pointer(string)
s_p2.set_string_pointer(string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

# This will cause undefined behaviour, because get_string_pointer returns a new(!) Python string
# which is then set using set_string_pointer. After that line the returned string is 
# invalidated/forgotten by Python.
s_p1.set_string_pointer(s_p2.get_string_pointer())

# The following would work as we keep a copy of the string:
# new_string = s_p2.get_string_pointer()
# s_p1.set_string_pointer(new_string)

print("uint1", s_p1.get_uint())
print("uint2", s_p2.get_uint())

# Since s_p1 was set to a valid pointer, which was then invalidated, the result of
# s_p1.get_string_pointer() is now undefined.
print("string1", s_p1.get_string_pointer())
print("string2", s_p2.get_string_pointer())

# This will try to deallocated a pointer, which is not managed by you.
# This is the invalidated pointer that was managed by Python
s_p1.get_pointer().dealloc()

  7. jordanbriere commented on May 17, 2023

    @jordanbriere
    jordanbriere
    on May 17, 2023
    Contributor

    I think this is an oversight of Pointer.set_string_pointer. The address is likely a capsule allocated by Boost, and its lifetime is valid only for the duration of the call.

  8. Ayuto
    added
    question
    on May 17, 2023

  9. Ayuto commented on May 17, 2023

    @Ayuto
    Ayuto
    on May 17, 2023
    Member

    STRING_ARRAY also allows buffer overrun [...]

    Yes, this is something you need to take care about. We are not able to determine maximum possible length.

  10. CookStar commented on May 17, 2023

    @CookStar
    CookStar
    on May 17, 2023
    ContributorAuthor

    This is not quite the equivalence. Let me bring some light into this topic by adding some comments to the very first example. set_string_pointer is actually a little bit dangerous, because you always have to keep a copy of the string alive in Python. As soon as your Python string is garbage collected, the pointer that has been set is also getting invalid. set_string_array is different, because it copies the bytes of the given string into an existing pointer.

    Yes, this is something you need to take care about. We are not able to determine maximum possible length.

    That seems to be the answer I was expecting, thank you.
    So, it means we cannot safely use STRING_ARRAY/STRING_POINTER for instance_attribute/pointer_attribute in TypeManager.

  11. Ayuto commented on May 17, 2023

    @Ayuto
    Ayuto
    on May 17, 2023
    Member

    Yes, reading is fine, but writing requires some extra caution at the moment.

  12. Ayuto commented on May 20, 2023

    @Ayuto
    Ayuto
    on May 20, 2023
    Member

    @CookStar Do you have any further questions or can we close this issue?

  13. CookStar commented on May 21, 2023

    @CookStar
    CookStar
    on May 21, 2023
    ContributorAuthor

    No more questions. Thanks for the answers. I have created a fix(#474) for some of the problems, so if you have any comments, please let me know.

  14. Ayuto
    closed this as completedon Apr 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    question

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions