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

Fix relocs errors on riscv64 #111317

Merged
merged 23 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/coreclr/jit/emitriscv64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1627,7 +1627,11 @@ unsigned emitter::emitOutputCall(const insGroup* ig, BYTE* dst, instrDesc* id, c
int reg2 = ((int)addr & 1) + 10;
addr = addr ^ 1;

assert(isValidSimm32(addr - (ssize_t)dst));
if (!emitComp->opts.compReloc)
{
assert(isValidSimm32(addr - (ssize_t)dst));
}

assert((addr & 1) == 0);

dst += 4;
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/jit/emitriscv64.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,11 +195,11 @@ static bool isValidSimm21(ssize_t value)
return -(((int)1) << 20) <= value && value < (((int)1) << 20);
};

// Returns true if 'value' is a legal signed immediate 32 bit encoding.
// Returns true if 'value' is a legal signed immediate 32-bit encoding with the offset adjustment.
static bool isValidSimm32(ssize_t value)
{
return -(((ssize_t)1) << 31) <= value && value < (((ssize_t)1) << 31);
};
return (-(((ssize_t)1) << 31) - 0x800) <= value && value < (((ssize_t)1) << 31) - 0x800;
}

// Returns the number of bits used by the given 'size'.
inline static unsigned getBitWidth(emitAttr size)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ The .NET Foundation licenses this file to you under the MIT license.
<NativeLibrary Include="$(IlcFrameworkNativePath)libbrotlicommon.a" />
</ItemGroup>


<ItemGroup Condition="'$(StaticICULinking)' == 'true' and '$(NativeLib)' != 'Static' and '$(InvariantGlobalization)' != 'true'">
<NativeLibrary Include="$(IntermediateOutputPath)libs/System.Globalization.Native/build/libSystem.Globalization.Native.a" />
<DirectPInvoke Include="libSystem.Globalization.Native" />
Expand Down Expand Up @@ -222,6 +221,13 @@ The .NET Foundation licenses this file to you under the MIT license.
<NativeSystemLibrary Include="log" Condition="'$(_linuxLibcFlavor)' == 'bionic'" />
<NativeSystemLibrary Include="icucore" Condition="'$(_IsApplePlatform)' == 'true'" />
<NativeSystemLibrary Include="m" />
<!--
TODO: Double-check if libatomic's emulated CAS-128 works as expected once AOT applications are
functional on linux-riscv64: https://github.com/dotnet/runtime/issues/106223.
CAS-128 is natively supported starting with the Zacas extension in Linux 6.8; however, hardware support
for RVA23 profile is not available at the time of writing.
-->
<NativeSystemLibrary Include="atomic" Condition="'$(_targetArchitecture)' == 'riscv64'" />
</ItemGroup>

<ItemGroup>
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/nativeaot/Runtime/amd64/UniversalTransition.asm
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,6 @@ endif ; TRASH_SAVED_ARGUMENT_REGISTERS

ALTERNATE_ENTRY ReturnFrom&FunctionName

; We cannot make the label public as that tricks DIA stackwalker into thinking
; it's the beginning of a method. For this reason we export the address
; by means of an auxiliary variable.

; restore fp argument registers
movdqa xmm0, [rsp + DISTANCE_FROM_CHILDSP_TO_FP_REGS ]
movdqa xmm1, [rsp + DISTANCE_FROM_CHILDSP_TO_FP_REGS + 10h]
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/nativeaot/Runtime/arm/UniversalTransition.S
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,6 @@ NESTED_ENTRY Rhp\FunctionName, _TEXT, NoHandler

GLOBAL_LABEL ReturnFrom\FunctionName

// We cannot make the label public as that tricks DIA stackwalker into thinking
// it's the beginning of a method. For this reason we export an auxiliary variable
// holding the address instead.

// Move the result (the target address) to r12 so it doesn't get overridden when we restore the
// argument registers. Additionally make sure the thumb2 bit is set.
orr r12, r0, #1
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/nativeaot/Runtime/arm64/UniversalTransition.S
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,6 @@
mov x1, xip1 // Second parameter to target function
blr xip0

// We cannot make the label public as that tricks DIA stackwalker into thinking
// it's the beginning of a method. For this reason we export an auxiliary variable
// holding the address instead.
ALTERNATE_ENTRY ReturnFrom\FunctionName

// Move the result (the target address) to x12 so it doesn't get overridden when we restore the
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/nativeaot/Runtime/arm64/UniversalTransition.asm
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@
mov x1, xip1 ;; Second parameter to target function
blr xip0

;; We cannot make the label public as that tricks DIA stackwalker into thinking
;; it's the beginning of a method. For this reason we export an auxiliary variable
;; holding the address instead.
ALTERNATE_ENTRY ReturnFrom$FunctionName

;; Move the result (the target address) to x12 so it doesn't get overridden when we restore the
Expand Down
4 changes: 0 additions & 4 deletions src/coreclr/nativeaot/Runtime/i386/UniversalTransition.asm
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,6 @@ ALTERNATE_ENTRY _Rhp&FunctionName&@0

ALTERNATE_ENTRY _ReturnFrom&FunctionName

; We cannot make the label public as that tricks DIA stackwalker into thinking
; it's the beginning of a method. For this reason we export an auxiliary variable
; holding the address instead.

pop edx
pop ecx
add esp, 8
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@
ori $a1, $t8, 0 // Second parameter to target function
jirl $ra, $t7, 0

// We cannot make the label public as that tricks DIA stackwalker into thinking
// it's the beginning of a method. For this reason we export an auxiliary variable
// holding the address instead.
ALTERNATE_ENTRY ReturnFrom\FunctionName

// Move the result (the target address) to t3 so it doesn't get overridden when we restore the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@

// Normal case where a valid return address location is hijacked
sd a1, 0(a3)
tail ClearThreadState
tail LOCAL_LABEL(ClearThreadState)

LOCAL_LABEL(TailCallWasHijacked):

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@
mv a1, t1 // Second parameter to target function
jalr t0, t1, 0 // Jump to the function in t1

ALTERNATE_ENTRY ReturnFrom\FunctionName

// Restore the result address from t2
mv t2, a0 // Move result to t2

Expand Down Expand Up @@ -169,7 +171,6 @@

.endm


// To enable proper step-in behavior in the debugger, we need to have two instances
// of the thunk. For the first one, the debugger steps into the call in the function,
// for the other, it steps over it.
Expand Down
12 changes: 3 additions & 9 deletions src/coreclr/nativeaot/Runtime/unix/unixasmmacrosriscv64.inc
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,12 @@ C_FUNC(\Name):
.endm

.macro PREPARE_EXTERNAL_VAR Name, HelperReg
lui \HelperReg, %hi(C_FUNC(\Name))
addi \HelperReg, \HelperReg, %lo(C_FUNC(\Name))
.endm

.macro PREPARE_EXTERNAL_VAR_INDIRECT Name, HelperReg
lui \HelperReg, %hi(C_FUNC(\Name))
ld \HelperReg, %lo(C_FUNC(\Name))(\HelperReg)
la \HelperReg, C_FUNC(\Name) // Resolves the address in one step
.endm

.macro PREPARE_EXTERNAL_VAR_INDIRECT_W Name, HelperReg
lui \HelperReg, %hi(C_FUNC(\Name))
lw \HelperReg, %lo(C_FUNC(\Name))(\HelperReg)
la \HelperReg, C_FUNC(\Name)
lw \HelperReg, 0(\HelperReg)
.endm

.macro PROLOG_STACK_ALLOC Size
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ public void EmitReloc(ISymbolNode symbol, RelocType relocType, int delta = 0)
case RelocType.IMAGE_REL_BASED_LOONGARCH64_JIR:

case RelocType.IMAGE_REL_BASED_RISCV64_PC:
case RelocType.IMAGE_REL_BASED_RISCV64_JALR:
Debug.Assert(delta == 0);
// Do not vacate space for this kind of relocation, because
// the space is embedded in the instruction.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ public enum RelocType
IMAGE_REL_BASED_LOONGARCH64_PC = 0x16, // LoongArch64: pcalau12i+imm12
IMAGE_REL_BASED_LOONGARCH64_JIR = 0x17, // LoongArch64: pcaddu18i+jirl
IMAGE_REL_BASED_RISCV64_PC = 0x18, // RiscV64: auipc
IMAGE_REL_BASED_RISCV64_JALR = 0x19, // RiscV64: jalr (indirect jump)
IMAGE_REL_BASED_RELPTR32 = 0x7C, // 32-bit relative address from byte starting reloc
// This is a special NGEN-specific relocation type
// for relative pointer (used to make NGen relocation
Expand Down Expand Up @@ -470,7 +469,7 @@ private static unsafe int GetRiscV64PC(uint* pCode)
private static unsafe void PutRiscV64PC(uint* pCode, long imm32)
{
// Verify that we got a valid offset
Debug.Assert((int)imm32 == imm32);
Debug.Assert((imm32 >= (long)-0x80000000 - 0x800) && (imm32 < (long)0x80000000 - 0x800));

int doff = (int)(imm32 & 0xfff);
uint auipcInstr = *pCode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void EmitJMP(ISymbolNode symbol)
}
else
{
Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_RISCV64_JALR);
Builder.EmitReloc(symbol, RelocType.IMAGE_REL_BASED_RISCV64_PC);
EmitPC(Register.X29); // auipc x29, 0
EmitJALR(Register.X0, Register.X29, 0); // jalr x0, 0(x29)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,9 +552,8 @@ private void EmitRelocationsRiscV64(int sectionIndex, List<SymbolicRelocation> r
{
IMAGE_REL_BASED_DIR64 => R_RISCV_64,
IMAGE_REL_BASED_HIGHLOW => R_RISCV_32,
IMAGE_REL_BASED_RELPTR32 => R_RISCV_RELATIVE,
IMAGE_REL_BASED_RISCV64_PC => R_RISCV_PCREL_HI20,
IMAGE_REL_BASED_RISCV64_JALR => R_RISCV_CALL32,
IMAGE_REL_BASED_RELPTR32 => R_RISCV_64_LO12,
IMAGE_REL_BASED_RISCV64_PC => R_RISCV_64_HI20,
_ => throw new NotSupportedException("Unknown relocation type: " + symbolicRelocation.Type)
};

Expand Down
Loading