large refactor to support nostd and noalloc. include small msf parsing - #167
large refactor to support nostd and noalloc. include small msf parsing#167Jozefpodlecki wants to merge 2 commits into
Conversation
…g few more fixtures
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| let str = core::str::from_utf8(self.0).map_err(|err| fmt::Error)?; | ||
| write!(f, "{}", str) | ||
| } |
There was a problem hiding this comment.
RawString formatting rejects invalid UTF-8
Medium Severity
Display and Debug for RawString now require valid UTF-8 and return a formatting error otherwise. The type is documented as possibly non-UTF-8, and the old implementations used lossy conversion, so printing PDB names can now fail or panic in format!/println!.
Reviewed by Cursor Bugbot for commit 55c35c0. Configure here.
There was a problem hiding this comment.
I could revert that part to utilize String::from_utf8_lossy as original but then impl Display would have to be alloc gated
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a4ce982. Configure here.
|
|
||
| #[test] | ||
| fn small_stream_sizes_match_toc() -> Result<()> { | ||
| let data = std::fs::read(r#"C:\repos\pdb\fixtures\small1.pdb"#).unwrap(); |
There was a problem hiding this comment.
Test uses absolute Windows path
Medium Severity
The new small_stream_sizes_match_toc test reads C:\repos\pdb\fixtures\small1.pdb instead of the in-repo fixtures/small1.pdb path used elsewhere. The test panics on CI and any machine that is not the author's Windows checkout.
Reviewed by Cursor Bugbot for commit a4ce982. Configure here.
| /// Returns the mode of this pointer. | ||
| pub const fn pointer_mode(self) -> PointerMode { | ||
| match (self.0 >> 5) & 0x7 { | ||
| 0x00 => PointerMode::Pointer, | ||
| 0x01 => PointerMode::LValueReference, | ||
| 0x02 => PointerMode::Member, | ||
| 0x03 => PointerMode::MemberFunction, | ||
| 0x04 => PointerMode::RValueReference, | ||
| _ => unreachable!(), | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: The pointer_mode() function panics via unreachable!() when parsing a malformed PDB file with an invalid pointer mode value (>= 5), as there is no input validation.
Severity: HIGH
Suggested Fix
Add validation to the pointer_mode() function to handle out-of-range values gracefully. Instead of panicking with unreachable!(), return a Result or a default/error variant from the function to allow the caller to handle the malformed data without crashing.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/tpi/types.rs#L307-L336
Potential issue: When parsing an `LF_POINTER` record from a PDB file, the raw `u32` for
pointer attributes is used to create a `PointerAttributes` struct without validation.
The `pointer_to_member()` method is then called, which in turn calls `pointer_mode()`.
The `pointer_mode()` function expects the pointer mode (bits 5-7) to be between 0 and 4.
If a malformed PDB provides a value of 5 or greater, the `match` statement will hit an
`unreachable!()` macro, causing the parser to panic. This is a reachable
denial-of-service vector when processing untrusted PDB files.
| pub const fn intrinsic_type(self) -> bool { | ||
| self.0 & 0x1000 != 0 | ||
| } | ||
|
|
||
| pub const fn mocom(self) -> u8 { | ||
| ((self.0 & 0x6000) >> 14) as u8 | ||
| } |
There was a problem hiding this comment.
Bug: The intrinsic_type() and mocom() methods in TypeProperties use incorrect bitmasks that do not match the documented struct layout, leading to incorrect results if ever called.
Severity: LOW
Suggested Fix
Correct the bitmasks in TypeProperties. Change the mask in intrinsic_type() from 0x1000 to 0x2000. Change the mask in mocom() from 0x6000 to 0xC000 to correctly align with the documented bitfield layout for bits 14 and 15.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/tpi/types.rs#L89-L95
Potential issue: In the `TypeProperties` struct, the bitmasks used by the
`intrinsic_type()` and `mocom()` methods are incorrect according to the documented C
struct layout. `intrinsic_type()` uses mask `0x1000` (bit 12) but should use `0x2000`
(bit 13). `mocom()` uses mask `0x6000` (bits 13-14) but should use `0xC000` (bits
14-15). While these methods are currently not called anywhere in the codebase, this
represents a latent bug that will cause incorrect property extraction if they are used
in the future.
1 participant
So this started as just let's get no_std + alloc working and then I kinda kept going lol
no_std + alloc- the original goal, doneno-alloc- figured while I was in there, might as well get something working for the no-allocator crowd too. It's basic but it worksalloc,std,nightlyandchronoPDBSignature