Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

je_malloc_usable_size not found #9167

Closed
aarlt opened this issue Jul 18, 2018 · 18 comments
Closed

je_malloc_usable_size not found #9167

aarlt opened this issue Jul 18, 2018 · 18 comments
Labels
F2-bug 🐞 The client fails to follow expected behavior. M0-build 🏗 Building and build system. P5-sometimesoon 🌲 Issue is worth doing soon.
Milestone

Comments

@aarlt
Copy link
Contributor

aarlt commented Jul 18, 2018

I'm running:

  • Which Parity version?: @ 3c27587
  • Which operating system?: MacOS 10.13.6
  • How installed?: from source

I having an issue building parity-clib. I get the following linker error, because of undefined symbols:

...
          Undefined symbols for architecture x86_64:
            "_je_malloc_usable_size", referenced from:
                heapsize::heap_size_of_impl::hb1048887d365f818 in libheapsize-3daab403b63f3097.rlib(heapsize-3daab403b63f3097.heapsize3.rcgu.o)
          ld: symbol(s) not found for architecture x86_64
          clang: error: linker command failed with exit code 1 (use -v to see invocation)

error: aborting due to previous error

error: Could not compile `parity-clib`.

To learn more, run the command again with --verbose.

I used cargo build -p parity-clib to build parity-clib.

@aarlt aarlt changed the title parity-clib: not able build "cdylib" on macos 10.13.6 parity-clib: not able build on macos 10.13.6 Jul 18, 2018
@niklasad1
Copy link
Collaborator

niklasad1 commented Jul 19, 2018

Hey,

I think this is an issue with the heapsize crate crate because it assumes je_malloc exists and in debug builds clang disables je_malloc!

For further description see: servo/heapsize#80

I don't have any Mac machine to confirm this but may just try building it in release-mode:

$ cargo build -p parity-clib --release

@tomaka any thoughts on this? I guess this will also impact iOS?

@niklasad1 niklasad1 added the F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. label Jul 19, 2018
@tomaka
Copy link
Contributor

tomaka commented Jul 19, 2018

Shouldn't this also happen when compiling the parity binary?

@Tbaut Tbaut added the M0-build 🏗 Building and build system. label Jul 19, 2018
@Tbaut Tbaut added this to the 2.1 milestone Jul 19, 2018
@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

@niklasad1 unfortunately cargo build -p parity-clib --release is also not working.

@tomaka a normal parity build with cargo build --release --features final works - no linking problems.

@niklasad1
Copy link
Collaborator

I think this only applies dynamically linked libraries i.e, with rust terms dylib or cdylib. If I have understood this correctly heapsize assumes that symbol je_malloc_usable_size exist in the binary when it is statically linked it does but when it dynamically linked it might not!

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

@niklasad1 yes, thats true. the static library is buildable, but im not very sure that this is usable - the symbol should be missing there too, am I right?

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

Hmm.. after looking into https://github.com/servo/heapsize/blob/master/src/lib.rs it seems that there is an dependency to the jemalloc library. After just running brew install jemalloc to install that library the problem still persists. After checking the exported symbols of that lib, it looks like that the symbol _je_malloc_usable_size doesn't exist there, but there is a symbol named _malloc_usable_size.

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

I found a workaround for this, but I'm not sure whether this problem will also exist on ios / android. Those are the first rust lines I wrote :D

However, I added the following function to parity-ethereum/parity-clib/src/lib.rs - this will just be a proxy function that will call the correct function exported by jemalloc on macos. If the code is correct I would be also able to create a pull request.

 #[no_mangle]
 #[cfg(target_os = "macos")]
 pub extern fn je_malloc_usable_size(_ptr: *const c_void) -> usize {
     unsafe {
       extern "C" {
         fn malloc_usable_size(ptr: *const c_void) -> usize;
       }
       return malloc_usable_size(_ptr);
     }
 }

I was able to compile & run the example with g++ main.cpp libparity.a -framework Security -framework IOKit -framework CoreFoundation -ljemalloc. I'm not sure whether the linking against jemalloc need to be manually added into your buildsystem.

@tomaka
Copy link
Contributor

tomaka commented Jul 19, 2018

This is an extremely hazardous work-around. The code can break every time we upgrade Rust. Also it would need to be enabled only in debug if I understand correctly.

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

@tomaka yeah that may be true. I guess thats really an issue within https://github.com/servo/heapsize. My understanding - I'm not a rust developer - https://github.com/servo/heapsize/blob/master/src/lib.rs#L44 seem to change the symbol name for macos, ios and android to be what is not correct - at least for macos. The symbol name need to be malloc_usable_size - as it would normally be from my understanding - but it will be changed to be je_malloc_usable_size.

What I don't understand: why is the stuff working for the normal parity-ethereum build. We should somehow see the same issues.

The code can break every time we upgrade Rust. Also it would need to be enabled only in debug if I understand correctly.

I don't understand why this may break every time if Rust is upgraded - its just an issue with the exported symbols of jemalloc. From my point of view the redefinition of the symbol name to je_malloc_usable_size is the real issue. I also don't get why this may only be needed in debug.

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

I just created an issue here: servo/heapsize#97

@tomaka
Copy link
Contributor

tomaka commented Jul 19, 2018

Unfortunately I think they have been having a pretty long discussion about this already, and the conclusion is that heapsize is wrong by design and that something else should be used.

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

Ah ok.. sounds not good..

However, I think it might be a good idea to add cargo build -p parity-clib --release to your CI. I guess youre also doing there ios and android builds - so at least there seem to be an easy way to find out whether this is not only an macos issue.

@aarlt
Copy link
Contributor Author

aarlt commented Jul 19, 2018

@tomaka please see servo/heapsize#97 (comment)

@aarlt
Copy link
Contributor Author

aarlt commented Jul 20, 2018

@tomaka @niklasad1 I still don't really understand why there is no linking issue with the normal parity build - any ideas?

@aarlt
Copy link
Contributor Author

aarlt commented Jul 24, 2018

@tomaka @niklasad1 I was able to understand what is happening here, but due to lack of rust & parity-ethereum codebase knowledge I will probably not yet be able to create fix for this issue.

@tomaka you where right, my work-around is extremely hazardous.

I just read https://doc.rust-lang.org/1.9.0/book/custom-allocators.html. rust is shipped with two default allocators - alloc_system and alloc_jemalloc. alloc_jemalloc seem not to work nicely on all systems, thats why it is sometimes just not used. But basically, if alloc_jemalloc is available on a specific platform, it will be used as a default allocator for binaries. In contrast to that, if a static- or dynamic library is build - it will use alloc_system. As I understood alloc_system will just use the standard allocation API like malloc and free.

That means at least for macos it looks like that alloc_jemalloc is available, thats why the default parity binary can be build. As described, for libraries it will use the alloc_system, where of course jemalloc will not be linked against. So my work-arround will basically mix different allocators - that is for sure quite dangerous.

I think the best solution is probably really to create a custom crate for heapsize, where somehow https://github.com/servo/heapsize/blob/master/src/lib.rs#L44 will need to be changed in a way that it will not redefine the symbol name - if compiled as a library.

@tomaka tomaka changed the title parity-clib: not able build on macos 10.13.6 je_malloc_usable_size not found Aug 3, 2018
@tomaka
Copy link
Contributor

tomaka commented Aug 3, 2018

With Rust 1.28, adding je_malloc_usable_size ourselves could actually be a solution if we use the system allocator that is now stable.

@5chdn 5chdn modified the milestones: 2.1, 2.2 Sep 11, 2018
@5chdn 5chdn modified the milestones: 2.2, 2.3 Oct 29, 2018
@5chdn 5chdn added the F2-bug 🐞 The client fails to follow expected behavior. label Dec 5, 2018
@5chdn 5chdn added P5-sometimesoon 🌲 Issue is worth doing soon. and removed F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. labels Dec 5, 2018
@5chdn 5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019
@gguoss
Copy link
Contributor

gguoss commented Jan 28, 2019

parity version: 4fec2f2
rustc 1.33.0-nightly
MacOS
Build parity error.

gguoss referenced this issue Jan 29, 2019
* Switch to non prefixed malloc_size_of on macos

* Fix

* Testing darwin build

* Fix import

* conflict

* switch heapsize deps commit

* switch heapsize commit

* Rename branch

* Restore gitlab ci to origin

* test for mac

* mac tests?

* Switch of macos CI tests.
@gguoss
Copy link
Contributor

gguoss commented Jan 29, 2019

Build Ok , this issue should close.

@5chdn 5chdn closed this as completed Jan 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F2-bug 🐞 The client fails to follow expected behavior. M0-build 🏗 Building and build system. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

6 participants