We should not need the personality routine on x86. Also, the x64 unwind info codes referred to in next comment do not make sense on x86.

Will the JIT ever pass non-zero unwindSize here for x86 Linux? If the unwindSize is always zero on x86 Linux (that I think should be the case), this method should do nothing for x86.

@jkotas I'm not sure. Compiler::unwindReserveFuncHelper seems to pass sizeof(func->unwindCodes) - func->unwindCodeSlot as unwindSize into CEEJitInfo::reserveUnwindInfo.

Is there any documented standard related with x86? I read https://github.com/dotnet/coreclr/blob/master/Documentation/botr/clr-abi.md , but some x86-related stuffs are missing.

There is no standard unwind information on Windows x86. In CoreCLR, we just store the unwind information as part of the GCInfo on x86.

increaseUnwindInfoSize now does nothing for x86.

@parjong from your other PR, I have just realized you are trying to add unwind info that's a clone of the amd64 unwind info for x86 Linux. @jkotas, your comment was assuming that we are not going to introduce that for x86, right?

@janvorli Is there any restriction on THUMB code generation? It seems that the direct encoding is related with THUMB code.

@janvorli While reading ARM implementation, I found the following code from UnwindFragmentInfo::Finalize:

#if defined(_TARGET_ARM_)
    DWORD header = headerFunctionLength | (headerVers << 18) | (headerXBit << 20) | (headerEBit << 21) |
                   (headerFBit << 22) | (headerEpilogCount << 23) | (headerCodeWords << 28);
#elif defined(_TARGET_ARM64_)
    DWORD header               = headerFunctionLength | (headerVers << 18) | (headerXBit << 20) | (headerEBit << 21) |
                   (headerEpilogCount << 22) | (headerCodeWords << 27);
#endif // defined(_TARGET_ARM64_)

    ufiPrologCodes.AddHeaderWord(header);

It seems that ARM also stores the length of functions as unwind info as x86 does. IMO, it would be better to introduce a compact encoding as a separate PR.

@parjong we should not add the compact encoding, since it is not used by managed code on any of the platforms. @jkotas has explained that to me.

@janvorli Oh! Thank for confirm (but still I'm wondering why direct encoding works for ARM/ARM64).

It is not used for ARM / ARM64 in CoreCLR, the information is never encoded using the compact encoding.

This function adds space for the personality routine only, so it would be better to name it to reflect that.

Something like reservePresonalityRoutineInfo?

Why is this change needed? We use the PSPSym for for ARM32, ARM64 and amd64, so why should it be different for x86 Linux?

It depends on the intricate implementation details. The current Win x86 exception handling or CoreRT exception handling on all platforms do not need PSPSym, so it is certainly possible to get away without having it ... I do not know whether it is a good idea for Linux x86.

@janvorli According to CLR ABI, all the variables are accessed via frame pointer (EBP) and the managed code always stores frame pointer in its frame. That is one reason that I disable this.

In addition, I encountered various assertions failures that I could't understand when I tried to enable PSPSym-related code.

The problem is that the CLR ABI doesn't consider the FEATURE_EH_FUNCLETS to be enabled for x86 and the funclets are actually the thing that uses the PSPSym.
But since this is just a one line change, I won't block you on this. Once we start handling exceptions in complex nested try / catch scenarios, it will quickly show up whether it is better to enable the PSPSym or to fix some place in the stack walker instead.

@janvorli I agree. Later we may need to revise this again, but I currently have no idea.

The main question is whether the unwinder on Linux/x86 is guaranteed to restore the EBP of the parent frame, such that it doesn't need to be restored by the code itself. That is the main reason for the PSPSym: we can't trust that EBP (or the frame pointer, on other architectures) is restored, especially in nested EH scenarios. CoreRT does this properly, and thus doesn't need the PSPSym.

In reply to: 95909398 [](ancestors = 95909398)

I currently have no evidence whether Linux/x86 unwider can guarantee the correctness of EBP. We need more investigation.

In addition, I encountered the following assertion failure while codegen when I tried to enable PSPSym:

Assert failure(PID 2080 [0x00000820], Thread: 2080 [0x0820]): Assertion failed 'u2.emitArgTrackTop <= u2.emitArgTrackTab + emitMaxStackDepth' in 'System.AppDomain:SetupDomain(bool,ref,ref,ref,ref):this' (IL size 72)

    File: /home/parjong/projects/dotnet/coreclr/src/jit/emit.cpp Line: 6819
    Image: /root/dotnet-test/overlay/current/corerun

It would be better to enable PSPSym-related features in a separate PR.

Why is this needed? It seems to be AMD64 specific.

As x86/Linux uses RUNTIME_FUNCTION with EndAddress, we need to update its value inside CEEJitInfo::allocUnwindInfo.

For that purpose, we need to store and deliver various emitLocation variables from JIT to EE.

I think we need it for x86 with EH funclets too. What was the reason for excluding it?

The name is confusing to me. And it also seems we don't need the EndAddress for x86, or do we? If not, we can just revert the ifdef around the EndAddress to TARGET_AMD64 and remove this define.

As I understand, we need EndAddress.

The implementation of RtlpGetFunctionEndAddress for ARM/ARM64 computes the end address via multiplying the length of function (which seems to be the number of instructions) and the size of each instruction, which means that this logic is valid only when the instruction size is fixed.

Ah, that makes sense then. So can you please just modify the COMPACT_RUNTIME_FUNCTION to something like RUNTIME_FUNCTION_HAS_ENDADDRESS or RUNTIME_FUNCTION_NEEDS_ENDADDRESS or something like that?

You do not need the EndAddress on x86. Having it for x86 is just wasting space - encoding the length in xdata is more compact. EndAddress on Win x64 was a mistake. It would not be there if we can go back in time.

It has nothing to do with instruction with. E.g. ARM32 has variable instruction width as well (the instruction is either 2 or 4 bytes long) and it is able to run without EndAddress just fine.

The multiplier has to do with instruction size granularity. For x86, the granularity is 1 byte so you should make the multiplier 1.

@jkotas Could you let me know where UNWIND_INFO is generated? I tested with the following implementation of RtlpGetFunctionEndAddress, but it shows some strange behavior:

// This function returns the length of a function using the new unwind info on arm.
// Taken from minkernel\ntos\rtl\arm\ntrtlarm.h.
FORCEINLINE
ULONG
RtlpGetFunctionEndAddress (
    __in PT_RUNTIME_FUNCTION FunctionEntry,
    __in ULONG ImageBase
    )
{
    ULONG FunctionLength;

    FunctionLength = FunctionEntry->UnwindData;
    if ((FunctionLength & 3) != 0) {
        FunctionLength = (FunctionLength >> 2) & 0x7ff;
    } else {
        FunctionLength = *(PTR_ULONG)(ImageBase + FunctionLength) & 0x3ffff;
    }

    return FunctionEntry->BeginAddress + FunctionLength;
}

FunctionLength is 0 at the return state. It seems that relevant-information is currently not emiited.

The unwind info format is different for each platform. The Windows ABI documents linked at the start of https://github.com/dotnet/coreclr/blob/master/Documentation/botr/clr-abi.md have the specs of unwind info format for x64 and ARM.

There is no standalone unwind info defined for Windows x86. Windows x86 ABI was defined before the concept of standalone unwind info came to Windows and nobody bothered to define it retroactively.

On Windows x86, the unwind info is encoded as part of GCInfo in CoreCLR. The unwinder for it is at https://github.com/dotnet/coreclr/blob/master/src/vm/eetwain.cpp#L3897. For Linux 86, you have basically two options:

1. Keep using the existing scheme where the unwind info is encoded as part of GCInfo, maybe with some minimal modifications. These modifications have to be written down so that everybody is on the same page.
   or
2. Define your own new x86 unwind info format that is similar to the unwind info formats used on non-x86 platforms. The spec for this format has to be written down so that everybody is on the same page.

I would recommend going with option 1, at least for now.

I tried with the following implementation of RtlpGetFunctionEndAddress

This implementation of RtlpGetFunctionEndAddress specific to the current ARM unwind info format. That's why it does not just work on x86.

It seems strange to me to move the x64 unwind code-related functions (including DumpUnwindInfo) to a common place to compile for x86, since (I assume) they won't be used for x86.

This will get it to compile, but is there a TODO issue here related to GC handling of this nop on x86?

I have no idea, yet.

Please add a "TODO" comment to remind us to look at this, e.g.:

// TODO-Linux-x86: Do we need to handle the GC information for this NOP or JMP specially, as is done for other architectures?

In reply to: 95926754 [](ancestors = 95926754)

Why do you need to change legacy backend? Isn't Linux/x86 using RyuJIT/x86?

This change is introduced to fix build failure. lvaShadowSPslotsVar is not available when FEATURE_EH_FUNCLETS is set.

Yes, but why is this file getting built with FEATURE_EH_FUNCLETS defined on x86?

Do you mean that FEATURE_EH_FUNCLETS should be enabled only for RyuJIT?

For x86, yes. (FEATURE_EH_FUNCLETS is defined for legacy JIT for ARM32.)

Can you add a TODO here to say what needs to be done for Linux/x86?

I currently have no idea on which action will be required for prolog/epilog. Could you let me know if you have any idea?

typically, it would be (1) set up PSPSym (if required), (2) save/restore callee-save registers, (3) handle incoming args/return value. I assume you'll at least need to do (2).

936ebb7 adds TODO (I'm not sure about (1) and (3), and thus it adds only (2) as TODO).

Now that we have the EndAddress on amd64 only again, could you please revert the change here and get rid of the RUNTIME_FUNCTION_HAS_ENDADDRESS?

@janvorli As I understand, the following code in CEEJitInfo::allocUnwindInfo should be aligned with the above _RUNTIME_FUNCTION definition:

#if RUNTIME_FUNCTION_HAS_ENDADDRESS
    pRuntimeFunction->EndAddress        = currentCodeOffset + endOffset;
#endif

For me, it was hard to find this relationship when the above definition is written using _TARGET_AMD64_.

I would like not to revert this change if there is no further issue. Could you let me know your opinion?

Based on what @jkotas said, this is really AMD64 specific and we don't ever want to use it for anything else. That's why I would really prefer keeping it as it was to not to make false expectations that it might be enabled for something else than AMD64.

@janvorli 89b2a72 reverts the changes related with RUNTIME_FUNCTION_HAS_ENDADDRESS. Please take a look.

I wonder if this should rather be #if defined(_TARGET_AMD64_) || (defined(_TARGET_X86_) && FEATURE_EH_FUNCLETS)

@janvorli As you pointed out, the implementation of these functions exists only when _TARGET_AMD64 || (_TARGET_X86 && FEATURE_EH_FUNCLET). 77dc419 revised #ifdef condition.

It looks like you've added #if FEATURE_EH_FUNCLETS, but x86 won't get any of the enclosed declarations since there is no _TARGET_X86_ section. So can you remove the (unnecessary) added #if FEATURE_EH_FUNCLETS?

fe513b3 removes #if statement.

This #ifdef is unnecessary: genSetPSPSym() will bail out early because lvaPSPSym == BAD_VAR_NUM which is true because ehNeedsPSPSym() will return false (so lvaMarkLocalVars() won't create one).

Thank you for comment! fe513b3 also removes this #if statement.