Skip to content

Miri: fix determining size of an "extra function" allocation - #63076

Merged
bors merged 3 commits into
rust-lang:masterfrom
RalfJung:miri-fn-ptr-alloc-size
Jul 29, 2019
Merged

Miri: fix determining size of an "extra function" allocation#63076
bors merged 3 commits into
rust-lang:masterfrom
RalfJung:miri-fn-ptr-alloc-size

Conversation

@RalfJung

@RalfJungRalfJung commented Jul 28, 2019

Copy link
Copy Markdown
Member

Fixesa bug introduced by #62982. Best reviewed commit-by-commit.

r? @oli-obk

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 28, 2019
@RalfJungRalfJung changed the title Miri fn ptr alloc sizeMiri: fix determining size of an "extra function" allocationJul 28, 2019
This was referenced Jul 28, 2019
Closed
Merged
@rust-highfive

This comment has been minimized.

Sign in to view
@RalfJung
RalfJungforce-pushed the miri-fn-ptr-alloc-size branch from 73d11f5 to c119291CompareJuly 28, 2019 11:26
@oli-obk

oli-obk commented Jul 28, 2019

Copy link
Copy Markdown
Contributor

The function now locks the global map twice for gloabal non function pointer alloc IDs. Probably irrelevant perf wise though.

@oli-obk

oli-obk commented Jul 28, 2019

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jul 28, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit c119291 has been approved by oli-obk

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 28, 2019
@RalfJung

RalfJung commented Jul 28, 2019

Copy link
Copy Markdown
MemberAuthor

The function now locks the global map twice for gloabal non function pointer alloc IDs. Probably irrelevant perf wise though.

Indeed. It will only do the fn ptr lookup though for allocations that are not in the local alloc_map, so only for a small fraction. I'd offer to perf both versions but you seem to be fine with this since you already r+'d?

@oli-obk

oli-obk commented Jul 28, 2019

Copy link
Copy Markdown
Contributor

I'm fine with it. We had that before my static cycle PR and I didn't see a perf improvement on that PR, so...

Centril
Centril reviewed Jul 28, 2019
View reviewed changes
Comment threadsrc/librustc_mir/interpret/machine.rs Outdated
@RalfJung

RalfJung commented Jul 28, 2019

Copy link
Copy Markdown
MemberAuthor

@bors r=oli-obk

@bors

bors commented Jul 28, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit 0e602f1 has been approved by oli-obk

Centril added a commit to Centril/rust that referenced this pull request Jul 29, 2019
@Centril
Rollup merge ofrust-lang#63076- RalfJung:miri-fn-ptr-alloc-size, r=…
5922a19
…oli-obk
Miri: fix determining size of an "extra function" allocation
Fixes [a bug](rust-lang/miri#862) introduced by rust-lang#62982. Best reviewed commit-by-commit.
r? @oli-obk
@CentrilCentril mentioned this pull request Jul 29, 2019
Merged
bors added a commit that referenced this pull request Jul 29, 2019
@bors
Auto merge of#63094- Centril:rollup-lm7peuh, r=Centril
8b94e9e
Rollup of 6 pull requests
Successful merges:
- #62809 (rustc: Update wasm32 support for LLVM 9)
- #63055 (Various cleanups to save analysis)
- #63076 (Miri: fix determining size of an "extra function" allocation)
- #63077 (cleanup: Remove some language features related to built-in macros)
- #63086 (Ignore test cases that are not supported by vxWorks)
- #63092 (Update `impl Trait` gate issues)
Failed merges:
r? @ghost
@bors
bors merged commit 0e602f1 into rust-lang:masterJul 29, 2019
@RalfJung
RalfJung deleted the miri-fn-ptr-alloc-size branch July 29, 2019 08:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@CentrilCentrilCentril left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@oli-obkoli-obk

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@RalfJung@rust-highfive@oli-obk@bors@Centril