update to serde 1.0

[m4b]

WIP do not merge

Requires:

1. rust-graph-algos PR to merge https://github.com/flanfly/rust-graph-algos/pull/7
2. update to use flanfly git (probably should publish this on crates.io or switch to bluss's crate)
3. fix UI brokeness
4. update test

[m4b]

This PR breaks the UI opener/save file functionality so I can't fix the one test; @flanfly do you know what is wrong?

[flanfly]

@m4b my guess is that Serde msgpack is not compatible w/ RustcSerialize msgpack. Ignore it. Ok, now I get it. You can't deserialize serialized Projects. I tried w/ serde_json and that fails too. :(

[flanfly]

I rebased everything and removed unused stuff: https://github.com/das-labor/panopticon/tree/serde

[m4b]

I reset the branch to your rebased version. I don't know why it can't deserialize still :/

[m4b]

also cargo test fails with linkage errors :crying_cat_face:

  = note: /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(glue.cpp.o): In function `QArrayData::sharedNull()':
          /usr/include/qt/QtCore/qarraydata.h:122: undefined reference to `QArrayData::shared_null'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(glue.cpp.o): In function `QTypedArrayData<unsigned short>::deallocate(QArrayData*)':
          /usr/include/qt/QtCore/qarraydata.h:228: undefined reference to `QArrayData::deallocate(QArrayData*, unsigned long, unsigned long)'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(glue.cpp.o): In function `__gnu_cxx::new_allocator<QRecentSession*>::deallocate(QRecentSession**, unsigned long)':
          /usr/include/c++/6.3.1/ext/new_allocator.h:110: undefined reference to `operator delete(void*)'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(glue.cpp.o): In function `__static_initialization_and_destruction_0(int, int)':
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::Init()'
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::~Init()'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(glue.cpp.o):(.data.DW.ref.__gxx_personality_v0[DW.ref.__gxx_personality_v0]+0x0): undefined reference to `__gxx_personality_v0'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(qpanopticon.cpp.o): In function `__static_initialization_and_destruction_0(int, int)':
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::Init()'
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::~Init()'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(qcontrolflowgraph.cpp.o): In function `__gnu_cxx::new_allocator<QControlFlowGraph*>::deallocate(QControlFlowGraph**, unsigned long)':
          /usr/include/c++/6.3.1/ext/new_allocator.h:110: undefined reference to `operator delete(void*)'
          /home/m4b/git/panopticon/target/debug/build/panopticon-c55410a95d10b912/out/lib/libpanopticon-glue.a(qcontrolflowgraph.cpp.o): In function `__static_initialization_and_destruction_0(int, int)':
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::Init()'
          /usr/include/c++/6.3.1/iostream:74: undefined reference to `std::ios_base::Init::~Init()'
          collect2: error: ld returned 1 exit status

I'm also seeing cargo run not stashing build results, and occasionally entire project recompiled with no changes, which is weird... hopefully it's a local problem

[flanfly]

The linker errors should be gone now. I also rebase'd everything onto master and bumped serde to version 1.0.

[flanfly]

Good news: I got serde to work. Bad news not with msgpack. I change the format to cbor for now. File sizes are comparable, esp after compression. ¯\(ツ)/¯

[m4b]

Man bummer. Should we open an issue on msg pack ? I feel like it might be an upstream bug. Seems like it should just work without issue

[sphinxc0re]

Why not use bincode? It has much smaller file sizes

[flanfly]

@m4b I though about it but there are some issues open that sound similar (dups?) Also, they'd probably want an minimal test case and I don't have the time to debug this further.

@sphinxc0re bincode isn't standardized.

[m4b]

Yea I hear no time; I thought about it, but repro would be annoying and time consuming and also it should just work :laughing:

I'm going to open an issue just letting them know we switched because we couldn't get it working and looks like upstream bug but we don't have time for repro.

[m4b]

reset this to upstream/serde

[flanfly]

Thanks!

[m4b]

This appears to be passing all tests, yes?

So once this gets merged, I think we need to think about getting panopticon off of the last git dependency, which iirc, is rust-graph-algos.

I suggest either:

1. publish rust graph algos
2. switch to petgraph

I really, really think having panopticon on crates will increase visibility, increase contributors, and also enable some really cool/interesting uses as a lib :D

[m4b]

heh, except for this, which i can't repro on my machine :/

failures:
---- bounded_addr_track::tests::qc_execute stdout ----
    thread 'bounded_addr_track::tests::qc_execute' panicked at 'attempt to shift right with overflow', core\src\il.rs:939
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'bounded_addr_track::tests::qc_execute' panicked at '[quickcheck] TEST FAILED (runtime error). Arguments: (Select(44, Constant { value: 14259615593554079564, size: 288 }, Constant { value: 16935478967001236414, size: 20 }))
Error: "attempt to shift right with overflow"', C:\Users\appveyor\.cargo\registry\src\github.com-1ecc6299db9ec823\quickcheck-0.3.2\src\tester.rs:118

[coveralls]

Coverage decreased (-0.04%) to 60.222% when pulling e8c43ee8a1481c10afc19bd3c07ad70f3c5a107a on m4b:serde into cbe522438a21b7b36bb101645c8ee52354a638b7 on das-labor:master.

[m4b]

Nvm, seems that the latest refactor pushed graph algos into the crate itself, so I think this is theoretically publishable on crates.io, no?

[flanfly]

Try this:

--- a/core/src/il.rs
+++ b/core/src/il.rs
@@ -935,7 +935,7 @@ pub fn execute(op: Operation) -> Rvalue {
         Operation::Select(off, Rvalue::Constant { value: _a, size: s }, Rvalue::Constant { value: _b, size: _s }) => {
             debug_assert!(off + _s <= s);

-            if off + _s <= 64 {
+            if off + _s < 64 {
                 let hi = _a >> (off + _s);
                 let lo = _a % (1 << off);
                 Rvalue::Constant { value: lo | (_b << off) | (hi << (off + _s)), size: s }

Re graph-algos: I'd prefer to move the project to petgraph or some other library.

[coveralls]

Coverage decreased (-0.4%) to 59.892% when pulling c1cddb3667039a9606ed488cf9bb5c02fb74aa78 on m4b:serde into cbe522438a21b7b36bb101645c8ee52354a638b7 on das-labor:master.

[m4b]

whoa, what's with these huge diffs on files, seems like there's more than serde changes in them? Did this get rebased funny (wondering if their will be duplicate commits?)

[flanfly]

I had to introduce a Aoperation type that's the same as Operation but not serializable. Otherwise we'd have to add a named livetime to everything touching Operation due to serde Deserializer having one. :/

[m4b]

Are you sure? You should be able to deserialize non lifetime structs, etc, no?

[flanfly]

Well, Operation has one type variable V that needs to be serializable. With rustc_serializable this looked link this:

enum Operation<V: ... + Decodable + Encodable> {
  Add(V,V).,
  ...
}

The serde equivalient of Decodable is Deserialize<'de>. So this becomes:

enum Operation<'de, V: ... + Deserialize<'de> + Serialize> {
  Add(V,V).,
  ...
}

The lifetime needs to be present in every type that has an Operation instance (directly or indirectly). So now we have Statement<'de>, Mnemonic<'de>, BasicBlock<'de>, Function<'de>, Program<'de> and Project<'de>.

[m4b]

Would adding 'static to the Deserialize impl make sense ? (Haven't thought about it just shooting out ideas)

If you think it's not worthwhile/ have thought through possibilities then we can drop it, probably not worth the mental energy.

[flanfly]

Ok, I managed to get rid of Aoperation. I'm not sure what #[serde(bound(deserialize = "V: Serialize + for<'a> Deserialize<'a> + Clone + PartialEq + Eq + Debug"))] means exactly but it works.

[coveralls]

Coverage decreased (-0.09%) to 60.172% when pulling dbaa46e0f3e091e093ccb719f00d43dc0381021b on m4b:serde into cbe522438a21b7b36bb101645c8ee52354a638b7 on das-labor:master.

[m4b]

:notes: Do you believe in magic, cause I hope you do :notes:

After this I think @sphinxc0re format PR can go in, yes? which will be awesome

[sphinxc0re]

What do you mean? The format already happened 😁

[m4b]

Well that was fast! :laughing:

[flanfly]

The reformatting was done in PR #295. Part of the reason it was 27k loc ;)