Skip to content

fix the srcBufferLength check#68270

Merged
kunalspathak merged 2 commits intodotnet:mainfrom
kunalspathak:spmi_replay_fix
Apr 20, 2022
Merged

fix the srcBufferLength check#68270
kunalspathak merged 2 commits intodotnet:mainfrom
kunalspathak:spmi_replay_fix

Conversation

@kunalspathak
Copy link
Contributor

@kunalspathak kunalspathak commented Apr 20, 2022

Should only get the buffer if it is > 0.

Fix failures in superpmi replay.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 20, 2022
@ghost ghost assigned kunalspathak Apr 20, 2022
@ghost
Copy link

ghost commented Apr 20, 2022

Tagging subscribers to this area: @JulieLeeMSFT
See info in area-owners.md if you want to be subscribed.

Issue Details

Should only get the buffer if it is > 0.

Fix failures in superpmi replay.

Author: kunalspathak
Assignees: kunalspathak
Labels:

area-CodeGen-coreclr
Milestone: -

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Apr 20, 2022

@dotnet/jit-contrib @BruceForstall

BruceForstall
BruceForstall approved these changes Apr 20, 2022
View reviewed changes
Copy link
Contributor

@BruceForstall BruceForstall left a comment

Choose a reason for hiding this comment

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

LGTM.

You could trigger the replay job to be sure this fixes (all) the issues.

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Apr 20, 2022

/azp run runtime-coreclr superpmi-replay

@azure-pipelines
Copy link

azure-pipelines bot commented Apr 20, 2022

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Apr 20, 2022

You could trigger the replay job to be sure this fixes (all) the issues.

I think we should include src\coreclr\tools\superpmi\* as one of the criteria to trigger this job. I will update this PR.

@BruceForstall
Copy link
Contributor

BruceForstall commented Apr 20, 2022

I think we should include src\coreclr\tools\superpmi* as one of the criteria to trigger this job. I will update this PR.

Agreed. Although it's a little uncommon to change superpmi without also changing the JIT/EE GUID.

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Seems like this should work (that is, manually triggering the pipeline). Maybe there's some configuration thing we could fix.

@kunalspathak
Copy link
Contributor Author

kunalspathak commented Apr 20, 2022

Seems like this should work (that is, manually triggering the pipeline). Maybe there's some configuration thing we could fix.

Yes, already triggered it in https://dev.azure.com/dnceng/public/_build/results?buildId=1728724&view=results

@runfoapp runfoapp bot mentioned this pull request Apr 20, 2022
Closed
@kunalspathak kunalspathak merged commit b733b38 into dotnet:main Apr 20, 2022
@kunalspathak
Copy link
Contributor Author

kunalspathak commented Apr 20, 2022

superpmi-replay is clean.

directhex pushed a commit to directhex/runtime that referenced this pull request Apr 21, 2022
@kunalspathak
fix the srcBufferLength check (dotnet#68270)
1d851ec 
* fix the srcBufferLength check

* trigger superpmi replay job when changes to superpmi tool
@ghost ghost locked as resolved and limited conversation to collaborators May 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jakobbotsch jakobbotsch jakobbotsch approved these changes

+1 more reviewer

@BruceForstall BruceForstall BruceForstall approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@kunalspathak kunalspathak

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kunalspathak @BruceForstall @jakobbotsch