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 whole-archive feature to libafl_targets #1544

Merged
merged 10 commits into from
Nov 20, 2023
Merged

Conversation

addisoncrump
Copy link
Collaborator

@addisoncrump addisoncrump commented Sep 22, 2023

This addresses a very annoying issue with libafl_libfuzzer where linkage drops symbols from libafl_targets. It does this by forcing the rustc linkage step to add all object files from libafl_targets, regardless if they are directly referenced or not. Confirmed with GDB, this does cause __sanitizer_weak_*cmp to be linked. This should address the issues we see with performance in e.g. PROJ4: https://www.fuzzbench.com/reports/experimental/2023-09-21-libafl/index.html

This is feature-gated as it requires nightly, and the unstable feature cannot be added via rustversion.

@addisoncrump
Copy link
Collaborator Author

Fixes #1540

@tokatoka
Copy link
Member

@andreafioraldi is this good to you?
I guess we might have a problem like multiple definition

@addisoncrump
Copy link
Collaborator Author

Fix is somehow incomplete 👻 We are still having some weak symbols not being overridden in fuzzbench. Testing locally, it works, but in Fuzzbench, it doesn't!

@addisoncrump
Copy link
Collaborator Author

addisoncrump commented Sep 23, 2023

^ This now adds the interceptors explicitly as well.

@domenukk
Copy link
Member

error: code should be clang-formatted :P

@addisoncrump
Copy link
Collaborator Author

error: code should be clang-formatted :P

Dammit 😂

@addisoncrump
Copy link
Collaborator Author

I've confirmed that this resolves the linkage issues in https://github.com/AFLplusplus/LibAFL/tree/libfuzzer-best

@domenukk
Copy link
Member

  error[E0554]: `#![feature]` may not be used on the stable release channel
   --> /home/runner/work/LibAFL/LibAFL/libafl_targets/src/lib.rs:2:40
    |
  2 | #![cfg_attr(feature = "whole_archive", feature(packed_bundled_libs))]
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Doesn't seem to work on stable then?

@addisoncrump
Copy link
Collaborator Author

Yeah, sadly there's no way to do this with stable.

@domenukk
Copy link
Member

Merge this then? And mark libfuzzer as nightly?

@addisoncrump
Copy link
Collaborator Author

Need to fix CI but I'm not sure what the problem is?

@tokatoka
Copy link
Member

maybe

whole_archive = ["nightly"]

to disable it on stable?

@addisoncrump addisoncrump force-pushed the targets-whole-archive branch 2 times, most recently from 9e5c847 to 9c25042 Compare September 29, 2023 15:36
@andreafioraldi
Copy link
Member

For reference, years ago I solved this with a dynamic list in AFL++ https://github.com/AFLplusplus/AFLplusplus/blob/stable/dynamic_list.txt but ofc you need a compiler wrapper, not our case for rustc I guess

@domenukk
Copy link
Member

domenukk commented Nov 3, 2023

So... @andreafioraldi merge or no merge?

@addisoncrump addisoncrump force-pushed the targets-whole-archive branch from 422165f to 4d8b443 Compare November 7, 2023 19:26
@addisoncrump
Copy link
Collaborator Author

Rebased.

@domenukk
Copy link
Member

So ready to merge?

@addisoncrump
Copy link
Collaborator Author

We shall see 🙂

@domenukk
Copy link
Member

We don't want a dynamic list then?

@domenukk domenukk merged commit a278357 into main Nov 20, 2023
17 checks passed
@domenukk domenukk deleted the targets-whole-archive branch November 20, 2023 09:38
tokatoka pushed a commit that referenced this pull request Nov 21, 2023
* maybe fix linkage?

* fix hack CI

* interceptors

* do not call strstr and friends

* format

* whoops

* enforce nightly; fixup linkage by featuring interceptors

* skip libafl_libfuzzer in stable cargo hack check

* oops

* packed_bundled_libs is stablised
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.

4 participants