2 ms·

Haha, you've missed the issue. The question is what does the system do when someone overrides the builtin allocator functions, but does not override all of them

by olliej 4y ago

Haha, you've missed the issue. The question is what does the system do when someone overrides the builtin allocator functions, but does not override all of them. You are absolutely correct that as a developer you can have your process override the allocator functions, and that is in fact what TF has done. The problem is that they have not overridden all of the allocation functions, and so they're crashing due to mismatching allocators being used. TF2 can "easily" fix this crash by implementing the aligned new, new[], delete, and delete[] operators in their custom allocator, or by simply removing their custom allocator's override of the global new & delete operators and using a common base class to get their faster allocator. The question we're talking about is "how does the standard library respond to this scenario in a way that maximizes correctness?".

viraptor 4y ago
I was going for "ignore the issue, let's just re-patch all alloc/free pointers, built-in or external, new or old" which I think would still work. (As long as anticheat doesn't freak out) It wouldn't suffer from inconsistencies, because you'd control all the calls again. Or is there something missing in this approach?
- olliej 4y ago
  You can't repatch all the calls. The OS/standard library provides a set of global operator new and delete implementations, and for largely historical reasons they are _required_ to allow processes to override them with their own implementations. Now when a program does decide that they're going to override the global operator new and delete functions the standard library is required to default to them instead. So generally the standard library exposes them as weak symbols, and the OS and stdlib links to them by symbol name. That way on program launch the program's version of the operator new/delete symbols are what win. So that's how the OS and standard library are able to interact with the program despite it overriding what is ostensibly the system allocator. So in principle the OS could simply make sure that the user provided operator new, delete, etc are always directed to the system allocator routines. The problem is that when compiling user code there's no obligation to call the user provided new, delete, etc through a symbol, and in general won't. Instead the calls will generally be compiled down to PC relative loads and branches as those are significantly faster. The net result is that while the OS _could_ force the symbols to always resolve to the system functions, things would break due to the user code still using the user specified functions, but those functions then would not be compatible and the result would be sadness. Hence the user defined operator implementations have to win. The problem is what happens when not all of the operators are overloaded. This historically hasn't been a problem: there's the plain and [] variants, which can be overloaded independently, and the no_throw variants of each which have in practice not been an issue because the way those are implemented by default is essentially try { return ::operator new(size); } catch(...) { return nullptr; } So does just directly the operator new that people override. The problem that operator new(size_t, align_val_t) is that depending on your compiler flags you will get different versions of ::operator new being called, and because of the alignment requirements the aligned operator new can't just forward to the default new implementation. So introducing it is the first time failure of a program to implement the full suite of operators results in an actual runtime error vs minor inefficiency. The reality is that the minimally correct solution is for all programs and libraries that overload the global allocation operators to override all of them. The better solution is for these programs and libraries to stop overloading the global allocators. Many years ago (talking >a decade at this point) when webkit first adopted a non-default allocator it overloaded the global operators. Perhaps unsurprisingly this caused issues, and now webkit (and presumably blink) do the correct thing: there's a standard base class (FastAllocated or something) that defines operator new, delete, and the [] variants, and using that as a base class results in the non-default allocator.
nemetroid 4y ago
> Haha, you've missed the issue. That's not very nice. The root comment said nothing about making the system handle this automatically, it just described an idea for a potential fix to be applied to this particular case: > It should be straightforward to make a little LD_PRELOAD shim to implement the new operator new on top of old overloads and thus restore proper functioning.
- olliej 4y ago
  > That's not very nice. :( It was not intended as a dismissive or derisive laugh at the author, but a laugh at the absurdity of the issue itself. Think "haha, you'd think that the reason is X, but technology is involved, and so everythong is terrible" vs "haha you're dumb" which sure as heck was not my intended message.

by olliej 4y ago

viraptor 4y ago

I was going for "ignore the issue, let's just re-patch all alloc/free pointers, built-in or external, new or old" which I think would still work. (As long as anticheat doesn't freak out) It wouldn't suffer from inconsistencies, because you'd control all the calls again. Or is there something missing in this approach?

olliej 4y ago
You can't repatch all the calls. The OS/standard library provides a set of global operator new and delete implementations, and for largely historical reasons they are _required_ to allow processes to override them with their own implementations. Now when a program does decide that they're going to override the global operator new and delete functions the standard library is required to default to them instead. So generally the standard library exposes them as weak symbols, and the OS and stdlib links to them by symbol name. That way on program launch the program's version of the operator new/delete symbols are what win. So that's how the OS and standard library are able to interact with the program despite it overriding what is ostensibly the system allocator. So in principle the OS could simply make sure that the user provided operator new, delete, etc are always directed to the system allocator routines. The problem is that when compiling user code there's no obligation to call the user provided new, delete, etc through a symbol, and in general won't. Instead the calls will generally be compiled down to PC relative loads and branches as those are significantly faster. The net result is that while the OS _could_ force the symbols to always resolve to the system functions, things would break due to the user code still using the user specified functions, but those functions then would not be compatible and the result would be sadness. Hence the user defined operator implementations have to win. The problem is what happens when not all of the operators are overloaded. This historically hasn't been a problem: there's the plain and [] variants, which can be overloaded independently, and the no_throw variants of each which have in practice not been an issue because the way those are implemented by default is essentially try { return ::operator new(size); } catch(...) { return nullptr; } So does just directly the operator new that people override. The problem that operator new(size_t, align_val_t) is that depending on your compiler flags you will get different versions of ::operator new being called, and because of the alignment requirements the aligned operator new can't just forward to the default new implementation. So introducing it is the first time failure of a program to implement the full suite of operators results in an actual runtime error vs minor inefficiency. The reality is that the minimally correct solution is for all programs and libraries that overload the global allocation operators to override all of them. The better solution is for these programs and libraries to stop overloading the global allocators. Many years ago (talking >a decade at this point) when webkit first adopted a non-default allocator it overloaded the global operators. Perhaps unsurprisingly this caused issues, and now webkit (and presumably blink) do the correct thing: there's a standard base class (FastAllocated or something) that defines operator new, delete, and the [] variants, and using that as a base class results in the non-default allocator.

nemetroid 4y ago

> Haha, you've missed the issue. That's not very nice. The root comment said nothing about making the system handle this automatically, it just described an idea for a potential fix to be applied to this particular case: > It should be straightforward to make a little LD_PRELOAD shim to implement the new operator new on top of old overloads and thus restore proper functioning.

olliej 4y ago
> That's not very nice. :( It was not intended as a dismissive or derisive laugh at the author, but a laugh at the absurdity of the issue itself. Think "haha, you'd think that the reason is X, but technology is involved, and so everythong is terrible" vs "haha you're dumb" which sure as heck was not my intended message.