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

Experiment: add Rust static lib linked into krabcake tool #1

Merged
merged 5 commits into from Mar 16, 2023

Conversation

pnkfelix
Copy link
Owner

Experiment: add Rust static lib that is linked into krabcake tool to enable krabcake tool in Rust itself.

@pnkfelix
Copy link
Owner Author

(This currently avoids use of CStr in the demo tool code, because that causes linkage issues that I have not managed to diagnose yet.)

@pnkfelix
Copy link
Owner Author

(one thing that is currently broken, or at least non-optimal: the Rust static lib is being built in the target/ of its source directory (rs_hello/target). That should probably be adjusted to a different target directory that is aligned with whatever autotools does.

It is also generating a Cargo.lock after being built, but given the nature of this as a static lib linked into a valgrind tool binary, I probably should check in the Cargo.lock file...

Copy link
Contributor

@bryangarza bryangarza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess once you add the code to show the Rust code was successfully called, we can merge :)

@@ -13,18 +33,25 @@ endif

KRABCAKE_SOURCES_COMMON = kc_main.c

# RUST_EXTRA_LD_FLAGS = -ldl -lrt -lpthread -lgcc_s -lc -lm -lrt -lutil
# RUST_EXTRA_LD_FLAGS = -ldl -lrt -lpthread -lc -lm -lrt -lutil
RUST_EXTRA_LD_FLAGS =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this + its usage below is currently a no-op right?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's right; this is a relic from when I was trying to figure out some linker stuff. (I never really resolved it to my satisfaction, though at least now I have some theories as to what's going on...)

@bryangarza
Copy link
Contributor

given the nature of this as a static lib linked into a valgrind tool binary, I probably should check in the Cargo.lock file...

Agreed

@pnkfelix pnkfelix merged commit 19d6c36 into krabcake-vg Mar 16, 2023
@pnkfelix
Copy link
Owner Author

(This currently avoids use of CStr in the demo tool code, because that causes linkage issues that I have not managed to diagnose yet.)

update: I think the aforementioned linkage issues are due to unresolved symbols that are injected when you have potential panicking paths in the code of the static library.

In particular, I observed really wonky behavior where, depending on how complex my code was, changes to arithmetic operations would cause linker errors, and I eventually realized it was due to array access a[i] where, once i was computed by something complicated, LLVM could no longer statically prove away the bound checks, and panic code had to be included, which caused the linker to fail.

This is not at all my long- or even medium-term intent here. My intent is that the Rust code here should be allowed to panic, and panics should map into Valgrind's internal abort machinery that already exists. But for the short-term, I'm working around the panicking by ensuring my static lib never actually panics (in a manner that LLVM can prove at compile time, sufficiently so to eliminate those otherwise unresolved symbols).

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.

None yet

2 participants