search

xd009642 / llvm-profparser

Mostly complete pure rust implementation of parsing llvm instrumentation profile data
Apache License 2.0
13 stars 9 forks source link

Fixed issue where the same byte was read twice instead of chopping a... #27

Closed Quessou closed 1 year ago

Quessou commented 1 year ago

... new byte from bytes buffer in parse_mapping_regions.

The crash is gone, the output seem relevant to me, but a double check on this may be more than welcome !

However I'm a bit curious about how the entire llvm-profparser could even remotely work even though counter parsing was apparently quite dysfunctional and was causing parsing offsets, a bit of enlightenment for my own culture here would be welcome ! :)

xd009642 commented 1 year ago

Aha I see it now, so when a program is compiled with LLVM coverage instrumentation, it's up to the compiler to insert some LLVM IR instructions to annotate regions and locations etc which then gets turned into instrumentation and the compiler bundles in a runtime to handle collecting and emitting the data.

The specific bug looks to be in a branch region, something which the rust compiler currently doesn't use as it only outputs line information. Whereas clang will likely output everything given it has first class support for this stuff - and has had it a lot longer

xd009642 commented 1 year ago

Also, don't worry about nightly failures - looks like Rust has updated it's llvm version and I need to add support to it (at least just test files). For 1.60 it says that a dependency has bumped the MSRV so change it to 1.64 and I'll just bump my minimum version as well (1.64 feels old enough to be a conservative bump)

Quessou commented 1 year ago

Aha I see it now, so when a program is compiled with LLVM coverage instrumentation, it's up to the compiler to insert some LLVM IR instructions to annotate regions and locations etc which then gets turned into instrumentation and the compiler bundles in a runtime to handle collecting and emitting the data.

The specific bug looks to be in a branch region, something which the rust compiler currently doesn't use as it only outputs line information. Whereas clang will likely output everything given it has first class support for this stuff - and has had it a lot longer

Ok, that explains everything, thanks for the insight :D Since the bug seemed to happen only in what looked like edge cases from the outside, I did not look thoroughly into the parsing algorithm, thinking that if there was an issue there, it would've appeared much earlier, while using profparser on vaguely complex rust programs. Huge mistake from my side lmao

Bump version is done btw

xd009642 commented 1 year ago

Cool LGTM!