Skip to content
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

Add suffix to mbedtls_log to avoid link name conflict error #245

Closed
wants to merge 1 commit into from

Conversation

Taowyoo
Copy link
Collaborator

@Taowyoo Taowyoo commented Apr 13, 2023

Summary of Changes in this PR

This PR addresses the link name conflict error that occurs when using different versions of the mbedtls crate in a single project, by adopting the approach used in PR #234.

Rationale Behind the Approach

During testing, it was discovered that simply renaming mbedtls_log resolves the linking error. Therefore, the export_name attribute is employed here, as done in PR #243.

Upon further investigation, it was found that the linker error stems from multiple definitions of mbedtls_log in the Rust code. The C compiler attempts to locate and link the mbedtls_log function implemented in Rust to the C library, but it encounters duplicate instances of the function with the same name.

It's important to note that the linking search direction is from C to Rust, so naming on the C side is not a concern. Functions named mbedtls_printf in multiple rust_printf.c files are similar to other functions in the C mbedtls code and do not cause link name conflict errors.

@jethrogb
Copy link
Member

I'm not convinced this actually works, or at least you fail to explain how this works. There are certainly multiple global symbols named mbedtls_printf in the compiled objects, so this should result in symbol conflicts. Even if it "works", this basically means one mbedtls_printf implementation is picked at random, which would result in surprising behavior at best.

Functions named mbedtls_printf in multiple rust_printf.c files are similar to other functions in the C mbedtls code

This doesn't make sense to me

@Taowyoo
Copy link
Collaborator Author

Taowyoo commented Apr 13, 2023

I got your point. You're correct

I had wrong understanding here

Functions named mbedtls_printf in multiple rust_printf.c files are similar to other functions in the C mbedtls code.

Let me rephrase what happened here:

First, compiler will compile rust_printf.c to rust_printf.o where: mbedtls_log is Undefined symbol and mbedtls_printf is local defined symbol

# part output of `nm rust_printf.o`
                 U mbedtls_log
0000000000000000 T mbedtls_printf

So compiler try to find mbedtls_log in rust compiled objects and here is where original error occurs.
And by changing name of mbedtls_log , this error is resolved by this PR.

But there indeed are multiple mbedtls_printf in multiple rust_printf.o. By inspecting to the compiled libmbedtls-abc.rlib files, the the content of rust_printf.o is being placed to the rlib file correspondingly because mbedtls rlib file is compiled separately by the build.rs script.

So I guess the reason why compiler do not throw conflict error here is: it does not detect the usage/re-export on mbedtls_printf, but this is not guaranteed in runtime and the behavior on runtime will become undefined.

In short, the approach in this PR does not solve the problem correctly.

I will close this PR, and could you please review the approve the #244 if it's ok?

@Taowyoo Taowyoo closed this Apr 13, 2023
@Taowyoo Taowyoo deleted the yx/link-fix branch April 13, 2023 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants