') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); [mono][jit] Emit a null check when storing to valuetype fields. by vargaz · Pull Request #82663 · dotnet/runtime · GitHub
Skip to content

[mono][jit] Emit a null check when storing to valuetype fields. - #82663

Merged
vargaz merged 3 commits into
dotnet:mainfrom
vargaz:store_membase_null_check
Apr 29, 2023
Merged

[mono][jit] Emit a null check when storing to valuetype fields.#82663
vargaz merged 3 commits into
dotnet:mainfrom
vargaz:store_membase_null_check

Conversation

@vargaz

@vargazvargaz commented Feb 25, 2023

Copy link
Copy Markdown
Contributor

Fixes#82535.

@ghostghost assigned vargazFeb 25, 2023
@vargaz

vargaz commented Feb 27, 2023

Copy link
Copy Markdown
ContributorAuthor

Still needs a regression test.

@vargaz
vargazforce-pushed the store_membase_null_check branch from 3e2401c to 3149a6dCompareFebruary 28, 2023 14:06
This was referenced Feb 28, 2023
Closed
Closed
@build-analysisbuild-analysisBot mentioned this pull request Feb 28, 2023
Closed
@vargaz

vargaz commented Mar 1, 2023

Copy link
Copy Markdown
ContributorAuthor

The new test seems to crash on the clr ?

 JIT/Regression/JitBlue/Runtime_82535/Runtime_82535/Runtime_82535.sh [FAIL]
[createdump] Invalid process id: task_for_pid(4941) FAILED (os/kern) failure (5)
[createdump] This failure may be because createdump or the application is not properly signed and entitled.
[createdump] Failure took 0ms
[createdump] waitpid() returned successfully (wstatus 0000ff00)
/private/tmp/helix/working/B0370906/w/B5A80970/e/JIT/Regression/JitBlue/Runtime_82535/Runtime_82535/Runtime_82535.sh: line 425: 4941 Segmentation fault: 11 (core dumped) $LAUNCHER $ExePath "${CLRTestExecutionArguments[@]}"

@build-analysisbuild-analysisBot mentioned this pull request Mar 1, 2023
Open
@lambdageek

lambdageek commented Mar 3, 2023•
edited
Loading

Copy link
Copy Markdown
Member

ping @mangod9 seems like there's two issues uncovered by the new test:

  1. some kind of segfault instead of a NullReferenceException - perhaps just in checked builds
  2. some kind of problem with coredump on osx

https://helixre107v0xdeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-82663-merge-d7b5be7d292f490986/JIT.Regression/1/console.40974b4f.log?helixlogtype=result

https://dev.azure.com/dnceng-public/public/_build/results?buildId=187998&view=logs&jobId=d660e68b-c00d-5466-3901-f111ca4fe7bd&j=d660e68b-c00d-5466-3901-f111ca4fe7bd&t=0b07bb4f-f6f8-5a4c-8b40-7d826f2cf02c

@mangod9

mangod9 commented Mar 4, 2023

Copy link
Copy Markdown
Member

Is this a consistent failure on linux x64 Debug builds? Adding @janvorli since we had fixed something similar in 7.

@lewinglewing added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 5, 2023
@ghost

ghost commented Mar 5, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #82535.

Author:vargaz
Assignees:vargaz
Labels:

area-CodeGen-coreclr, area-Codegen-JIT-mono

Milestone:-

@lewinglewing closed this Mar 10, 2023
@lewinglewing reopened this Mar 10, 2023
This was referenced Mar 11, 2023
Closed
Closed
@marek-safarmarek-safar removed the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 27, 2023
@vargaz
vargazforce-pushed the store_membase_null_check branch from 72080c6 to 03f8a97CompareMarch 28, 2023 15:22
@build-analysisbuild-analysisBot mentioned this pull request Mar 28, 2023
Closed
@vargaz

vargaz commented Apr 10, 2023

Copy link
Copy Markdown
ContributorAuthor

/azp run runtime

@azure-pipelines

azure-pipelinesBot commented Apr 10, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
vargazforce-pushed the store_membase_null_check branch from 03f8a97 to bba19c0CompareApril 10, 2023 04:56
@build-analysisbuild-analysisBot mentioned this pull request Apr 19, 2023
Closed
@vargaz
vargazforce-pushed the store_membase_null_check branch from bba19c0 to e9adfdeCompareApril 28, 2023 19:06
@build-analysisbuild-analysisBot mentioned this pull request Apr 28, 2023
Closed
@vargaz
vargazforce-pushed the store_membase_null_check branch from e9adfde to 917feaeCompareApril 28, 2023 22:37
@vargaz
vargaz merged commit 6bb5449 into dotnet:mainApr 29, 2023
@vargaz
vargaz deleted the store_membase_null_check branch April 29, 2023 04:23
MichalStrehovsky
MichalStrehovsky reviewed May 1, 2023
View reviewed changes
Comment threadsrc/tests/issues.targets
<Issue>https://github.com/dotnet/runtime/issues/78899</Issue>
</ExcludeList>
<ExcludeList Include="$(XunitTestBinBase)/JIT/Regression/JitBlue/Runtime_82535/*">
<Issue>https://github.com/dotnet/runtime/pull/82663</Issue>

@MichalStrehovskyMichalStrehovskyMay 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there a tracking issue for the CoreCLR problem with this test? We don't typically put pull requests into the Issue field because Mono pull requests are not CoreCLR tracking issues that someone would look at.

Cc @mangod9@janvorli @dotnet/jit-contrib

@vargazvargazMay 1, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Will make one.

MichalStrehovsky
MichalStrehovsky reviewed May 1, 2023
View reviewed changes
Comment threadsrc/tests/JIT/Regression/JitBlue/Runtime_82535/Runtime_82535.csproj
<PropertyGroup>
<OutputType>Exe</OutputType>
<Optimize>True</Optimize>
<RequiresProcessIsolation>true</RequiresProcessIsolation>

@MichalStrehovskyMichalStrehovskyMay 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does this require process isolation? RequiresProcessIsolation adds extra costs that the JIT team is actively working on reducing right now. Throwing and catching a nullref doesn't sounds like something that would need it.

@vargazvargazMay 1, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It caused a process crash on CI, which doesn't seem to happen any more.

vargaz added a commit to vargaz/runtime that referenced this pull request May 1, 2023
@vargaz
Reenable a test which doesn't seem to fail any more.
6be92c2
Re: dotnet#82663.
vargaz added a commit that referenced this pull request May 1, 2023
@vargaz
Reenable a test which doesn't seem to fail any more. (#85576)
d75e811
Re: #82663.
@vargaz

vargaz commented May 1, 2023

Copy link
Copy Markdown
ContributorAuthor

#85606

@ghostghost locked as resolved and limited conversation to collaborators May 31, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MichalStrehovskyMichalStrehovskyMichalStrehovsky left review comments
@lewinglewinglewing approved these changes
@lambdageeklambdageekAwaiting requested review from lambdageek
@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRT

Assignees

@vargazvargaz

Labels

area-Codegen-JIT-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mono: Crash when accessing a field of a null class object

6 participants

@vargaz@lambdageek@mangod9@lewing@MichalStrehovsky@marek-safar