Skip to content

JIT implementation for SHA instructions - #3

Merged
deeprobin merged 5 commits into
issue-256from
issue-256-jit
Jan 3, 2022
Merged

JIT implementation for SHA instructions#3
deeprobin merged 5 commits into
issue-256from
issue-256-jit

Conversation

@deeprobin

@deeprobindeeprobin commented Dec 20, 2021

Copy link
Copy Markdown
Owner

Please review 😄

Comment threadsrc/coreclr/jit/instrsxarch.h Outdated

// SHA1
INST3(sha1rnds4, "sha1rnds4", IUM_WR, BAD_CODE, BAD_CODE, 0xcc, Undefined_OF | Undefined_SF | Writes_ZF | Undefined_AF | Undefined_PF | Writes_CF )
INST2(sha1nexte, "sha1nexte", IUM_WR, BAD_CODE, 0xc8, Undefined_OF | Undefined_SF | Writes_ZF | Undefined_AF | Undefined_PF | Writes_CF )

@EgorBoEgorBoJan 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: how did you get that 0xc8 for sha1nexte for example?

Copy link
Copy Markdown
OwnerAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious: how did it get that 0xc8 for sha1nexte for example?

I looked at the opcode table in the kernel :D
https://github.com/torvalds/linux/blob/c9e6606c7fe92b50a02ce51dda82586ebdf99b48/arch/x86/lib/x86-opcode-map.txt#L790

@deeprobin
deeprobin merged commit 89c89dd into issue-256Jan 3, 2022
deeprobin pushed a commit that referenced this pull request Jun 23, 2022
* Initial implementation for contract customization
fix build errors
Move converter rooting to DefaultJsonTypeInfoResolver so that it can be used standalone
Fix ConfigurationList.IsReadOnly
Minor refactorings (#1)
* Makes the following changes:
* Move singleton initialization for DefaultTypeInfoResolver behind a static property.
* Consolidate JsonSerializerContext & IJsonTypeInfoResolver values to a single field.
* Move reflection fallback logic away from JsonSerializerContext and into JsonSerializerOptions
* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs
* remove testing of removed field
Simplify the JsonTypeInfo.CreateObject implemenetation (#2)
* Simplify the JsonTypeInfo.CreateObject implemenetation
* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs
* Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs
Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>
Co-authored-by: Krzysztof Wicher <mordotymoja@gmail.com>
Tests and fixes for JsonTypeInfoKind.None
TypeInfo type mismatch tests
Allow setting NumberHandling on JsonTypeInfoKind.None
test resolver returning wrong type of options
JsonTypeInfo/JsonPropertyInfo mutability tests
rename test file
Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver (#3)
* Move default converter rooting responsibility behind DefaultJsonTypeInfoResolver
* address feedback
Add simple test for using JsonTypeInfo<T> with APIs directly taking it
fix and tests for untyped/typed CreateObject
uncomment test cases, remove todo
More tests and tiny fixes
Add a JsonTypeInfoResolver.Combine test for JsonSerializerContext (#4)
* Fix JsonTypeInfoResolver.Combine for JsonSerializerContext
* Break up failing test
Fix simple scenarios for combining contexts (#6)
* Fix simple scenarios for combining contexts
* feedback
JsonSerializerContext combine test with different camel casing
Remove unneeded virtual calls & branching when accessing Get & Set delegates (#7)
JsonPropertyInfo tests everything minus ShouldSerialize & NumberHandling
Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
throw InvalidOperationException rather than ArgumentNullException for source gen when PropertyInfo.Name is assigned through JsonPropertyInfoValues
tests for duplicated property names and JsonPropertyInfo.NumberHandling
Add tests for NumberHandling and failing tests for ShouldSerialize
disable the failing test and add extra checks
disable remainder of the failing ShouldSerialize tests, fix working one
Fix ShouldSerialize and IgnoreCondition interop
Add failing tests for CreateObject + parametrized constructors
Fix CreateObject support for JsonConstructor types (#10)
* Fix CreateObject support for JsonConstructor types
* address feedback
Make contexts more combinator friendly (#9)
* Make contexts more combinator friendly
* remove converter cache
* redesign test to account for JsonConstructorAttribute
* Combine unit tests
* address feedback
* Add acceptance tests for DataContract attributes & Specified pattern (#11)
* Add private field serialization acceptance test (#13)
* tests, PR feedback (#14)
* PR feedback and extra tests
* Shorten class name, remove incorrect check (not true for polimorphic cases)
* Make parameter matching for custom properties map property Name with parameter (#16)
* Test static initialization with JsonTypeInfo (#17)
* Fix test failures and proper fix this time (#18)
* Fix test failures and proper fix this time
* reinstate ActiveIssueAttribute
* PR feedback and adjust couple of tests which don't set TypeInfoResolver
* fix IAsyncEnumerable tests
* Lock JsonSerializerOptions in JsonTypeInfo.EnsureConfigured()
Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
SamMonoRT pushed a commit that referenced this pull request Jun 15, 2026
…128163)
> [!NOTE]
> This PR was authored with assistance from GitHub Copilot.
Fixesdotnet#128044.
## Problem
createdump SIGSEGVs on Linux when generating a Heap-type minidump for a
process running interpreted code. The crash reproduces locally with the
`InterpreterStack` DumpTests debuggee and matches the CI failure that
prompted `<DumpTypes>Full</DumpTypes>` to be added as a temporary
workaround.
The faulting backtrace is:
```
#0 Thread::IsAddressInStack threads.cpp:6741
#1 Thread::EnumMemoryRegionsWorker threads.cpp:6909 (calls IsAddressInStack(currentSP))
#2 Thread::EnumMemoryRegions threads.cpp
#3 ThreadStore::EnumMemoryRegions
#4 ClrDataAccess::EnumMemDumpAllThreadsStack
#5 ClrDataAccess::EnumMemoryRegionsWorkerHeap (HEAP2-only path)
```
## Root cause
`Thread::m_pInterpThreadContext` was declared as a raw
`InterpThreadContext *`. In non-DAC code that's a normal host pointer,
but in
DAC mode the field's value is a target-process address. When
`IsAddressInStack` (a DAC-callable helper) dereferenced
`m_pInterpThreadContext->pStackStart` it read from a target-process
address
as if it were a host address, which faults inside createdump.
## Fix
Change the field type to `PTR_InterpThreadContext` (DPTR), matching the
treatment of other Thread fields like `m_pFrame`. In non-DAC builds
`DPTR(T)` is just `T*`, so there is no overhead or behavior change. In
DAC
builds the read goes through `__DPtr<T>` and marshals correctly from the
target.
Also remove the `<DumpTypes>Full</DumpTypes>` workaround on the
`InterpreterStack` DumpTests debuggee so the Heap path that originally
failed is exercised again.
## Validation
Locally reproduced the original SIGSEGV on Linux x64 with the auto-dump
mechanism (`DOTNET_DbgMiniDumpType=2` + `DOTNET_Interpreter=MethodA`)
running the `InterpreterStack` debuggee. With this fix applied,
createdump
produces a complete Heap dump (~74 MB) instead of crashing.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SamMonoRT pushed a commit that referenced this pull request Jun 15, 2026
An Android production app reported a native abort while building an
X.509 chain on arm64. The available tombstone snippet showed the process
aborting in `AndroidCryptoNative_X509ChainBuild` from `pal_x509chain.c`,
with the native guard reporting that parameter `ctx` was not a valid
pointer. The report did not include a repro or full tombstone, but the
observed failure mode means managed code reached the native build entry
point with a null `X509ChainContext*`.
```
Thread
/__w/1/s/src/native/libs/System.Security.Cryptography.Native.Android/pal_x509chain.c:113 (AndroidCryptoNative_X509ChainBuild): Parameter 'ctx' must be a valid pointer
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
pid: 0, tid: 31609 >>> com.app.name <<<
backtrace:
#00 pc 0x000000000002232c /system/lib64/libc.so (abort+116)
#1 pc 0x0000000000021fe8 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so
#2 pc 0x00000000000220b0 [removed]-KwPZdoEumri00C7kBm3pQw==/lib/arm64/libSystem.Security.Cryptography.Native.Android.so (AndroidCryptoNative_X509ChainBuild+88)
#3 pc 0x000000000000cfcc
```
`X509ChainContext` is created by
`AndroidCryptoNative_X509ChainCreateContext`. That initialization can
fail if Android certificate store setup or PKIX parameter construction
throws, or if required JNI global references cannot be created.
Previously, the managed Android chain path stored the returned
`SafeHandle` without checking whether context creation failed, so a
later build could pass a null native context to
`AndroidCryptoNative_X509ChainBuild` and terminate the app process.
This change makes context creation fail gracefully:
- The native create path checks Java exceptions around object creation
and method calls more consistently.
- Partial native contexts are destroyed if global-reference creation
fails.
- The Android interop wrapper checks the returned chain context
immediately, including a null safe-handle return, and throws
`CryptographicException` if initialization failed.
No regression test is included because the reliable failure modes depend
on Android platform/provider state or artificial fault injection, and a
test hook would be fragile and not representative.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Kevin Jones <kevin@vcsjones.com>
Sign up for freeto 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.

2 participants

@deeprobin@EgorBo