Skip to content

Comments

Fix issue 20809 - return statement might access memory from destructed temporary#11214

Closed
welkam wants to merge 263 commits intodlang:stablefrom
welkam:issue_20809
Closed

Fix issue 20809 - return statement might access memory from destructed temporary#11214
welkam wants to merge 263 commits intodlang:stablefrom
welkam:issue_20809

Conversation

@welkam
Copy link
Contributor

@welkam welkam commented Jun 3, 2020

This is an attempt at fixing this issue. I realized that I bit more than I can chew on this one.
What this patch provides:

  1. An idea where the bug is and what kind of change is needed to fix it. (I copy pasted function and commented out code so that the peace of code that I want to run gets to run. To my surprise it worked and it passes all the tests. ¯_(ツ)_/¯)
  2. A test case that is combination of issue 20401, issue 20809 and Fix return statements: Don't access memory from destructed temporaries ldc-developers/ldc#3426

The problems:

  1. Copy paste is not good engineering but I dont know exactly how to proceed forward. There is not enough information arriving at appendDtors() to make proper decision. One solution is to create new function specific to this case. Or pass additional information trough function parameters(kinda hard when its in else statement). And of course dont break more code.
  2. Attached test is run with -inline and it triggers Issue 20803 - [REG2.071.0] Objects are destroyed too early with -inline

Solutions:
A. Some one uses this to kickstart a new pull request.
B. Some one helps with guidance because im not confident that I can make required changes and think of all possible cases to make sure it doesnt break something else.

MoonlightSentinel and others added 30 commits April 23, 2020 21:58
…s-live-function

getFunctionAttributes supports live function attribute
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
This was an issue on other platforms, as CC might be gcc, cc,
clang, sometimes even c++, g++ or clang++.
Instead of dealing with that, just provide our own wrapper script
which writes the arguments passed to it, and voila.
Rewrite test6952 to be independent of the CC variable
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
add TYrestrictPtr to backend
merged-on-behalf-of: Iain Buclaw <ibuclaw@gdcproject.org>
For the case of nested or non-trivial structs, there's no point checking all fields, the answer is still no.
dmd.dstruct: Return false immediately after setting StructPOD.no
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
Better compiler error message
merged-on-behalf-of: unknown
fix Issue 20771, 20772, 20775 disallow passing non-trivially copyable types as variadic arguments
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
elem.d: replace p/len with str
merged-on-behalf-of: Mathias LANG <pro.mathias.lang@gmail.com>
fix Issue 20747 - @LiVe tracking of non-pointer owners not done
merged-on-behalf-of: Atila Neves <atilaneves@users.noreply.github.com>
Detect and shortcut AliasSeq templates
merged-on-behalf-of: Atila Neves <atilaneves@users.noreply.github.com>
`$r:<regex>$` matches the output against the given regex
Don't use explicit mangles for builtins
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
Trim excessive zeros when printing integral compile-time reals
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
merge stable
merged-on-behalf-of: Walter Bright <WalterBright@users.noreply.github.com>
dlang-bot and others added 16 commits May 31, 2020 09:27
gloop now recycles memory instead of free/malloc
merged-on-behalf-of: Mathias LANG <pro.mathias.lang@gmail.com>
cxx-unittest: Update C++ baseline support to C++11
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
add scope varargs support to ob
merged-on-behalf-of: Walter Bright <WalterBright@users.noreply.github.com>
Previously, a regex used to match the result of __DATE__ would fail on
the 1st through 9th of a month, because it expected one space between
the month and day instead of two.
Make tests pass on single-digit days of month
… prefixed with a non-standard "ERROR:"

So that dmd developpers working with IDE can click the first message.
fix issue 20824 - error messages generated by dmd build script can be…
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
De-virtualize toBasetype() so it is inlineable
merged-on-behalf-of: Walter Bright <WalterBright@users.noreply.github.com>
Fix tag generation on posix
merged-on-behalf-of: Florian <moonlightsentinel@disroot.org>
Port runnable/paranoia.sh to D
merged-on-behalf-of: Nicholas Wilson <thewilsonator@users.noreply.github.com>
@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @welkam! 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
2077 normal Union literal crashes the compiler
20747 normal @LiVe tracking of non-pointer owners not done
20771 normal va_arg doesn't work for structs with postblits
20772 normal va_arg doesn't work for structs with copy constructors
20775 normal Missing fail compilation test for passing types that need destruction

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 + dmd#11214"

@welkam
Copy link
Contributor Author

welkam commented Jun 3, 2020

Cmon git. I wanted to pull one commit not millions

@MoonlightSentinel
Copy link
Contributor

You don’t need to open another pull request, you can fix & force push your branch.

Pro Git is a worthwhile read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.