[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

Should I propose a simple patch to bump INSTR_PROF_RAW_VERSION ?
Or will it be part of something bigger, taken care of by someone else ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

This commit has broken oss-fuzz workflow for rust projects.
I do not know if the fix is 
https://reviews.llvm.org/rG66e2772e4285588ccc3bcdb5f392c8326debbd7d

Scenario is

  export RUSTFLAGS="--cfg fuzzing -Cdebuginfo=1 -Cforce-frame-pointers 
-Zinstrument-coverage -C link-dead-code -C link-arg=-lc++"
  git clone https://github.com/ctz/rustls && cd rustls
  cargo fuzz build -O
  ./fuzz/target/x86_64-unknown-linux-gnu/release/client 
./fuzz/target/x86_64-unknown-linux-gnu/release/client
  llvm-profdata merge -sparse default.profraw -o profdata

Current output is

  warning: default.profraw: malformed instrumentation profile data
  error: no profile can be merged

Scenario does not output error for in 13.0.0-rc2

Error does not happen on commit 69cdadddecaf97f572c311aa07044f79a5b8329a 
 just 
previous this integration, if we add `git cherry-pick 
a6c14fba70e170a279f7e77f068368f09d8c5eaf` to fix the other pending bug which 
was fixed in 13.0.0-rc2
Error happens on commit a1532ed27582038e2d9588108ba0fe8237f01844 
 even if 
we `git cherry-pick a6c14fba70e170a279f7e77f068368f09d8c5eaf` and fix the 
conflict about only llvm/test/tools/llvm-profdata/Inputs/c-general.profraw


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

Thanks for the quick reply

> I don't think this change is responsible. This has been tested by many C++ 
> downstream groups.

What did I do wrong ?
Adding this only commit makes the scenario fail.

> Rust should use upgrade its lib/ProfileData and compiler-rt/lib/profile, and 
> not mix raw profile files at different commits.

I am not sure I understand.
What are raw profiles ? 
In my scenario, only `llvm-profdata` acts on default.profraw
And before that, the linker, not the rust compiler, mixes together the 
different coverage sections...
So, how would Rust mix raw profiles ?

> The binary ID change has caused a bit of churn to ProfileData but it is 
> unrelated to this patch.

Well, there may be other bugs, but this is not a problem in my scenario...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

> Bump the raw profile format version to 6. (Last bump happened in 2019-10.)

This does not seem to be in the patch, ie no change of `INSTR_PROF_RAW_VERSION` 
in compiler-rt/include/profile/InstrProfData.inc
Is that missing ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

> .profraw are files with the raw profile format. The compiler-rt/lib/profile 
> runtime and llvm-profdata only support one version at any commit.

Ok.

> Mixing .profraw files produced by different compiler-rt/lib/profile runtimes 
> is unsupported.

I do not think I am mixing profraw files as I have only one profraw file.
If my llvm-profdata is not supporting the profraw file produced by rust + 
linker, should it not error with
` Unsupported instrumentation profile format version` instead of `malformed 
instrumentation profile data`
As it does with rust nightly and clang 12 ?

> I don't know what rustc and https://github.com/ctz/rustls do.

Rustls is just an example rust project, I think we get the same result with 
every rust project.

> I don't think this change is responsible as the change has been well 
> released/tested by many C++ groups.

I guess I did not mean to use the word responsible.
I am just saying that if I `git revert 
a1532ed27582038e2d9588108ba0fe8237f01844` and solve the conflicts on top of 
main, I get my scenario to work again

> If Rust adapts compiler-rt and does something different, I think the 
> investigation responsibility is on Rust's side.
>
> Perhaps you can get some help from rustc folks who do the compiler-rt 
> adaptation in rustc.

I am asking indeed


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

> Even if `INSTR_PROF_RAW_VERSION` is bumped, I think it is very likely your 
> Rust downstream will observe an `if (GET_VERSION(Version) != 
> RawInstrProf::Version)` error in llvm-profdata,
> because from your description it seems you are likely mixing raw profile 
> files produced by compiler-rt/lib/profile built at different commits.

For the record, bumping `INSTR_PROF_RAW_VERSION` on top of main branch, (or 
after reverting this commit), in both 
compiler-rt/include/profile/InstrProfData.inc and 
llvm/include/llvm/ProfileData/InstrProfData.inc, I indeed get the error message 
`unsupported instrumentation profile format version`

The output for `file default.profraw` is `default.profraw: LLVM raw profile 
data, version 7`

> Bumping INSTR_PROF_RAW_VERSION will give a better diagnostic but won't 
> address the root cause that only one version is supported, which I totally 
> agree since otherwise this would add too much burden on the upstream LLVM 
> developer side.

Well, now I have my diagnostic :-) (it is not like 
https://github.com/rust-lang/rust/issues/82875 for instance)

> this is "Support for older format versions in RawInstrProfReader" 
> https://lists.llvm.org/pipermail/llvm-dev/2021-August/152287.html

Thanks for the pointer

> You'll need to rebuild everything on the Rust side if you want to use git 
> LLVM.

Indeed, I will try to rebuild rust with clang-14


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104556: [InstrProfiling] Make CountersPtr in __profd_ relative

[Philippe Antoine via Phabricator via cfe-commits]

catenacyber added a comment.

Should we still bump `INSTR_PROF_RAW_VERSION ` so that we are able to 
distinguish profraw files produced by clang13 and the ones produced by clang14 ?

Right now, both produce `LLVM raw profile data, version 7`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104556/new/

https://reviews.llvm.org/D104556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits