0

I'm having a similar issue to this fellow, but the problem is not the same cause as in that answer.

I'm getting a const char * from a C function. This function is actually implemented in C++ and it returns a struct by value which contains the return value of std::string c_str() which is a C pointer to the string contents. This string lives in static memory. Since the pointer is copied into the return, I would expect the data to be fine.

According to Xcode's debugger, the string I get back is actually the correct length as I'd expect (see countAndFlags below with correct character count of 7), but the contents are garbage. This is in a UITableView method, and what's very strange is that the garbage problem disappears if I use the reloadData() TableView method (which I've wired to a button) which then has valid content for the name variable below. However this subsequent invocation of reloadData() does not alter any of the data used elsewhere in the table, only this problematic string garbage goes away.

enter image description here

Here is the debugger on first pass, showing garbage but correct length:

enter image description here

And here is the next pass when the text is correct:

enter image description here

I'm at a loss to understand this behaviour. The struct s contains others properties that are not corrupted when they are returned, but this char * is garbage on first pass only, with correct length nonetheless.

I'd appreciate any pointers (no pun intended) on what is happening here.

Finally, here is the C++ code for the function returning the C string:

scene getScene(sceneID s){
  static std::map<sceneID, std::vector<nodeID> > nodeArrays;
  auto ss = globalObjects.scenes.at(s);
  auto sceneroots = ss.roots;
  if (nodeArrays.count(s)>0) nodeArrays.at(s).clear();
  else confirmInsertion(nodeArrays.emplace(s, std::vector<nodeID>{}));
  auto v = nodeArrays.at(s);
  v.reserve(sceneroots.size());
  for (const auto&i:sceneroots) v.push_back(i);
  scene sr;
  sr.name=ss.name.c_str();
  sr.roots=vec2array(v);
  return sr;
}

If I log sr.name it is always correct.

Here is scene:

  typedef struct {
    const char* name;
    arrayID roots;
  } scene;
Community
  • 1
  • 1
johnbakers
  • 24,158
  • 24
  • 130
  • 258
  • Did you try reproducing the problem outside of a `UITableView`? Also, in what mathod of `UITableView` are you calling this C function? – Tomer Jun 12 '16 at 17:20
  • @Tomer thanks, I'm calling it inside `cellForRowAtIndexPath` -- is that a bad idea? I'm a bit new to the Apple APIs – johnbakers Jun 12 '16 at 17:24
  • 2
    "This string lives in static memory. Since the pointer is copied into the return, I would expect the data to be fine" I don't know C++ but surely this expectation is suspect. Maybe you should show more about how you are obtaining this C string, in the hopes that someone who knows about C++ behavior can come along and answer. You might want to add C++ tag to your question too. – matt Jun 12 '16 at 17:25
  • Also show the code between when you obtain the C string and when you call `String.fromCString`. Since this thing is merely a pointer, there is always a danger that, despite your expectations, it is getting shifted out from under you somehow. – matt Jun 12 '16 at 17:27
  • @johnbakers I guess it should be fine as long as the C string does not get destroyed somehow. Try to isolate the problem from `UITableView` – Tomer Jun 12 '16 at 17:29
  • @matt I've added the c++ tag. However, a simple std::string c_str() pointer on a string that lives in memory (and is not local to any function) is what is being passed here. – johnbakers Jun 12 '16 at 17:31
  • What do you mean by "lives in memory (and is not local to any function)"? See http://stackoverflow.com/questions/6456359/what-is-stdstringc-str-lifetime – jtbandes Jun 12 '16 at 17:32
  • @jtbandes I mean the string is stored in a global std::map and it persists long after these functions are done. Thus the c_str pointer on that string should persist as well. And in fact subsequent invocations of these functions do get the correct value without making any changes to memory anywhere, so the issue does not appear related to the storage of the string itself. – johnbakers Jun 12 '16 at 17:37
  • Perhaps you could try doing the same thing from C or Obj-C and see if it works as expected? Or access the raw bytes to see if they look right before you try to use the String initializer? – jtbandes Jun 12 '16 at 17:38
  • Also try to eliminate potential causes by always returning the same C string, or starting from scratch to create the smallest amount of code that reproduces the problem. – Tomer Jun 12 '16 at 17:59
  • @Tomer I shall have an update in a few minutes with much more interesting information about the issue. – johnbakers Jun 12 '16 at 18:02
  • @jtbandes updated. – johnbakers Jun 12 '16 at 18:05
  • @johnbakers You've described the C++ code, but you never actually posted it. C++ is a language where mere description is not enough. You say "static", but "static" has multiple definitions in the C++ language. – PaulMcKenzie Jun 12 '16 at 18:08
  • @PaulMcKenzie I added the C++ function. – johnbakers Jun 12 '16 at 18:18
  • @johnbakers Look at your description: `std::string c_str()`. That pointer is not guaranteed to point to the same data after that any non-const to the string. Given this, a C++ programmer would never save that pointer returned from `c_str()` "for later use", but only for "immediate use". If you copied the contents of the `c_str()` call to a buffer, then we can assume that the contents are safe within that buffer you copied to. – PaulMcKenzie Jun 12 '16 at 18:19
  • @PaulMcKenzie c_str() is const function on std::string: http://www.cplusplus.com/reference/string/string/c_str/ – johnbakers Jun 12 '16 at 18:20
  • `sr.name=ss.name.c_str();` If you look at `ss`, it is a local variable. So you're saving a local variable's `c_str()` address. As soon as that function returns, `ss` goes up in a puff of smoke and disappears. – PaulMcKenzie Jun 12 '16 at 18:21
  • @PaulMcKenzie feel free to make this an answer, it is useful and I'll try it right now. However, is this not a copied pointer? If the original string is not mutated, wouldn't a pointer copied to its c_str pointer still point to it and be valid? – johnbakers Jun 12 '16 at 18:23
  • @johnbakers Please post the declaration of `scene.name`. If it is a mere pointer, then I will post my last comment as an answer. If it is a type that knows to make a copy of the *data* pointed to (not the pointer itself), then the code would / should be ok. – PaulMcKenzie Jun 12 '16 at 18:25
  • @PaulMcKenzie I have added it, thanks. – johnbakers Jun 12 '16 at 18:30

2 Answers2

1

One issue you should observe is this within the C++ function:

Given that this is your scene definition:

typedef struct {
    const char* name;
    arrayID roots;
  } scene;

Inside the getScene C++ function, you're doing this:

auto ss = globalObjects.scenes.at(s);

Then you do this later:

  scene sr;
  sr.name = ss.name.c_str();
  //...
  return sr;

The issue is that since ss is a local variable, the ss.name.c_str() pointer assigned to sr.name will not be valid when the function returns. Accessing this pointer leads to undefined behavior.

One fix for the problem is to copy the contents of the string to a buffer that is sent by the client program, or copy the contents to a buffer you know is available by the client and won't be invalidated. Since I do not know Swift, you will have to negotiate how to accomplish this.


Another solution would be to make sure you are accessing the same std::string, not a copy of the std::string:

auto& ss = globalObjects.scenes.at(s);

Now a reference to the scene is returned. But then you have the issue of making sure that the string within the scene is not mutated at all when you actually refer to the returned c_str() value later on in your application. A C++ programmer would not hold onto the return value of c_str() for an extended time, since this can lead to hard-to-diagnose bugs.


The bottom line is that always be suspicious of returning a character pointer as a means of returning string data. The usual way that an API returns string data is to have the client supply a buffer and the API fills the buffer with the string data, or less likely, the API allocates memory and returns a pointer to the allocated memory (which leads to complications as the API has to free the memory by some means to avoid memory leaks).

PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • I did this instead and it worked: `const char* getSceneName(sceneID){ return globalObjects().scenes.at(s).name.c_str(); }` – johnbakers Jun 12 '16 at 18:41
  • however, I don't know why: ss is a ref, not a local object (the return from at() on a map) and also, the pointer is copied from a valid pointer, so it should point to the same thing since that original string is never mutated. Between these two facts, I don't know why the c_str() pointer is invalidated. – johnbakers Jun 12 '16 at 18:42
  • The reason why there is a difference is that you are not creating an intermediate (local) variable, and then accessing that local variable's `c_str()` return value. – PaulMcKenzie Jun 12 '16 at 18:45
  • I guess the pointer from c_str() doesn't behave the way most pointers are thought of? normally, a pointer when copied still points to the same original memory. I understands pointers are invalidated in classes like string and vector if you mutate that original memory, but I never do in this case, so just copying a pointer shouldn't point to the wrong place. – johnbakers Jun 12 '16 at 18:46
  • @johnbakers As stated, the problem is when you used a local variable. Your latest attempt uses no local variables. – PaulMcKenzie Jun 12 '16 at 18:48
  • Thanks for the help. I'l have to open another question for clarification here. Yes I have a local variable but I am not copying the memory anywhere. You can copy and pass around pointers freely in C/C++ and they always point to the same memory until the underlying class mutates or moves that data, which is not happening here. – johnbakers Jun 12 '16 at 18:50
  • @johnbakers See my edit. You may want to refer to the same string object by using a reference variable, not a copy of the string object. – PaulMcKenzie Jun 12 '16 at 18:53
  • What is the type of ss in `auto ss = globalObjects.scenes.at(s);` ? According to the docs, `.at()` on `std::map` returns a reference, therefore `ss` is already a reference, right? I am not actually copying the string to a local stack space anywhere in my example, of if I am, that is not my intention. `ss.name.c_str();` is a pointer to memory that is *not* local to this function. – johnbakers Jun 12 '16 at 18:58
  • wow, it appears all my problems here come down to this: http://stackoverflow.com/questions/8542873/c11-auto-semantics i totally thought the type deduction would give me a reference automatically – johnbakers Jun 12 '16 at 19:03
  • @johnbakers Just as my latest demonstrated. If indeed the `auto` would automatically infer it is a reference, that would be at odds with simplifying code like this: `scene s = globalObjects.scene.at(s)` from ever being possible (where the programmer indeed wants a copy). So `auto` requires you to state "I want a reference" by specifying `&`. – PaulMcKenzie Jun 12 '16 at 19:29
0

It seems like you are passing a valid C string at the time you call String.fromCString which is later being invalidated. As you note, the Swift String is the correct length, but from the debugger you can see that it has embedded \0 in it. The only way I can think of for that to happen is if the C string was valid initially, and then it got corrupted before Swift finished parsing it.

From http://www.cplusplus.com/reference/string/string/c_str/:

The pointer returned points to the internal array currently used by the string object to store the characters that conform its value.

The pointer returned may be invalidated by further calls to other member functions that modify the object.

You should strdup the C string in your C++ code so that you know that it can't change after that, and then you can confirm that your Swift code is working as expected.

Ewan Mellor
  • 6,747
  • 1
  • 24
  • 39