Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Fix Issue 21110 - OOB memory access, safety violation#3267

Merged
dlang-bot merged 1 commit intodlang:stablefrom
RazvanN7:Issue_21110
Aug 9, 2021
Merged

Fix Issue 21110 - OOB memory access, safety violation#3267
dlang-bot merged 1 commit intodlang:stablefrom
RazvanN7:Issue_21110

Conversation

@RazvanN7
Copy link
Contributor

@RazvanN7 RazvanN7 commented Nov 10, 2020

It seems that this PR [1] forgot to add checks for OOB accesses. I had to modify _d_assert_fail to be able to put a nice error message.

[1] #1891

@dlang-bot
Copy link
Contributor

dlang-bot commented Nov 10, 2020

Thanks for your pull request and interest in making D better, @RazvanN7! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
21110 regression OOB memory access, safety violation

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "stable + druntime#3267"

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Nov 10, 2020
Copy link
Contributor

@thewilsonator thewilsonator left a comment

Choose a reason for hiding this comment

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

Please target stable

@Geod24
Copy link
Member

Geod24 commented Nov 26, 2020

Test seems to be failing on all platforms

@dlang-bot dlang-bot added Needs Rebase needs a `git rebase` performed Needs Work stalled labels May 27, 2021
@dlang-bot dlang-bot removed Needs Work Needs Rebase needs a `git rebase` performed stalled labels Aug 9, 2021
@RazvanN7 RazvanN7 force-pushed the Issue_21110 branch 3 times, most recently from 416940d to 4ca598b Compare August 9, 2021 10:20
@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Aug 9, 2021

@Geod24 @thewilsonator This seems to be green now. Could you please take another look?

@thewilsonator
Copy link
Contributor

This still targets master.

@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Aug 9, 2021

It'a an old regression. Since 2.065.

@RazvanN7 RazvanN7 changed the base branch from master to stable August 9, 2021 10:40
@RazvanN7
Copy link
Contributor Author

RazvanN7 commented Aug 9, 2021

Of course now we have spurious error messages :((

@RazvanN7 RazvanN7 closed this Aug 9, 2021
@RazvanN7 RazvanN7 reopened this Aug 9, 2021
@dlang-bot dlang-bot merged commit f978df3 into dlang:stable Aug 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Bug Fix Include reference to corresponding bugzilla issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants