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

Fill uninitialized address-taken locals with poison pattern in debug codegen #13072

Closed
jkotas opened this issue Jul 13, 2019 · 3 comments · Fixed by #54685
Closed

Fill uninitialized address-taken locals with poison pattern in debug codegen #13072

jkotas opened this issue Jul 13, 2019 · 3 comments · Fixed by #54685
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jkotas
Copy link
Member

jkotas commented Jul 13, 2019

C/C++ compilers have this debugging/diagnostic aid for years.

More context: dotnet/coreclr#25674 (comment)

Related to: dotnet/csharplang#868

category:testing
theme:testing
skill-level:beginner
cost:small

@jkotas jkotas changed the title Fill uninitialized address-taken locals with poison pattern in debug builds Fill uninitialized address-taken locals with poison pattern in debug codegen Jul 18, 2019
@sandreenko sandreenko self-assigned this Aug 27, 2019
@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@GrabYourPitchforks
Copy link
Member

Honestly, I'd like to see this not just for debug codegen, but also as an opt-in thing that admins could enable in production applications. Going to my cloud hosting portal and hitting a checkbox that says "my app is misbehaving; please enable additional error checking at runtime" is a compelling scenario.

@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@sandreenko sandreenko removed the JitUntriaged CLR JIT issues needing additional triage label Jan 4, 2021
@sandreenko sandreenko modified the milestones: Future, 6.0.0 Jan 4, 2021
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Apr 21, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 21, 2021
@xtqqczze
Copy link
Contributor

#if DEBUG
// In debug, write some bogus data to the struct to ensure we have filled everything
// properly.
fixed (CORINFO_EE_INFO* tmp = &pEEInfoOut)
MemoryHelper.FillMemory((byte*)tmp, 0xcc, Marshal.SizeOf<CORINFO_EE_INFO>());
#endif

#if DEBUG
// In debug, write some bogus data to the struct to ensure we have filled everything
// properly.
fixed (CORINFO_LOOKUP* tmp = &pLookup)
MemoryHelper.FillMemory((byte*)tmp, 0xcc, sizeof(CORINFO_LOOKUP));
#endif

also L1210-L1214, L1385-L1389

#if DEBUG
result._size = size;
unsafe
{
byte* p = (byte*)(result.DangerousGetHandle());
for (int i = 0; i < size; i++)
{
p[i] = 0xcc;
}
}
#endif

also: L39-L48

#if DEBUG
Digits.Fill(0xCC);
#endif

@xtqqczze
Copy link
Contributor

#if DEBUG
// Ensure no "implicit 0" is happening, in case we move to pooling.
_buffer.AsSpan(_offset).Fill(0xCA);
#endif

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 26, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 24, 2021
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Jun 25, 2021
jakobbotsch added a commit that referenced this issue Jul 1, 2021
* Poison address exposed user variables in debug code

Fix #13072

* Run jit-format

* Use named scratch register and kill it in LSRA

* Enable it unconditionally for testing purposes

* Remove unnecessary modified reg on ARM

* Fix OSR and get rid of test code

* Remove a declaration

* Undo modified comment and use modulo instead of and

* Add a test

* Rephrase comment

Co-authored-by: Kunal Pathak <[email protected]>

* Disable poisoning test on mono

* Remove outdated line

Co-authored-by: Kunal Pathak <[email protected]>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 1, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
7 participants