Missing license attribution for gdbstub derived code

[daniel5151]

Hey there,

It looks like reverie is using a fork of my gdbstub library without including the appropriate MIT license attribution (as per the license terms here). Based on a quick glance through the code, the forks to be pretty old, but there's nonetheless still some substantial chunks of my original implementation in there...

The relevant code in reverie lives under the following project subdirectory.

https://github.com/facebookexperimental/reverie/tree/15d2f6141137177e8fd292936dd01f63f3535a7f/reverie-ptrace/src/gdbstub

---

There are many examples of identical code, but one simple one in hex.rs:

reverie/reverie-ptrace/src/gdbstub/hex.rs

Lines 265 to 288 in 15d2f61

	/// Decode a GDB dex string into the specified integer.
	///
	/// GDB hex strings may include "xx", which represent "missing" data. This
	/// method simply treats "xx" as 00.
	pub fn decode_hex<I>(buf: &[u8]) -> Result<I, GdbHexError>
	where
	I: FromPrimitive + Zero + CheckedAdd + CheckedMul,
	{
	if buf.is_empty() {
	return Err(GdbHexError::Empty);
	}
	let radix = I::from_u8(16).ok_or(GdbHexError::InvalidOutput)?;
	let mut result = I::zero();
	for &digit in buf {
	let x = I::from_u8(from_hex(digit).ok_or(GdbHexError::NotAscii)?)
	.ok_or(GdbHexError::InvalidOutput)?;
	result = result.checked_mul(&radix).ok_or(GdbHexError::Overflow)?;
	result = result.checked_add(&x).ok_or(GdbHexError::Overflow)?
	}
	Ok(result)
	}

vs. my implementation at

https://github.com/daniel5151/gdbstub/blob/dev/0.6/src/protocol/common/hex.rs#L11-L36

---

A more GDB RSP specific one is here:

reverie/reverie-ptrace/src/gdbstub/packet.rs

Lines 54 to 72 in 15d2f61

	let end_of_body = bytes
	.iter()
	.position(|b| *b == b'#')
	.ok_or(PacketParseError::MissingChecksum)?;
	// Split buffer into body and checksum, note the packet
	// starts with a `$'.
	let (body, checksum) = bytes.split_at_mut(end_of_body);
	let checksum = &checksum[1..][..2]; // skip the '#'
	// Validate checksum without leading `$'.
	let checksum = decode_hex(checksum).map_err(|_| PacketParseError::MalformedChecksum)?;
	let calculated = body.iter().skip(1).fold(0u8, |a, x| a.wrapping_add(*x));
	if calculated != checksum {
	return Err(PacketParseError::ChecksumMismatched {
	checksum,
	calculated,
	});
	}

vs. my implementation at

https://github.com/daniel5151/gdbstub/blob/0.4.5/src/protocol/packet.rs#L38-L56

[jasonwhite]

Hey Daniel, thanks for pointing out this mistake! We're definitely not trying to take credit for your excellent work. I'll fix that ASAP.

There should probably be two copyright headers for some files since we made quite a few significant changes. We initially tried to use your gdbstub crate as-is, but ran into problems integrating it with our async-ptrace implementation. We had to make a lot of changes to get around that limitation and so there shouldn't be too much in common. I'll go through everything and add the copyright attribution to the files where it makes sense.

I don't think we looked into using gdbstub again since daniel5151/gdbstub#36 was solved. I'd love to circle back and get rid of our version completely if it's possible now.

[daniel5151]

Hey Jason, all good!

I've got a habit of occasionally searching GitHub for gdbstub to see if any new projects are using it, and just happened to stumble across this one. Always happy to see more folks finding my niche little library useful :)

And yeah, if you do happen to have some spare cycles to get back in-sync with upstream gdbstub now that the async issue has been resolved, that'd be awesome! The library's gained a bunch of functionality over the past few months, and I'd be super keen to integrate any extensions y'all have developed as well.

Plus, I've really been dragging my feet on finally publishing gdbtsub 0.6, so maybe this'll be some good motivation to finally do that 😄

Thanks again!

[jasonwhite]

The library's gained a bunch of functionality over the past few months, and I'd be super keen to integrate any extensions y'all have developed as well.

That's great to hear! I don't think we have additional GDB extensions implemented above what is already implemented in (your) gdbstub.

Plus, I've really been dragging my feet on finally publishing gdbtsub 0.6, so maybe this'll be some good motivation to finally do that 😄

Alas, Reverie doesn't work on stable yet, so there's no requirement for gdbstub 0.6 to be published to crates.io yet. Time is the more limiting factor here. I'll try to contribute back fixes if/when we can get rid of our own version of gdbstub. :)

Please let me know if you believe I missed anything in #10. Otherwise, I plan on merging it as-is. Thanks!

[daniel5151]

That's great to hear! I don't think we have additional GDB extensions implemented above what is already implemented in (your) gdbstub.

I don't think I've gotten around to implementing the various fork/vfork/clone stop reasons upstream, or the thread exit reason. Not that they'd be hard to implement, but no consumers of the library have needed them yet, so they haven't been implemented ¯\_(ツ)_/¯

Please let me know if you believe I missed anything in #10. Otherwise, I plan on merging it as-is. Thanks!

LGTM 👍

Thanks again for the quick response and resolution!