3 ms·

It was accurate. See my comment below -- just because `SubstructureFields` had a lifetime parameter already doesn't mean it was borrowing data of a similar life

It was accurate. See my comment below -- just because `SubstructureFields` had a lifetime parameter already doesn't mean it was borrowing data of a similar lifetime. That data being borrowed could probably live longer than the attributes, but fortunately the attributes live long enough that I can equate the lifetimes. I was able to use the same lifetime parameter by luck -- there was a good chance it wouldn't have compiled and I would have had to introduce another lifetime. (Though since this was a fix the compiler suggested I didn't need to worry too much about that happening -- generally those just work) Just having a lifetime doesn't mean that it's safe to put any random borrowed data in. In C++ we could have a single pointer to a very long-lived struct ("SubstructureFields"), and wish to introduce another struct ("FieldInfo") which contains a pointer to something that is shorter lived. Note that in large codebases knowing which is "longer lived" is not easy, so from the programmer's perspective there are just two pointers. Assuming that "Okay, we don't have any segfaults now due to the first pointer, introducing the second FieldInfo pointer should be fine then" would be fallacious -- we might be accessing data during a period of time when the first, original pointer is alive, but the second is invalidated. Use after free.

SamReidHughes 12y ago
I'm trying to look more closely at the situation and will compose an answer, hopefully soon, but after going to Staples to get a $50 DVI cable so I can plug this workstation into a monitor (I just moved) and running make -j33 to build the rust compiler and play around with it, it's been thirty minutes and this is what I see: https://i.imgur.com/2JL66Fg.png https://i.imgur.com/2JL66Fg.png so bear with me. Update: You don't have to wade through a lot of code at all. All the code is taking AST parameters that are the source of these Attribute vectors by const reference and returning stuff like a P<Expr> and P<Item> and the like, AST stuff that by its nature doesn't have references with tricky lifetime dependencies to other far-flung AST stuff. So in C++ you can see right from the type signatures (and some basic institutional knowledge) that you're OK. You can see that the lifetimes are not long or dangerous. SubstructureFields is used in Substructure. A simple grep for that type shows a bunch of functions that take a Substructure by const reference and return a P<Expr>. There is nothing holding wiggly little references to Substructure objects or the like.
- pcwalton 12y ago
  You can easily write a function in C++ that takes an object by const reference and creates a dangling reference without returning it. Such functions regularly result in use-after-free (sometimes causing security vulnerabilities) in the wild. Running grep on the signatures of functions is not a sound analysis.
  - SamReidHughes 12y ago
    You can pretty well see by grepping whether it's going to create a dangling reference or not. For example, in C++ codebases I'm most familiar with, anything that takes a const foo& isn't going to retain a reference, and for unfamiliar stuff that takes a const foo* or foo*, you can look at the outputs, which, if it's a P<Expr>, all you need is the institutional knowledge that Exprs don't have dangling references. You don't need to debate with me the merits of replacing visual analysis with sound analysis. (If I have a negative opinion of Rust on the matter, it's that it's not good enough at that.) My beef here is with the way the blog article overstates the case, saying you just wouldn't do this sort of thing or this specific thing. You so would make temporary references deep into an AST while expanding derived implementations.
- Manishearth 12y ago
  Substructure objects contain other objects contain other objects and so on. Are you going to grep for all of these? Yes, in this situation owned Expr pointers are returned, and fortunately I know that Expr contained no borrowed data (with a lack of lifetime annotation that is easy, but in C++ you might have to check) But there could be other pointers being passed around internally (eg replacing the old pointer from a field of an argument with a new one, one which actually is short lived or is being iterated over. Just because a pointer isn't returned doesn't mean we're safe. With const we get some safety until interior mutability and casts come into the picture -- as I said before, consting a complex codebase is not easy unless you keep switching back and forth.

SamReidHughes 12y ago

I'm trying to look more closely at the situation and will compose an answer, hopefully soon, but after going to Staples to get a $50 DVI cable so I can plug this workstation into a monitor (I just moved) and running make -j33 to build the rust compiler and play around with it, it's been thirty minutes and this is what I see: https://i.imgur.com/2JL66Fg.png https://i.imgur.com/2JL66Fg.png so bear with me. Update: You don't have to wade through a lot of code at all. All the code is taking AST parameters that are the source of these Attribute vectors by const reference and returning stuff like a P<Expr> and P<Item> and the like, AST stuff that by its nature doesn't have references with tricky lifetime dependencies to other far-flung AST stuff. So in C++ you can see right from the type signatures (and some basic institutional knowledge) that you're OK. You can see that the lifetimes are not long or dangerous. SubstructureFields is used in Substructure. A simple grep for that type shows a bunch of functions that take a Substructure by const reference and return a P<Expr>. There is nothing holding wiggly little references to Substructure objects or the like.

pcwalton 12y ago
You can easily write a function in C++ that takes an object by const reference and creates a dangling reference without returning it. Such functions regularly result in use-after-free (sometimes causing security vulnerabilities) in the wild. Running grep on the signatures of functions is not a sound analysis.
- SamReidHughes 12y ago
  You can pretty well see by grepping whether it's going to create a dangling reference or not. For example, in C++ codebases I'm most familiar with, anything that takes a const foo& isn't going to retain a reference, and for unfamiliar stuff that takes a const foo* or foo*, you can look at the outputs, which, if it's a P<Expr>, all you need is the institutional knowledge that Exprs don't have dangling references. You don't need to debate with me the merits of replacing visual analysis with sound analysis. (If I have a negative opinion of Rust on the matter, it's that it's not good enough at that.) My beef here is with the way the blog article overstates the case, saying you just wouldn't do this sort of thing or this specific thing. You so would make temporary references deep into an AST while expanding derived implementations.
Manishearth 12y ago
Substructure objects contain other objects contain other objects and so on. Are you going to grep for all of these? Yes, in this situation owned Expr pointers are returned, and fortunately I know that Expr contained no borrowed data (with a lack of lifetime annotation that is easy, but in C++ you might have to check) But there could be other pointers being passed around internally (eg replacing the old pointer from a field of an argument with a new one, one which actually is short lived or is being iterated over. Just because a pointer isn't returned doesn't mean we're safe. With const we get some safety until interior mutability and casts come into the picture -- as I said before, consting a complex codebase is not easy unless you keep switching back and forth.