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

JITs should register unwind information (xdata) on Win64 to make longjmp and exceptions work #24607

Open
rnk opened this issue Jul 23, 2015 · 12 comments
Labels
bugzilla Issues migrated from bugzilla mcjit

Comments

@rnk
Copy link
Collaborator

rnk commented Jul 23, 2015

Bugzilla Link 24233
Version trunk
OS Windows NT
CC @AndyAyersMS,@chfast,@lhames,@weliveindetail,@vtjnash

Extended Description

Currently RTDyldMemoryManager::registerEHFrames() implements this on Mac and Linux, but we need the same support for Win64. It involves calling RtlInstallFunctionTableCallback or RtlAddFunctionTable, as described on MSDN at https://msdn.microsoft.com/en-us/library/ft9x1kdx.aspx?f=255&MSPPError=-2147217396.

@lhames
Copy link
Contributor

lhames commented Jul 23, 2015

Hi Reid,

I want to avoid adding this to RTDyldMemoryManager::registerEHFrames if possible. I'm actually going to propose on llvm-dev that we take the existing registration code out of the base class and require clients to call it manually. The problem is that since MCJIT is cross-process/cross-platform, clients can get caught unawares when the target EH frames are registered by default in the JIT process. When JITing cross target this can result in flat-out invalid frames getting registered (e.g. 32-bit target frames registered on a 64-bit host), with unpredictable results. There's also currently a memory leak in this code, as the frames aren't deregistered by default (at least on Darwin).

If there's any non-trivial helper code that we cold provide to wrap these functions I'd be happy for that to go in tree though. It'd be nice if all the client had to do was something along the lines of:

class MyMemMgr : public RuntimeDyld::MemoryManager {
public:
void registerEHFrames() override { RegisterWin64EHFrames(...); }
void deregisterEHFrames() override { DeregisterWin64EHFrames(...); }
/// ... Other overrides ...
}

@rnk
Copy link
Collaborator Author

rnk commented Jul 23, 2015

Sure, sounds reasonable. I'm not planning to work on this right now, I mostly wanted to get this filed to make it easier to explain why longjmp doesn't work with MCJIT on Win64.

I don't recall what the current status of MCJIT producing COFF files is, but I think that's the first step towards generating the right frame information in the first place.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 19, 2015

We currently have the situation that, in windows, the callstack is lost inside jitted code even though we don't use exceptions or longjumps at that point.
That is, when stepping through the assembly instructions the callstack disappears when entering the call to a jitted function.

Similarly when a crash occurs in a jitted function (due to e.g. null pointer dereference) the debugger cannot show the callstack.

Is this also due to the same problem? If so, I'd be very interested in some helper code and/or a working example.

@rnk
Copy link
Collaborator Author

rnk commented Oct 19, 2015

Is this also due to the same problem? If so, I'd be very interested in some
helper code and/or a working example.

Yes, registering the UNWIND_INFO tables should solve the problems you described.

Unfortunately, I don't know of any small example apps that use this API that I can point at. There are large examples like CoreCLR.

@rnk
Copy link
Collaborator Author

rnk commented Oct 21, 2015

*** Bug #12922 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 4, 2016

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 4, 2016

Hi all,

I did some testing with this issue under Win64 and the exception handling works sometimes.

I modified the example from duplicate bug report 12550 to work with 3.7.1 and MCJit. I'm recursively traversing in a generated function, eventually the generated function calls an static function that throws a std::runtime_error.

If I don't invoke the FibF->dump() at line 152, the exception handling works as expected. But if I do enable that call, the the exception is not handled.

So why is the dump() call having affect on the EH data ?
The win64 EH seems to work magically except when certain function have been called. I'm trying to find a workaround for EH in win64. I'd rather not MCJIT if possible :)

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2016

ADDR32NB relocations
Hi all,

In my project I've solved this issue passing Function Table from .pdata to RtlAddFunctionTable. But resolving of IMAGE_REL_AMD64_ADDR32NB relocations is incomplete in LLVM. All relocations are filling with zeros now. So I added trivial RVA calculation here and then saved ".pdata" section instead of ".xdata" for passing to MemoryManager::registerEHFrames. Here I attached patch file.

Potential problem is as described in comments:
// Note ADDR32NB requires a well-established notion of
// image base. This address must be less than or equal
// to every section's load address, and all sections must be
// within a 32 bit offset from the base.

Proposed code doesn't manage sections load addresses.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2016

Modified Fib. example
I've modified previous sample to show how I registered the function table.
Custom memory manager is required to catch image base and implement final registerEHFrames function.

This sample is rough enough and may contain errors.

P.S. Can anyone propose more accurate way to retrieve Image Base pointer?
What is the difference between LoadAddr and Addr of section?

@AndyAyersMS
Copy link
Contributor

There are some built-in assumptions in RUNTIME_FUNCTION. PE files are guaranteed to occupy no more than a 4GB extent when loaded, so 32 bit offsets plus the base of the loaded image are enough to identify locations within the image.

Hence the RUNTIME_FUNCTION structure expects that the unwind data (.pdata and .xdata) will be located sufficiently "close" to the function that 32 bit offsets are sufficient. You need to provide these same guarantees, somehow.

The actual loaded location of the .pdata doesn't matter since you will report this information yourself via RtlAddFunctionTable. But the location of the .xdata does matter, and that's the tricky bit.

So here's a sketch of a solution.

RTDyld will relocate each .text section independently. Call RtlAddFunctionTable once per .text section, setting the BaseAddress to the address of the relocated base of the section. Then use the contents of the associated .pdata as the function table. You can safely ignore the relocations on the begin/end members, since the underlying values are already correct if the base of the image is the same as the base of the section.

Then, you must, somehow, guarantee that the associated .xdata is loaded at a higer address than the .text, within 4GB. Compute the 32 bit delta from the start of the relocated .text and this relocated .xdata, and add that delta to the UnwindInfoAddress fields for each RUNTIME_FUNCTION in the .pdata section.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 22, 2016

There are some built-in assumptions in RUNTIME_FUNCTION.

Yes, I know about these limitations. Some special MemoryManager should be implemented to guarantee 32 bit delta between sections.

RTDyld will relocate each .text section independently. Call
RtlAddFunctionTable once per .text section, setting the BaseAddress to the
address of the relocated base of the section. Then use the contents of the
associated .pdata as the function table. You can safely ignore the
relocations on the begin/end members, since the underlying values are
already correct if the base of the image is the same as the base of the
section.

Then, you must, somehow, guarantee that the associated .xdata is loaded at a
higer address than the .text, within 4GB. Compute the 32 bit delta from the
start of the relocated .text and this relocated .xdata, and add that delta
to the UnwindInfoAddress fields for each RUNTIME_FUNCTION in the .pdata
section.

Thanks for valuable reply. As I understand if the 32 bit difference is guaranteed all relocations will be safely performed by RTDyld.

@weliveindetail
Copy link
Contributor

Thanks for valuable reply. As I understand if the 32 bit difference is
guaranteed all relocations will be safely performed by RTDyld.

Don't want to add more noise to this issue. Just in case anyone wants to workaround the 64-bit relocations issue on Win64, you may want to use this patch to RuntimeDyldCOFFX86_64.h
weliveindetail/pj-llvm@f9f26dc

Works great for us, I just didn't find the time to push this upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla mcjit
Projects
None yet
Development

No branches or pull requests

5 participants