-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Contradicting documentation on default allocator #108
Comments
I think one solution here would be to decide that
And also given that (IIUC) it's actually a vendored dlmalloc port on WebAssembly targets |
So basically you are saying the second docs I quoted are right and the first one should be removed? |
That's my opinion, yes. |
I think we shouldn't assume |
Yeah, that's a pretty compelling point. While I do feel like |
Miri runs binary crates, not lib crates, so it's not possible for someone else to set anything. If the documentation says that Rust works as if this exists by default #[global_allocator]
static A: System = System; then we have to allow such code, we have no grounds for calling it UB. So @bjorn3 are you saying we should remove the 2nd documentation and keep the first? |
Miri is often used to test library crates.
Yes. |
Yes -- by running a binary crate that exercises them (the test crate). What I said remains true. |
@bjorn3 I disagree with this, because it means there's no way to "wrap" the default allocator. I should be able to write something like this: #[global_allocator]
static A: MyAllocationTracker = MyAllocationTracker::new(System); And have it track my allocations without otherwise changing the behaviour of my program. If you think |
It shouldn't change the behavior to switch from the default allocator to the system allocator. At most the performance profile. If we decided that the |
FTR, |
In practice it can. We need a way to get precisely the default behaviour, not just "approximately". Whether we do that by upholding the guarantee that System is the default, or by introducing a new |
I'm unsure you could rely upon that. Making it generally unspecified would
allow, for example, delegating to C++ operator new (which I contemplated
doing for proc-macros back when lccc was written in C++ - it switched to
just pulling in xlang_allocate via custom allocator).
…On Sat, Nov 19, 2022 at 10:59 bjorn3 ***@***.***> wrote:
And have it track my allocations without otherwise changing the behaviour
of my program.
It shouldn't change the behavior to switch from the default allocator to
the system allocator. At most the performance profile. If we decided that
the System allocator is always the default allocator, then
MyAllocationTracker::new(System) would not cause any perf regression
other than inherent to MyAllocationTracker. If we decided to use a
different default allocator, we did likely do so to improve performance. In
that case using MyAllocationTracker::new(System) would be a perf
regression, but not beyond what we would have anyway by deciding to only
use System as default allocator. If we decide to only use System as
default, maybe should rename System itself to DefaultGlobal and remove
any relation to the actual system allocator?
—
Reply to this email directly, view it on GitHub
<#108 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGLD2YHNOJVPG7JYKGHOR3WJD2NJANCNFSM6AAAAAASFHO4YI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
It is also explicitly document as not being something you can rely on, so you really can't go off the docs here, unfortunately. |
Ah, my bad. I thought this was covered by the same feature as |
(Not saying it's necessarily a good idea, but) If we really want to change the name of a stable type, we can move the type and expose a (potentially deprecated) type alias and For a unit struct like That said, I personally think that |
You mean, even if the global allocator is explicitly set one cannot allocate via Global and then free via the underlying allocator?
Hm yeah that makes sense. We want to treat Global special for optimizations.
|
Just for the sake of considering all options, an alternative to this:
Would be to say that the "special behaviour" (eg. that we can optimize away allocations) is a behaviour imparted by the (To be clear this is completely tangential to my argument about having some way to access the default allocator) |
The special behavior comes from LLVM recognizing |
@bjorn3 That's an implementation detail though. You could in theory export the methods on the |
That would mean the methods on a static of type T are not the methods from that type T. Seems rather strange to me?
|
In fact it's not even coherent... #[global_allocator]
static S: MyAlloc = MyAlloc::new();
fn mk_alloc(s: &MyAlloc) {
s.allocate(...);
}
fn main() {
mk_alloc(&S);
}
|
on allocation elisionReplaceable allocation is a tricky subject. The way to justify allocation elision in the face of an in-language allocator definition (ignoring the other hard parts around provenance et al) is typically something along the lines of the two part:
These two rules allow the optimizer to remove and insert allocations, respectively. It is expected that the optimizer has reasonable Quality Of Implementation and will not insert extra allocation for no reason. It would be a nice property to provide that any executed allocation actually is an allocation performed in the source, but this makes code motion more difficult.1 It may also be necessary to explicitly allow shifting allocator calls around respective to other code, but I believe this can be justified by combining the two rules to elide the written allocation and insert a new allocation where desired. This is another reason why providing a guarantee of all performed allocations corresponding to an allocation in the source is difficult. On top of this, it's not necessarily sufficient to make just This is AIUI a feature that C++ supports — In current Rust, you can't replicate this behavior. Instead, you have to either make your I don't have a good solution to offer here. If CHERI has a solution for removable allocation defined within CHERI-C, looking at it for inspiration is likely to be useful. I think best resolution to the documentation issue is something along the lines of
Footnotes
|
I would like for there to additional rules arround linking multiple standalone crates together. In particular, if you link a As an alternative, saying that it must use the top-level binary's I have need of this rule for lccc in order to be able to allow linking to staticlib crates from rust final link targets, or to link together multiple staticlib crates into a single final link target written in any language, as well as to permit overriding default allocator when dynamically linking std (except maybe on windows). The former is an absolute requirement for my use cases (standard system libraries written in rust) and the latter a QoL improvement since I want to dynamically link std by default on capable targets. |
@CAD97 I suppose one option would be for struct OpaqueAllocator<T>(pub T);
impl<T: Allocator> Allocator for OpaqueAllocator<T> { ... } Then it can simply be part of the contract of this type that it may not call the underlying allocator, or may call it multiple times, allowing the compiler to optimize allocations through this type. The |
@Diggsey Having a wrapper This would work for removing allocations, but the other code motion (adding, reordering) aren't possible to describe just with a wrapper selecting if it's going to call your impl or not. Perhaps we don't need these, but it should be noted that @chorman0773 There's open discussion around a As for dynamic linking, similarly, Even if lccc wants to define |
The issue is that what I want to do is compile a system library (such as libcrypto) and then have that available to link into, including from rust code, via the standard interface expected of any libcrypto implementation (such as the one from openssl, libressl, etc.). The fact that if I do this I cannot link that version of the library to a rust crate means I do not comply with the standard definition of the library. Same for cdylib, I want to be able to produce system libraries using this type.
This is non-functional wrt. staticly linked libraries: There is no such thing as a symbol that can be linked by all objects of an archive and no other object in the same link step, at least not in ELF, COFF, or o65 objects (or OMF, a.out, xo65, and a number of others when using GNU or BSD archive format). Defining it as internal would limit it to being available only in the object file that defines it, and defining it as external or weak makes it available to all objects in the entire ld invocation. Even with 1 cgu, the allocator will be depended upon by upstream crates, so the definition must be external. The issue with cdylib is that to support linking against a dynamically linked std, all providers of the alocator symbol are defined with default visibility as external. Additionally, to support MacOS and static linking of std, the symbol is defined as weak in a linkonce COMDAT group. The default allocator is just the
Again, this is impossible on existing file formats using the standard archive for static libraries. |
If the standard interface expected of any libcrypto implementation only has it importing symbols from libc, then you simply cannot match that with a Rust crate without entirely encapsulating the use of Rust such that no Rust symbols are imported or exported. You can't just add extra imported symbols (even if they're weak symbols) and claim that it provides the exact same interface. You're making your own compiler. Make your own crate type which does what you need it to. This is far from the locale to argue about lccc trying to force existing compiler options to do things they were not designed to do. |
The standard interface is that it is usable. It's already assumed (the
My assumption was that the cdylib and staticlib options, that is the options that are designed to produce system library types, produced files usable as system libraries. I was also implicitly assuming that I can write my rust code against the standard options expected of the rust compiler, and not have to limit a generally useful crate (lc-crypto) to being "lccc only", that seems highly counterproductive to the whole enterprise. I would further expect in general to be able to link any system library into either a rust program or a program that also links in other rust code, or to be able to combine system libraries, many of which are written in rust.
No - what I'm doing here is trying to ensure I have the proper guarantees to actually implement the language wrt. the global_allocator which is up for discussion here, while working arround a problem that prevents me from writing low-level system code in a systems programming language. And that I'm not given encapsilation requirements that are impossible to implement on existing file formats. |
@chorman0773 @CAD97 you are veering widely off-topic for this issue; whether or not the allocator is shared when linking together crate type X is not what this thread is about. Could someone with moderation power please mark these posts as off-topic? |
It could be a wrapper that applies the full set of allocation magic though -- that would be a language primitive, sure, but instead of saying that the magic elision/introduction of allocations is tied to the global allocator, we could tie it to that wrapper. That doesn't seem any harder to specify? And it would make the same magic available to custom allocators. Pragmatically speaking, we'd add the appropriate LLVM attributes to calls of the functions on this magic wrapper. |
I quite like the suggestion of having |
As I understand it the canonical abi of the component model requires exposing methods that allocate within the component itself. As such any component would either need |
Cdylibs and staticlibs shouldn't leak anything about being a rust crate. That includes the global allocator. For cdylibs we indeed only export |
Please move the symbol leakage discussion to a different issue, will you? It is independent of what we guarantee for default crate types, which this issue is about. The only reason crate types show up in the OP is that I quoted the docs with context. But it's really a separate question. Looks like some people here are way too easily nerd-sniped... (I feel you, I often have the same problem. It's bad for discussions though.) Who has moderation rights here and can collapse those off-topic posts? |
Opened rust-lang/rust#104707. |
Documentation states that
and it also states that
These two statements directly contradict each other -- one says that
System
is the default everywhere, one says that that is not specified. Which one should it be? :)FWIW Miri currently considers it a bug to create an allocation with the default global allocator and then free it with
System
, matching the first bit of documentation, but contradicting the second.The text was updated successfully, but these errors were encountered: