Repository navigation
Arm64: [PAC-RET] cDAC changes #135959
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
base: main
Are you sure you want to change the base?
Arm64: [PAC-RET] cDAC changes #135959
Changes from all commits
549fe9d
5207462
22083ef
9a8b6a7
1f0e561
ed2360c
4029b6d
b6848e6
39c8e35
853f6aa
bc27a06
7716146
7eabd5d
b8a6130
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,10 +24,6 @@ internal static TargetCodePointer CodePointerFromAddress(TargetPointer address, | |
| { | ||
| return new TargetCodePointer(address.Value | Arm32ThumbBit); | ||
| } | ||
| else if (flags.HasFlag(CodePointerFlags.HasArm64PtrAuth)) | ||
| { | ||
| throw new NotImplementedException($"{nameof(CodePointerFromAddress)}: ARM64 with pointer authentication"); | ||
| } | ||
| Debug.Assert(flags == default); | ||
| return new TargetCodePointer(address.Value); | ||
| } | ||
|
|
@@ -40,11 +36,18 @@ internal static TargetPointer AddressFromCodePointer(TargetCodePointer code, Tar | |
| { | ||
| return new TargetPointer(code.Value & ~Arm32ThumbBit); | ||
| } | ||
| else if (flags.HasFlag(CodePointerFlags.HasArm64PtrAuth)) | ||
| { | ||
| throw new NotImplementedException($"{nameof(AddressFromCodePointer)}: ARM64 with pointer authentication"); | ||
| } | ||
| Debug.Assert(flags == default); | ||
| return new TargetPointer(code.Value); | ||
| } | ||
|
|
||
| internal static TargetCodePointer StripPtrAuthFromReturnAddress(TargetCodePointer returnAddress, Target target) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason this is separated out from stripping the thumb bit? This breaks the idea that If there are cases we aren't converting correctly, I'd rather update those to use the proper
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This applies to return addresses during stackwalk only. It does not apply to arbitrary code addresses like Arm32 thumb-bit stripping.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This feature is about PAC for return addresses only. If the PAC striping is done in AddressFromCodePointer, it is going to be done for code addresses, not just for return addresses. I guess it would work, but it would behave differently from how the runtime behaves. @max-charlamb Are you intentionally accepting this divergence to simplify cDAC? |
||
| { | ||
| if (returnAddress == TargetCodePointer.Null) | ||
| { | ||
| return TargetCodePointer.Null; | ||
| } | ||
|
|
||
| IPlatformMetadata metadata = target.Contracts.PlatformMetadata; | ||
| return new TargetCodePointer(returnAddress.Value & ~metadata.GetPointerAuthenticationMask()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using Microsoft.Diagnostics.DataContractReader.Contracts; | ||
| using Microsoft.Diagnostics.DataContractReader.TestInfrastructure; | ||
| using Xunit; | ||
|
|
||
| namespace Microsoft.Diagnostics.DataContractReader.Tests; | ||
|
|
||
| public class PlatformMetadataTests | ||
| { | ||
| private const ulong MetadataAddress = 0x1000; | ||
|
|
||
| [Fact] | ||
| public void MissingMaskLeavesReturnAddressUnchanged() | ||
| { | ||
| Target target = CreateTarget(); | ||
| Assert.Equal(0UL, target.Contracts.PlatformMetadata.GetPointerAuthenticationMask()); | ||
| Assert.Equal(0x1234UL, CodePointerUtils.StripPtrAuthFromReturnAddress(new(0x1234), target).Value); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(0xff7f800000000000UL, 0xa53afffebc33aae4UL, 0x00007ffebc33aae4UL)] | ||
| [InlineData(0xff70000000000000UL, 0xa53afffebc33aae4UL, 0x000afffebc33aae4UL)] | ||
| public void ClearsOnlyMaskBits(ulong mask, ulong signedAddress, ulong address) | ||
| { | ||
| Target target = CreateTarget(mask); | ||
|
|
||
| Assert.Equal(address, CodePointerUtils.StripPtrAuthFromReturnAddress(new(signedAddress), target).Value); | ||
| } | ||
|
|
||
| private static Target CreateTarget(ulong? mask = null) | ||
| { | ||
| MockTarget.Architecture arch = new() { Is64Bit = true, IsLittleEndian = true }; | ||
| TestPlaceholderTarget.Builder builder = new(arch); | ||
| TargetTestHelpers helpers = new(arch); | ||
| Dictionary<string, Target.FieldInfo> fields = []; | ||
| byte[] data = new byte[8]; | ||
| if (mask.HasValue) | ||
| { | ||
| fields[nameof(Data.PlatformMetadata.PointerAuthenticationMask)] = new() { Offset = 0 }; | ||
| helpers.Write(data, mask.Value); | ||
| } | ||
| builder.MemoryBuilder.AddHeapFragment(new() { Name = "PlatformMetadata", Address = MetadataAddress, Data = data }); | ||
| return builder | ||
| .AddTypes(new Dictionary<DataType, Target.TypeInfo> { [DataType.PlatformMetadata] = new() { Fields = fields } }) | ||
| .AddGlobals((Constants.Globals.PlatformMetadata, MetadataAddress)) | ||
| .AddContract<IPlatformMetadata>("c1") | ||
| .Build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We may need to change wording with certainty if we can confirm
TCR_EL1.T0SZalways= 17on Windows. It would make PAC algorithm useTCR_EL1.T0SZ = PACEffectiveTxSZ = 17inInsertPAC(). Thus, bit indexed 47 will always be used for storing PAC.