fix(labels): report confirmed-format parse failures at warning - #246
Open
r0ny123 wants to merge 1 commit into
Open
fix(labels): report confirmed-format parse failures at warning#246r0ny123 wants to merge 1 commit into
r0ny123 wants to merge 1 commit into
Conversation
Once a provider or loader has positively confirmed the input's format - a magic check, an isinstance(lief.<Format>.Binary) gate, or a structurally validated header - a whole-parse or whole-loop failure that leaves it with zero output is a real failure, not a "this is not my format" non-event. Recording it at debug, or in three cases at no level at all, is what made the PDB symbol defect in danielplohmann#230 invisible: the run completes, every function stays unnamed, and the log says nothing about why. Elevate those records to warning across the CIL, Rust, Mach-O, Delphi and Go providers, BinaryInfo.getBinaryData, and the PE/ELF/Mach-O whole-image mapping. The Go provider had no record at all after a validated pclntab header - struct.error, IndexError and ValueError are not re-raised by reraise_non_operational_exception - and the Mach-O stub loop swallowed every failure through a bare except/pass. FileLoader confirms the format with isCompatible() and then shares one parseBinary() result across every accessor, so a parse that fails there made mapBinary and getCodeAreas both return empty with nothing logged. Record that once where the confirmation and the failure meet, rather than in each accessor, so one root cause yields one warning. Per-item failures inside loops that continue, and accessors called speculatively against every binary (ElfFileLoader.getPltRanges and getGotBases run for raw dumps and other formats too), stay at debug by design.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The class
Once a provider or loader has positively confirmed the input's format — a magic check, an
isinstance(lief.<Format>.Binary)gate, or a structurally validated header — a whole-parse or whole-loop failure that leaves it with zero output is a real failure, not a "this is not my format" non-event. Recording it atdebug, or in three cases at no level at all, is what made the PDB symbol defect in #230 invisible: the run completes, every function stays unnamed, and the log says nothing about why.That defect had siblings. This raises them to
warningacross the CIL, Rust, Mach-O, Delphi and Go symbol providers,BinaryInfo.getBinaryData(), and the PE/ELF/Mach-O whole-image mapping.The three that logged nothing at all
GoLabelProvider.update()caught the failure of_parse_pclntab()and returned bare. The header was already validated for magic, zero padding, PC quantum and pointer size before that call, so this only fires on a genuine Go binary — andreraise_non_operational_exceptionre-raises onlyAssertionError/ImportError/MemoryError/NameError/ReferenceError/SyntaxError, sostruct.error,IndexErrorandValueErrorvanished with no trace. Every Go symbol lost, silently.MachoSymbolProvider's symbol-stub loop was a bareexcept Exception: pass, afterisinstance(lief.MachO.Binary)had already confirmed the container.FileLoaderconfirms the format withisCompatible()and then shares oneparseBinary()result across every accessor, so a parse that failed there mademapBinary()andgetCodeAreas()both return empty with nothing logged. Recording it once where the confirmation and the failure meet — rather than in each accessor — means one root cause yields one warning instead of several.What deliberately stays at debug
Per-item failures inside loops that
continueto the next entry: one malformed symbol should not be reported as a failed provider.Accessors called speculatively against every binary regardless of format.
ElfFileLoader.getPltRanges()andgetGotBases()run for raw dumps and other containers too, so "no PLT here" is the ordinary case, not a fault.RustSymbolProvider's "no lief binary" path likewise returns without a log — only an actual raise fromgetLiefBinary()is elevated.Validation
ruff check .andruff format --check .clean;ty checkexits 0 with no new diagnostics;python -m pytest tests/1191 passed, 1 skipped, 1482 subtests;diff-cover --fail-under=100reports 18/18 changed lines covered across all ten files. New failure-path tests cover each elevated site intestCommonModels,testDelphiPythiaProvider,testFileFormatParsers,testGoSymbolProvider,testMachoSymbolProviderandtestRustSymbolProvider.One judgement call worth a second opinion
DelphiReSymProvider's "no code areas found" warning sits behind a PE-with-.textcheck and theTObjectmarker, butcode_areasis empty for buffer-mode analysis —disassembleBuffer()defaults it to[]. So a Delphi PE analyzed as a raw buffer, which is a normal path for memory dumps, will now warn on every run. The message is accurate (Delphi parsing really is being skipped and symbols really are lost), but if that reads as noise rather than signal for dump-centric workflows, that one line is the one to reconsider.Relationship to #230
Complementary, not overlapping. #245 fixes this class inside
PdbSymbolProvider; this fixes the pre-existing sites elsewhere. The two change disjoint sets of files and can land in either order. Note that both raise log levels, so a corpus run over malformed inputs gets louder than master by design.