Skip to content

fix Issue 11176 - array.ptr in @safe code may point past end of array#5860

Merged
andralex merged 1 commit intodlang:masterfrom
WalterBright:fix11176
Sep 5, 2016
Merged

fix Issue 11176 - array.ptr in @safe code may point past end of array#5860
andralex merged 1 commit intodlang:masterfrom
WalterBright:fix11176

Conversation

@WalterBright
Copy link
Member

@WalterBright WalterBright commented Jun 12, 2016

@WalterBright
Copy link
Member Author

This is likely to be disruptive.

@WalterBright
Copy link
Member Author

WalterBright commented Jun 12, 2016

Depends on dlang/druntime#1590 and dlang/phobos#4426

@WalterBright WalterBright changed the title fix https://issues.dlang.org/show_bug.cgi?id=11176 fix Issue 11176 - array.ptr in @safe code may point past end of array Jun 13, 2016
@quickfur
Copy link
Member

LGTM. I like this, it's a neat solution to the nasty can o' worms about when/whether/how .ptr should/shouldn't be allowed in @safe code. Disruptive, yes, but it does solve the problem in a nice, self-contained way.

@JackStouffer
Copy link
Contributor

Looks like there are still some std.format issues

@andralex
Copy link
Member

I'll mark this for merging.

@andralex
Copy link
Member

Auto-merge toggled on

@WalterBright
Copy link
Member Author

This is blocked by numerous Phobos issues with .ptr

@WalterBright
Copy link
Member Author

Auto-merge toggled off

@WalterBright
Copy link
Member Author

Turned off auto-merge for the time being so the autotester doesn't waste time on this while it is blocked.

@MartinNowak
Copy link
Member

MartinNowak commented Jul 26, 2016

And again, and again, and again: Stop breaking code!

This is likely to be disruptive.

You already mention it yourself, but still do it.

Depends on dlang/druntime#1590 and dlang/phobos#4426

As always, this is a lie. You're about to break almost 1K dub packages that all need similar PRs.
We'll never be able to build a healthy ecosystem of keeping trashing working code at that pace.
The mechanism to properly do such changes is simple and well established, instead of breaking code, you introduce a deprecation that informs people about the upcoming error, then turn it into an error a few month later when the majority of packages is already fixed.

@dlang-bot
Copy link
Contributor

@WalterBright, thanks for your PR! By analyzing the annotation information on this pull request, we identified @yebblies, @klickverbot and @9rnsr to be potential reviewers. @yebblies: The PR was automatically assigned to you, please reassign it if you were identified mistakenly.

(The DLang Bot is under development. If you experience any issues, please open an issue at its repo.)

@codecov-io
Copy link

codecov-io commented Aug 25, 2016

Current coverage is 87.42% (diff: 66.66%)

Merging #5860 into master will increase coverage by <.01%

@@             master      #5860   diff @@
==========================================
  Files            98         97     -1   
  Lines         56216      56194    -22   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits          49143      49126    -17   
+ Misses         7073       7068     -5   
  Partials          0          0          

Powered by Codecov. Last update c0e821e...c8c1c8c

@WalterBright
Copy link
Member Author

Blocked by #6097

@WalterBright
Copy link
Member Author

This one is ready-to-merge now.

src/mtype.d Outdated
e.error("%s is not an expression", e.toChars());
return new ErrorExp();
}
else if (!(flag & 2) && sc.func && !sc.intypeof && sc.func.setUnsafe() && global.params.safe)
Copy link
Member

Choose a reason for hiding this comment

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

so what's 2 again?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@andralex
Copy link
Member

andralex commented Sep 5, 2016

Let's use a symbol for that 2 before pulling this Thanks!

@WalterBright
Copy link
Member Author

done

@andralex andralex merged commit 02d7ebe into dlang:master Sep 5, 2016
@WalterBright WalterBright deleted the fix11176 branch September 5, 2016 09:59
PetarKirov added a commit to PetarKirov/D-YAML that referenced this pull request Nov 4, 2016
The following DMD PRs added more rigorous safety checks directly
affecting this project:
* dlang/dmd#5852 (fix Issue 15399 - unaligned pointers are not
  `@safe`) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/emitter.d#L1011

* dlang/dmd#5940 (Unions may break immutability / unions with
  pointers are un-`@safe` ) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/event.d#L230

* dlang/dmd#5876 (Casting from `void[]` to `T[]` is erroneously
  considered `@safe`) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/loader.d#L186

* dlang/dmd#5860 (array.ptr in @safe code may point past end
  of array) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/zerostring.d#L35
PetarKirov added a commit to PetarKirov/D-YAML that referenced this pull request Nov 4, 2016
The following DMD PRs added more rigorous safety checks directly
affecting this project:
* dlang/dmd#5852 (fix Issue 15399 - unaligned pointers are not
  `@safe`) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/emitter.d#L1011

* dlang/dmd#5940 (Unions may break immutability / unions with
  pointers are un-`@safe` ) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/event.d#L230

* dlang/dmd#5876 (Casting from `void[]` to `T[]` is erroneously
  considered `@safe`) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/loader.d#L186

* dlang/dmd#5860 (array.ptr in @safe code may point past end
  of array) - triggered at line:
  https://github.com/kiith-sa/D-YAML/blob/v0.5.3/source/dyaml/zerostring.d#L35
Sign up for free to 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.

10 participants