Make a disable-jemalloc build work #43589
Conversation
|
@bors: r+ |
|
📌 Commit 947cfb3 has been approved by |
|
Oh, I see, I copied the |
|
⌛ Testing commit 947cfb3d003b41a64c5ee754dbf518a53d54d4b4 with merge 899c27a18bcf2e6044af87d64afa5865dada3d17... |
|
💔 Test failed - status-appveyor |
|
|
|
Hum, I'll have to have a think about this, looks like it worked fine on all the osx and linux variants, but not on windows and I'm not 100% sure why - in theory Thoughts from onlookers appreciated. |
|
I'm no Windows expert, but I think it's more particular about resolving symbols only from directly-specified libraries. I'm not sure what that implies in this case. It looks like the msvc builds always disable jemalloc anyway, so maybe this workaround can just be skipped there. Appveyor didn't go far enough to see if windows-gnu has a similar link error. |
|
AFAICS none of the Windows targets change |
|
@bors retry (Queue is empty, and let's see if windows-gnu have any messages.) |
|
⌛ Testing commit 947cfb3d003b41a64c5ee754dbf518a53d54d4b4 with merge ed77d9fbc01ca9ded17300f6256558c0ea891d88... |
|
💔 Test failed - status-appveyor |
aidanhs
added
the
S-waiting-on-author
|
Errors like this are typically indicative of a missing "dllexport" attribute of some shape/form when the object is compiled. We have to apply dllexport manually and IIRC we don't do this correctly for allocator attributes. I think, though, the fix may not be too too hard if you'd like to pioneer it here. The boolean on whether to do this is here, [here] is where allocator modules are trans'd, and I think here is roughly the right location for where to say "this symbol is a C symbol," e.g. needs a dllexport. |
|
Ok, I've had a look at this and it's a bit humbling to realise how little I know about linking on Windows! Fortunately I have access to Windows so I'll play around this weekend and I think I have a route forward - looking at the export details, I think I might be able to just add extern to the allocator symbols during trans, rather than special casing them? We shall see anyway. |
|
@aidanhs ah yeah that sounds like a fine strategy to me as well! |
aidanhs
force-pushed
the
aphs-fix-system-malloc
branch
5 times, most recently
from
a2b1347
to
62231b1
Compare
aidanhs
force-pushed
the
aphs-fix-system-malloc
branch
from
62231b1
to
56a0753
Compare
|
This is the best I've come up with after wrestling with it for a bit. Note that because the stage0 snapshot has the broken behavior on msvc, I've had to cfg the global allocator stuff until the next snapshot update (which also means a temporary tidy check change). In terms of approach:
This gets past building stage0 term locally on Windows (obviously, because the new code is cfged out) but I've been unable to test further than that because building LLVM on Windows is being problematic. In particular, I'm not sure I've got the correct number of underscores. |
|
@bors: r+ That all sounds good to me! I think you're right about the "Rust level" as well instead of "C" |
|
📌 Commit 56a0753 has been approved by |
Make a disable-jemalloc build work Fixes #43510. I've tested this up to building a stage1 compiler. r? @alexcrichton cc @cuviper @vorner @cuviper your fix was almost correct, you just had a stray `!` in there which caused the second error you saw.
|
☀️ Test successful - status-appveyor, status-travis |
Fixes #43510. I've tested this up to building a stage1 compiler.
r? @alexcrichton
cc @cuviper @vorner
@cuviper your fix was almost correct, you just had a stray
!in there which caused the second error you saw.