RFC: target_feature #2045

gnzlbg · 2017-06-26T16:59:26Z

This is an RFC for cfg!(target_feature) and #[target_feature].

dlight · 2017-06-26T20:15:21Z

On most widely used platforms memory unsafety can never occur because the CPU will raise an illegal instruction exception (SIGILL) crashing the process.

Question: isn't triggering an illegal instruction a form of memory unsafety? Or other things like handling the FPU state.

eddyb · 2017-06-26T21:13:57Z

@dlight No, crashes are safe. Deterministically causing a SIGSEGV is perfectly fine, but it's usually a sign of an access that could have ended up in valid memory, causing corruption instead.

nagisa · 2017-06-27T04:56:09Z

Similarly with instructions. Unless an instruction is defined as invalid (e.d. ud2), it's encoding could be used in some future processors for e.g. rocket launching. It is also possible that there are certain already-released SKUs which use encoding of the invalid instruction for some secret instruction or decode to any other valid instruction.

gnzlbg · 2017-06-27T11:00:42Z

I asked @chandlerc on IRC about the "safety" of target_feature. He:

explained that once you emit an instruction for architecture X LLVM and everything below (in particular the assembler) gets to assume that the code is executed on that actual architecture, if the code is reached in a different architecture the behavior is undefined.
recommends not relying on the behavior of the decoder in the face of unsupported encodings since, at least for Intel CPUs (and probably for others as well), this is not a reliable part of the decoder.

and mentioned two examples:

Example 1: the assembler could be in a mode that aligns every instruction with nops, and it could use any nop encoding it wants, including one that will jump to an arbitrary address on some processor. The assembler emits this code assuming a particular architecture, but if the code runs in a different one the program exhibits undefined behavior before the illegal instruction is actually reached (and hence, before a SIGILL exception can be raised).
Example 2: the padding used in some contexts between symbols in the text section is usually selected to be single byte trap instructions (e.g. int3 which has a magical one byte encoding of 0xCC). The point is to help avoid random exploits that attempt to make the control flow land in the middle of some instruction, because even if it isn't a valid instruction, they don't want to rely on SIGILL handling, so instead, all bytes decode to a valid trap, so that every address is safe.

@chandlerc Did I summarize your points correctly?

So IIUC calling a function marked with the #[target_feature = "..."] is always unsafe, since if it's called on a different architecture it might introduce undefined behavior in LLVM, the assembler, CPU, ... which might lead to memory unsafety. It is the responsibility of the user, or whoever wraps it on a safe function, to make sure that it is only called on architectures that supports the feature.

EDIT:

I need to update the RFC with @pedrocr suggestion about run-time feature detection.
I need to update the RFC with this information and also add an example of how this can be made safe in practice:

// Raw intrinsic function: dispatches to LLVM directly. 
// Calling this on an unsupported target is undefined behavior. 
extern "C" { fn raw_intrinsic_function(f64, f64) -> f64; }

// Software emulation of the intrinsic, 
// works on all architectures.
fn software_emulation_of_raw_intrinsic_function(f64, f64) -> f64;

// Safe zero-cost wrapper over the intrinsic
// (i.e. can be inlined -- whether this is a good idea 
// is an unresolved question).
fn my_intrinsic(a: f64, b: f64) -> f64 {
  #[cfg!(target_feature = "some_feature")] {
    // If "some_feature" is enabled, it is safe to call the 
    // raw intrinsic function
    unsafe { raw_intrinsic_function(a, b) }
  }
  #[not(cfg!(target_feature = "some_feature"))] {
     // if "some_feature" is disabled calling 
     // the raw intrinsic function is undefined behavior (per LLVM), 
     // we call the safe software emulation of the intrinsic:
     software_emulation_of_raw_intrinsic_function(a, b)
  }
}

// Provides run-time dispatch to the best implementation:
// ifunc! is a procedural macro that generates copies 
/// of `my_intrinsic` with different target features,
// does run-time feature detection on binary initialization, 
// and sets my_intrinsic_rt  to dispatch to the appropriate 
// implementation:
static my_intrinsic_rt = ifunc!(my_intrinsic, ["default_target", "some_feature"]);

// This procedural macro expands to the following:

// Copies the tokens of `my_intrinsic` for each feature
// into a different function and annotates it with
// #[target_feature]. Due to this, calling this function
// is unsafe, since doing so on targets without the feature
// introduces undefined behavior.
#[target_feature = "some_feature"]
unsafe fn my_intrinsic_some_feature_wrapper(a: f64, b: f64) 
{
  #[cfg!(target_feature = "some_feature")] {
    // this branch will always be taken because 
    // `#[target_feautre = "..."]` defines `cfg!(target_feature = "...") == true`
    unsafe { raw_intrinsic_function(a, b) }
  }
  #[not(cfg!(target_feature = "some_feature"))] {
     // dead code: this branch will never be taken
     software_emulation_of_raw_intrinsic_function(a, b)
  }
}

// This function does run-time feature detection to return a function pointer
// to the "best" implementation (run-time feature detection is not part of this RFC):
fn initialize_my_intrinsic_fn_ptr() {
  if std::cpuid::has_feature("some_feature") -> typeof(my_intrinsic) {
    // Since we have made sure that the target the binary is running on 
    // has the feature, calling my_intrinsic_some_feature_wrapper is safe: 
    unsafe { 
        my_intrinsic_some_feature_wrapper 
        /* do we need a cast here?: as typeof(my_intrinsic) */ 
    }
  } else { 
    // because we passed "default_target" to ifunc we fall back 
    // to the safe implementation. We could otherwise return a 
    // function pointer to a stub function: 
    // fn my_intrinsic_fallback(f64,f64) -> f64 { panic!("nice error message") }
    my_intrinsic
  }
}

Note how here one can still run into undefined behavior by using -C --target-feature="+some_feature" to change the default target feature set of the whole binary, if one then runs the binary on an arch that doesn't support the feature. But this is defined by the RFC to be undefined behavior anyways, and the undefined behavior is then everywhere (every instruction can invoke it), not only unsafe code. This UB allows rustc to, on a best-effort basis, check the global features on binary initialization, but we need to be careful that this code is generated with the default feature set to avoid UB in it.

gnzlbg · 2017-06-27T13:24:15Z

So I've updated the RFC with the discussion of undefined behavior and run-time feature detection and these issues as unresolved questions.

gnzlbg · 2017-06-27T18:25:14Z

Updated the RFC with the interaction between #[target_feature], inlining, and #[always_inline].

Still need to go through the whole RFC, update the motivation for making #[target_feature] unsafe, and remove the discussion based on this uncertainty.

gnzlbg · 2017-06-27T20:23:21Z

So I have resolved the unsafety issue and pushed a much smaller and certain version of the RFC.

BurntSushi · 2017-06-27T20:30:30Z

text/0000-target-feature.md

+`#[target_feature]` requires run-time feature detection is required.
+
+How to do this is an unresolved question of this RFC that must be resolved
+before stabilization. This section does not propose a solution, but explores the


Why does run-time feature detection need to be resolved before this RFC is stabilized? It doesn't seem required to me?

What must be resolved is whether we need to stabilize run-time detection alongside this RFC or not. I need to phrase this differently, it's a bit meta.

Oh, I see. I think we'd need a very strong reason to couple this with run-time detection as well. I'd much rather leave run-time detection to the Wild West (i.e., crates.io) until it's more thoroughly understood.

@BurntSushi I've rewritten this to make it more clear, let me know if it helps or does not help.

BurntSushi · 2017-06-27T20:32:59Z

text/0000-target-feature.md

+  API for run-time feature detection?
+
+- Can we lift the restriction on `target_feature="-feature"` to provide
+  substractive features?


What restriction is this referring to?

Currently, the RFC only allows #[target_feature = "+..."], that is, adding features to the default features set. This is an attempt to avoid non-sensical code like this example that you showed.

Since I haven't given #[target_feature = "-..."] much thought I've listed it as an unresolved question in case somebody wants to give it a try.

Oh I see. I seem to recall someone giving me a use case for "-...", but I can't recall it off the top of my head. However, if we're making #[target_feature] unsafe anyway, then its quirks seem less risky?

alexcrichton · 2017-09-25T14:20:22Z

Alright I've now merged this RFC, thanks again @gnzlbg for it!

Tracking issue

hsivonen · 2017-11-06T18:11:37Z

@est31 :

@lambda I had a very similar view as you, until I realized that allowing #[target_feature] functions calling each other without unsafe is not really useful.

Why not?

Suppose I have code that's compiled generally for x86_64, which includes SSE2. Let's call this SSE2 code. Then for some functionality, I have a faster SSE4 implementation that get conditionally called at run-time. Let's call this SSE4 code.

All the functions that are part of SSE4 code need to use #[target_feature] to tell the compiler that it is allowed to generate SSE4 instructions.

AFAICT, the part that's unsafe is the call from SSE2 code into SSE4 code. It would devalue the safe/unsafe distinction of Rust for the entirety of SSE4 code be marked unsafe.

gnzlbg · 2017-11-06T18:37:16Z

It would devalue the safe/unsafe distinction of Rust for the entirety of SSE4 code be marked unsafe.

Let me check IIUC. You prefer this:

#[target_feature = "sse4.1"]
unsafe fn foo() {
  safe_bar();
  unsafe { sse41_intrinsic() }
}

to this:

#[target_feature = "sse4.1"]
unsafe fn foo() -> u32x4 {
    let a = bar();
    sse41_intrinsic(a)  // no unsafe block required
}

Is that correct?

hsivonen · 2017-11-06T18:48:23Z

I prefer

#[target_feature = "sse4.1"]
fn sse41_intrinsic() {
  // ...
}

#[target_feature = "sse4.1"]
unsafe fn foo() {
  bar();
  sse41_intrinsic();
}

#[target_feature = "sse4.1"]
fn bar() {
  sse41_intrinsic();
}

fn baz() {
  unsafe { foo(); }
}

over

#[target_feature = "sse4.1"]
unsafe fn sse41_intrinsic() {
  // ...
}

#[target_feature = "sse4.1"]
unsafe fn foo() {
  bar();
  sse41_intrinsic();
}

#[target_feature = "sse4.1"]
unsafe fn bar() {
  sse41_intrinsic();
}

fn baz() {
  unsafe { foo(); }
}

That is, sse41_intrinsic() and bar() shouldn't need to be unsafe, because their callers are #[target_feature = "sse4.1"]. baz() should not be allowed to call bar() or sse41_intrinsic(), because such calls would constitute calling a function with target_features that the caller doesn't have.

gnzlbg · 2017-11-06T18:52:55Z

How would you call sse41_intrinsic from baz in your preferred version?

because their callers are #[target_feature = "sse4.1"]

So whether unsafe fn is required depends on where the function is called?

hsivonen · 2017-11-06T19:44:07Z

How would you call sse41_intrinsic from baz in your preferred version?

because their callers are #[target_feature = "sse4.1"]

So whether unsafe fn is required depends on where the function is called?

I wrote unsafe fn for the callee that needed unsafe to call, but as your questions highlight, it seems that that's a bad idea. After all, there's nothing unsafe about foo() when called from a function that has #[target_feature = "sse4.1"].

So I revise my preference to drop unsafe from the callee, because the unsafety isn't a property of the callee alone but a property of the caller-callee pair:

#[target_feature = "sse4.1"]
fn sse41_intrinsic() {
  // ...
}

#[target_feature = "sse4.1"]
fn foo() {
  bar();
  sse41_intrinsic();
}

#[target_feature = "sse4.1"]
fn bar() {
  sse41_intrinsic();
}

fn baz() {
  unsafe { foo(); }
  unsafe { sse41_intrinsic(); }
}

gnzlbg · 2017-11-06T19:47:10Z

That makes more sense.

The definition of unsafe fn is: unsafe to call.

Why do you need unsafe in baz to call foo if foo is safe to call?

For foo to be unsafe to call it would need to be unsafe fn.

hsivonen · 2017-11-06T20:24:07Z

Why do you need unsafe in baz to call foo if foo is safe to call?

Because foo is unsafe to call from baz.

For foo to be unsafe to call it would need to be unsafe fn.

I guess the key insight here is that the nature of [#target_feature] is such that with unsafety arising from [#target_feature], unsafety is property of the call taking into account the both the caller and the callee and isn't a property of the callee alone.

Therefore, in order to avoid everything becoming unsafe and unsafe/safe distinction getting less useful as a result, the rule that only a callee marked unsafe can cause the requirement for an unsafe block to be used to call it be relaxed: In addition, an unsafe block is needed to call a function not marked unsafe if the callee has target_features that the caller doesn't have.

(Alternatively, there could be a block with a keyword other than unsafe for marking calls where the callee has target_features that the caller doesn't have, but so far, Rust has used unsafe for all kinds of unsafety instead of making the programmer specify what kind of unsafety the programmer intends to attempt.)

gnzlbg · 2017-11-06T20:58:33Z

What you propose sounds in general like a good idea, I encourage you to write an RFC that explores that and please ping us here once you have a draft!

As mentioned in the RFC, removing the need for unsafe in some cases is a backward compatible change, so depending how such an RFC goes, what you want might significantly improve the experience of using the bare Rust intrinsics.

Some thoughts:

#[target_feature]s would need to be part of a function ABI/type signature/metadata/... for this to properly work across crates.
a "+sse4.2" function would not be able to safely call a "+sse3" function (it would need to be "+sse4.2,+sse3" instead). So even if you "just" do this, your code will probably still have a lot of unnecessary unsafe. Removing those would require introducing feature hierarchies (or similar).

parched · 2017-11-06T21:26:41Z

I also like this proposal and think it should definitely be implemented at some stage (it bugs me that there are AArch64 NEON intrinsics in stdsimd that require unsafe, yet there are no AArch64 builtin targets in rustc without NEON enabled by default). Though, as @gnzlbg mentions, it is backward compatible so no hurry to do it now.

#[target_feature]s would need to be part of a function ABI/type signature/metadata/... for this to properly work across crates.

That's required now anyway unless cross crate calls are always going to use the lowest common denominator calling convention.

This also won't be able to be applied where the target features are unknown e.g., dynamic dispatch but that's not a big drawback. It just means functions with #[target_feature] will only be allowed to coerce into unsafe function pointers and trait methods with #[target_feature] will always have to be unsafe.

RalfJung · 2017-11-07T09:51:21Z

So I revise my preference to drop unsafe from the callee, because the unsafety isn't a property of the callee alone but a property of the caller-callee pair:

However, that is the normal case. Most if not all unsafe functions are safe to call in certain ways. In contrast, a function being "safe" means that it is safe to call in any well-typed way, by anyone.

So, I think it makes a lot of sense that a function which implicitly makes assumptions about the instruction set of the CPU should be unsafe. This function is not safe to call anytime, anywhere. Rather, similar to get_mut_unchecked, it requires the caller to do some checking before being called.

gnzlbg · 2017-11-07T10:02:04Z

@RalfJung I think we could phrase #[target_feature] fn as sugar for #[target_feature] unsafe fn but with two twists:

to call any unsafe code from those functions, the user would need to write an explicit unsafe block,
this unsafe block would not be necessary for other #[target_feature] functions that the compiler can prove are safe to call from that context (and if the compiler cannot prove it, then the user would need to explicitly write unsafe).

So the following code:

#[target_feature = "+sse4.2"] fn foo_sse42();
#[target_feature = "+avx"] fn bar_avx();
unsafe baz();

#[target_feature = "+sse4.2"]
fn meow() {
    foo_sse42(); // OK: compiler can prove this safe
    bar_avx(); // ERROR: unsafe fn
    baz(); // ERROR: unsafe fn
}

could desugar to:

#[target_feature = "+sse4.2"] unsafe(target_feature = "+sse4.2") fn foo_sse42();
#[target_feature = "+avx"] unsafe(target_feature = "+avx") fn bar_avx();
unsafe baz();

#[target_feature = "+sse4.2"]
unsafe(target_feature = "+sse4.2") fn meow() {
    safe(target_feature = "+sse4.2") {  
        // unsafe(target_feature = "+sse4.2") functions are safe to call
        foo_sse42(); // OK:
        bar_avx(); // ERROR: unsafe to call
        baz(); // ERROR: unsafe to call
    }
    unsafe {
        bar_avx(); // OK
        baz(); // OK
    } 
}

where I have introduced unsafe(...) to track unsafety conditions and safe(...) to denote when these conditions are met.

Expanding on this, currently, when the user does some checking, unsafe is still required:

if cfg_feature_enabled("avx") {
    // here AVX is enabled at run-time
    unsafe { foo_avx() }; // but foo_avx is unsafe to call
}

something like this (handwaiving) could allow us to remove the need for the unsafe block there if we can make the cfg_feature_enabled("avx") macro open a safe(target_feature = "+avx") block when the branch is taken.

hsivonen · 2017-11-07T11:04:36Z

In contrast, a function being "safe" means that it is safe to call in any well-typed way, by anyone.

The "well-typed" bit is the key. In order to avoid unsafe proliferation that makes the safe/unsafe distinction less useful that could be, the set of instruction set extensions a function is compiled with should be viewed as part of its type for the purpose of determining whether a call is allowed.

Like the type of the arguments imposes a precondition the caller has to satisfy when calling safe functions, the set of instruction set extensions should be viewed as a precondition that the caller has to satisfy for the call to be safe. However, unlike the type checking of arguments, which can't be waived with unsafe, an unsafe block would waive the enforcement of the callee having a subset (including equal) of the instruction set extensions relative to the caller.

In general, I think this issue should be analyzed from the perspective of avoiding impractical results that damage the unsafe/safe distinction of Rust. That is, if mechanical application of precedent yields undesirable results, instead of just going with the undesirable conclusion, it's worthwhile to consider if the description of precedent could be amended somehow. (In this case, by observing that the instruction set extensions that apply should be viewed as a type-like constraint.)

est31 · 2017-11-07T11:24:15Z

@hsivonen

Therefore, in order to avoid everything becoming unsafe and unsafe/safe distinction getting less useful as a result, the rule that only a callee marked unsafe can cause the requirement for an unsafe block to be used to call it be relaxed: In addition, an unsafe block is needed to call a function not marked unsafe if the callee has target_features that the caller doesn't have.

Like the type of the arguments imposes a precondition the caller has to satisfy when calling safe functions, the set of instruction set extensions should be viewed as a precondition that the caller has to satisfy for the call to be safe. However, unlike the type checking of arguments, which can't be waived with unsafe, an unsafe block would waive the enforcement of the callee having a subset (including equal) of the instruction set extensions relative to the caller.

Both of these statements are very similar to my own views on the matter. One can even go a step further and allow safe calling of target_feature fn's inside contexts where you know the target via a cfg macro or cfg attribute. However, I think all of this can be done as a refinement. Remember, right now Rust has no target feature at all :). See this as a first step!

eddyb · 2017-11-07T11:25:38Z

This almost sounds like having an "unsafe_{if,unless}" feature, or safety preconditions/effects.
The only tricky bit seems to be scoping an assumption for unsafety/effect checking.
cc @nikomatsakis I believe the old typestate system had some similarities?

est31 · 2017-11-07T11:44:54Z

@eddyb yes is a bit novel because you'd have to do some control flow analysis for the feature (for reference, what I want is to have fn_without_target_feature() { if cfg!(target_feature = "avx") { fn_that_has_target_feature(...) } } to not require unsafe).

Now cfg macros and attributes are evaluated statically, so combining them with functions with target_feature attributes is not really useful. However, one can also consider safe calling support for some dynamic dispatch mechanism, whether it is matching on the result of some lang item function, or something function pointer based with some automation provided by the language.

golddranks · 2017-11-07T12:04:49Z

What if target_feature is kept unsafe, but it is used only for constructing a zero-sized struct that has the target platform API as methods that are safe to call. This way the type system guarantees safety.

hsivonen · 2017-11-07T12:11:33Z

What you propose sounds in general like a good idea, I encourage you to write an RFC that explores that and please ping us here once you have a draft!

I wrote a draft.

gnzlbg · 2017-11-07T12:46:05Z

@hsivonen the RFC or the unresolved questions section should mention function pointers and trait methods (e.g calling a target feature function via a trait object). If you phrase that as an improvement over the current proposal, you might get away with just falling back to having to annotate those with unsafe (that is, your RFC would improve most cases, but for function pointers and trait methods we would need an improvement over that). Also it might be worth to point if there are any issues in this RFC that would make improvements like the one you propose to require a backward incompatible change. @est31 'if cfg_feature_enabled!("sse4.1") { ... }' Could expand to: 'if __runtime_detection("sse4.1") #[safe(target_feature = "sse4.1")] { ... }' allowing us to remove the need for unsafe in those blocks (but we would need to allow attributes in more positions).

…

On Tue 7. Nov 2017 at 13:11, Henri Sivonen ***@***.***> wrote: What you propose sounds in general like a good idea, I encourage you to write an RFC that explores that and please ping us here once you have a draft! I wrote a draft <https://github.com/hsivonen/rfcs/blob/minimal-target-feature-unsafe/text/0000-minimal-target-feature-unsafe.md> . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2045 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3NpstR0Cb-f4QCHfvgQgdx25fU1O2-ks5s0EkAgaJpZM4OFlXl> .

hsivonen · 2017-11-09T09:59:50Z

@gnzlbg :

the RFC or the unresolved questions section should mention function pointers and trait methods (e.g calling a target feature function via a trait object).

Thanks. I covered those and created a PR.

gnzlbg changed the title ~~target_feature~~ RFC: target_feature Jun 26, 2017

gnzlbg force-pushed the target_feature branch 2 times, most recently from a2791a4 to 07e23d4 Compare June 26, 2017 19:55

gnzlbg force-pushed the target_feature branch 3 times, most recently from 5693463 to 0aa1e75 Compare June 27, 2017 13:23

target_feature

2da82c6

gnzlbg force-pushed the target_feature branch from 0aa1e75 to 2da82c6 Compare June 27, 2017 18:23

gnzlbg closed this Jun 27, 2017

gnzlbg reopened this Jun 27, 2017

gnzlbg force-pushed the target_feature branch from 7aba0f5 to bc6e292 Compare June 27, 2017 20:25

BurntSushi reviewed Jun 27, 2017

View reviewed changes

gnzlbg force-pushed the target_feature branch 8 times, most recently from 00c48d7 to 98d5ec6 Compare June 27, 2017 21:20

withoutboats added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jun 27, 2017

gnzlbg force-pushed the target_feature branch from 98d5ec6 to b4a94f4 Compare June 27, 2017 21:26

alexcrichton merged commit 549d0eb into rust-lang:master Sep 25, 2017

alexcrichton mentioned this pull request Oct 28, 2017

Minimal path to stabilization rust-lang/stdarch#159

Closed

3 tasks

main-- mentioned this pull request Feb 17, 2018

Tracking issue for asm (inline assembly) rust-lang/rust#29722

Closed

Centril mentioned this pull request Apr 10, 2018

Target Feature: target_feature 1.1 Centril/rfc-effects#10

Open

Centril added A-target Target related proposals & ideas A-cfg Conditional compilation related proposals & ideas A-target_feature target_feature related proposals & ideas A-attributes Proposals relating to attributes labels Nov 23, 2018

newpavlov mentioned this pull request Jul 29, 2021

Tracking issue for WebAssembly SIMD support rust-lang/rust#74372

Closed

7 tasks

taiki-e mentioned this pull request Nov 8, 2024

Tracking Issue for sparc_target_feature rust-lang/rust#132783

Open

4 tasks

RFC: target_feature #2045

RFC: target_feature #2045

Conversation

gnzlbg commented Jun 26, 2017

dlight commented Jun 26, 2017

eddyb commented Jun 26, 2017

nagisa commented Jun 27, 2017 via email • edited by Manishearth Loading

gnzlbg commented Jun 27, 2017 • edited Loading

gnzlbg commented Jun 27, 2017

gnzlbg commented Jun 27, 2017 • edited Loading

gnzlbg commented Jun 27, 2017

BurntSushi Jun 27, 2017

Choose a reason for hiding this comment

gnzlbg Jun 27, 2017

Choose a reason for hiding this comment

BurntSushi Jun 27, 2017

Choose a reason for hiding this comment

gnzlbg Jun 27, 2017

Choose a reason for hiding this comment

BurntSushi Jun 27, 2017

Choose a reason for hiding this comment

gnzlbg Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

BurntSushi Jun 27, 2017

Choose a reason for hiding this comment

gnzlbg Jun 27, 2017

Choose a reason for hiding this comment

alexcrichton commented Sep 25, 2017

hsivonen commented Nov 6, 2017

gnzlbg commented Nov 6, 2017

hsivonen commented Nov 6, 2017

gnzlbg commented Nov 6, 2017 • edited Loading

hsivonen commented Nov 6, 2017

gnzlbg commented Nov 6, 2017 • edited Loading

hsivonen commented Nov 6, 2017

gnzlbg commented Nov 6, 2017

parched commented Nov 6, 2017

RalfJung commented Nov 7, 2017

gnzlbg commented Nov 7, 2017 • edited Loading

hsivonen commented Nov 7, 2017 • edited Loading

est31 commented Nov 7, 2017

eddyb commented Nov 7, 2017

est31 commented Nov 7, 2017

golddranks commented Nov 7, 2017 • edited Loading

hsivonen commented Nov 7, 2017

gnzlbg commented Nov 7, 2017 via email

hsivonen commented Nov 9, 2017

nagisa commented Jun 27, 2017 via email •

edited by Manishearth

Loading

gnzlbg commented Jun 27, 2017 •

edited

Loading

gnzlbg commented Jun 27, 2017 •

edited

Loading

gnzlbg Jun 27, 2017 •

edited

Loading

gnzlbg commented Nov 6, 2017 •

edited

Loading

gnzlbg commented Nov 6, 2017 •

edited

Loading

gnzlbg commented Nov 7, 2017 •

edited

Loading

hsivonen commented Nov 7, 2017 •

edited

Loading

golddranks commented Nov 7, 2017 •

edited

Loading