Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Add sanitizer support (notably AddressSanitizer) to core.thread.Fiber.#152

Merged
JohanEngelen merged 1 commit intoldcfrom
sanitizer_switch_fiber
Feb 10, 2019
Merged

Add sanitizer support (notably AddressSanitizer) to core.thread.Fiber.#152
JohanEngelen merged 1 commit intoldcfrom
sanitizer_switch_fiber

Conversation

@JohanEngelen
Copy link
Copy Markdown
Member

@JohanEngelen JohanEngelen commented Jan 20, 2019

Updated to latest state:

Here are the changes that are needed to support ASan with fibers.

To enable support, the runtime must be built with version = SupportSanitizers, which is done by passing -DRT_SUPPORT_SANITIZERS=ON to cmake when building LDC. This is OFF per default. See ldc-developers/ldc#2975

The calls to __sanitizer_finish_switch_fiber and __sanitizer_start_switch_fiber pass through a layer that makes linking with AddressSanitizer library optional, such that the runtime can be used without ASan aswell with only a small overhead. This optional linking method is the same as Folly, see: facebook/folly@2ea64dd).
The same layer will be used to add other sanitizer support to the runtime (e.g. GC support).

@kinke
Copy link
Copy Markdown
Member

kinke commented Jan 20, 2019

I think it should keep being versioned. Building druntime oneself isn't rocket-science with ldc-build-runtime and should be acceptable for advanced instrumentation with according overhead.

@JohanEngelen
Copy link
Copy Markdown
Member Author

Yeah. I'll keep it versioned and I'll do the dynamic loading / weak link thing such that the resulting library can also be used without asan linked in.

@JohanEngelen
Copy link
Copy Markdown
Member Author

How's this?

I can use the same method for other sanitizer support additions needed.

else version (Windows)
{
import core.sys.windows.windows : GetModuleHandleA, GetProcAddress;
fptr = cast(typeof(fptr)) GetProcAddress(GetModuleHandleA(null), functionName);
Copy link
Copy Markdown
Member

@kinke kinke Jan 20, 2019

Choose a reason for hiding this comment

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

This requires the binary containing druntime executable to explicitly dllexport a function with matching name.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's what LLVM appears to be doing.
To be honest, I don't expect there to be much support for sanitizers on windows.

@JohanEngelen JohanEngelen force-pushed the sanitizer_switch_fiber branch 4 times, most recently from 93dfd18 to 37f9af1 Compare January 25, 2019 00:06
@JohanEngelen JohanEngelen force-pushed the sanitizer_switch_fiber branch from 37f9af1 to 2a7f21b Compare January 25, 2019 12:50
@JohanEngelen JohanEngelen changed the title [WIP] Add sanitizer support (notably AddressSanitizer) to core.thread.Fiber. Add sanitizer support (notably AddressSanitizer) to core.thread.Fiber. Jan 25, 2019
@JohanEngelen
Copy link
Copy Markdown
Member Author

Mergable?

// Note that the fake stack mechanism is disabled during fiber switch, so if a
// signal callback runs during the switch, it will not benefit from the stack
// use-after-return detection.
void __sanitizer_start_switch_fiber(void** fake_stack_save,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

only thing i think is missing is a code example.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is such specific stuff that a code example would be the size of a book :P Here I copied the documentation that is in the LLVM interface file, which is kindof the only documentation there is.

@JohanEngelen JohanEngelen merged commit 1971de4 into ldc Feb 10, 2019
@JohanEngelen JohanEngelen deleted the sanitizer_switch_fiber branch February 10, 2019 10:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants