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

wip correct unwinding for JIT frames #14799

Closed
wants to merge 1 commit into from
Closed

Conversation

carnaval
Copy link
Contributor

No description provided.

@yuyichao
Copy link
Contributor

Ref #12380 ... I guess I finally understand what that PR was for after going through these ourselves. It looks like ORCJIT might not have the problem with 32bit offset? (although the function we tested with was also very small). There also seems to be another (local only) mechanism that support larger offset.

@maleadt
Copy link
Member

maleadt commented Jan 26, 2016

Maybe add some comments on what the code does?

@carnaval
Copy link
Contributor Author

@yuyichao oh man. I knew I saw this somewhere. arg. Well all this time we spent reading libunwind source was somewhat instructive anyway. I'll see if we can use the "nice" local version today to avoid parsing the sections ourselves.

@maleadt yup at least before merging. This is just clerical work because the feature of libunwind we use to register new unwinding info dynamically for JIT code was not meant to register whole ELF objects at once so we have to do a little manual work (split the relevant section into pieces).

@carnaval
Copy link
Contributor Author

I don't think the local format allows larger offsets though since as I remember it it just fills the same table of (int32,int32) in the end.

As for the ugly code, it may make sense to just patch libunwind anyway to have a proper register_elf_image_buffer() or something, but in the meantime that will do.

@carnaval
Copy link
Contributor Author

(or just write an unwinder in julia =))

@tkelman
Copy link
Contributor

tkelman commented Jan 26, 2016

Keno probably has a repo for that somewhere

@yuyichao
Copy link
Contributor

I'll see if we can use the "nice" local version today to avoid parsing the sections ourselves.

IIRC all the functions that contains section names are called on a file so probably not without patching.

I remember it it just fills the same table of (int32,int32) in the end.

You are right, I thought the list is stored in a slightly different format (since I remember there's no pointer cast). Turns out they just use struct table_entry * but didn't define it in the header.....

patch libunwind anyway to have a proper register_elf_image_buffer() or something

Seems that libunwind also only look for debug_frame but not eh_frame. That might need to be fixed too...

or just write an unwinder in julia =)

Unfortunately not really when we are in signal context.

@carnaval
Copy link
Contributor Author

carnaval commented Feb 5, 2016

let's use #12380 instead, it's cleaner and together with the libunwind patch handles the 32 bit overflow condition Keno found with mcjit.

@carnaval carnaval closed this Feb 5, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Feb 5, 2016

Agree. I've just been using this patch since it's easier to rebase and test =P.

@yuyichao yuyichao deleted the ob/i_hate_libunwind branch February 13, 2016 23:10
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

Successfully merging this pull request may close these issues.

None yet

4 participants