Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ykstackmaps can't deal with >1 compilation unit #2

Open
vext01 opened this issue Jul 4, 2018 · 7 comments
Open

ykstackmaps can't deal with >1 compilation unit #2

vext01 opened this issue Jul 4, 2018 · 7 comments

Comments

@vext01
Copy link
Member

vext01 commented Jul 4, 2018

Currently ykstackmaps finds the stackmap section, and reads the stackmap info it finds there, allowing the user to iterate it, stopping after the end of all stackmap records.

The problem is that multiple stackmap "tables" can be encoded in the section. If you have more than one CGU, then multiple sets of stackmap info are created independently in separate object files. Then at the end, the linker simply concatenates all of the stackmap infos together back-to-back (using the documented 8-byte alignment on the end of each stackmap "block").

ykstackmaps only expects one "table" of stackmap info, but there can be many.

You can verify this using radare2.

$ r2 mybin # assume >1 CGU was used
[0x00022e90]> f | grep LLVM_Stack   
0x000f2f50 0 loc.__LLVM_StackMaps   
0x000f3008 0 loc.__LLVM_StackMaps_1 
0x000f3078 0 loc.__LLVM_StackMaps_2 
0x000f30b8 0 loc.__LLVM_StackMaps_3 
0x000f3248 0 loc.__LLVM_StackMaps_4 
0x00116568 0 loc.__LLVM_StackMaps_5 
0x001343e0 0 loc.__LLVM_StackMaps_6 
0x0014adb8 0 loc.__LLVM_StackMaps_7
...
[0x000f2f50]> s loc.__LLVM_StackMaps
[0x000f2f50]> px
- offset -   0 1  2 3  4 5  6 7  8 9  A B  C D  E F  0123456789ABCDEF
0x000f2f50  0300 0000 0200 0000 0000 0000 0500 0000  ................
0x000f2f60  c02f 0200 0000 0000 4800 0000 0000 0000  ./......H.......
[0x00000000]> s loc.__LLVM_StackMaps_1
[0x000f3008]> px
- offset -   0 1  2 3  4 5  6 7  8 9  A B  C D  E F  0123456789ABCDEF
0x000f3008  0300 0000 0100 0000 0000 0000 0300 0000  ................
0x000f3018  5030 0200 0000 0000 7800 0000 0000 0000  P0......x.......

We can see in the hexdumps above the stackmap version, # of funcs, # of constants and # of records fields for those two stackmap "tables".

Ultimately, we'd want to allow >1 CGU, which would mean finding a way to know how many tables to expect and reading them all.

Until then, we can use the following hack to force one CGU and thus one set of stackmap data. In Cargo.toml:

[profile.dev]                                                                   
codegen-units = 1                                                               
incremental = false       

(profile.release for --release builds)

This seems to give a full set of stackmaps in the first table, BUT sadly r2 still finds other tables (loc.__LLVM_StackMaps_1 etc.) which contain garbage. The have a correct looking LLVM stackmap version header, but the info beyond that is junk.

We'd need to look into a way to find how many tables to look for.

@ltratt
Copy link
Member

ltratt commented Jul 4, 2018

I think the answer to the following question is "no", but: can we enforce / check codegen-units somewhere? Because what I think is going to happen is we use ykstackmaps in another library with codegen-units > 1, and then we're going to get confused. But I might be hoping for too much here...

@vext01
Copy link
Member Author

vext01 commented Jul 4, 2018

There's another side-problem relating to your question too. To correctly issue sequential stackmap IDs we cannot use incremental compilation either. Since we are forking rustc, we can pretty do what we want, so yes, we could check for these things.

@vext01
Copy link
Member Author

vext01 commented Jul 4, 2018

Bah, sadly the workaround doesn't work if you have dependencies, as each dependency forks off a fresh rustc with its own CGU.

@ltratt
Copy link
Member

ltratt commented Jul 4, 2018

Surely each dependency would have to be compiled with our fork of rustc?

@vext01
Copy link
Member Author

vext01 commented Jul 4, 2018

That is true, but irrelevant.

Because a dependency is built using a fresh rustc (and thus a fresh CGU), each dependency is compiled separately and gets its own stackmap table. Only at the linking stage do the stackmap tables get merged by the linker. It also means that stack map IDs are not unique, which is another issue for us.

@ltratt
Copy link
Member

ltratt commented Jul 4, 2018

Surely it was inevitable that stackmaps are only valid for their crate? I don't see how it could be any other way, but maybe I'm missing out on something.

@vext01
Copy link
Member Author

vext01 commented Jul 4, 2018

Surely it was inevitable that stackmaps are only valid for their crate?

Maybe, yeah.

We can work around the ID issue, but there's no avoiding the need to change ykstackmaps to cater for >1 stackmap table.

What is slightly ironic is that LLVM's own stackmap dumping tool doesn't cater for >1 table either!

jacob-hughes pushed a commit that referenced this issue May 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants