Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix the srcBufferLength check #68270

Merged
merged 2 commits into from
Apr 20, 2022
Merged

fix the srcBufferLength check #68270

merged 2 commits into from
Apr 20, 2022

Conversation

kunalspathak
Copy link
Member

Should only get the buffer if it is > 0.

Fix failures in superpmi replay.

@dotnet-issue-labeler dotnet-issue-labeler bot 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
Member Author

@dotnet/jit-contrib @BruceForstall

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

@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
Member Author

/azp run runtime-coreclr superpmi-replay

@azure-pipelines
Copy link

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

@kunalspathak
Copy link
Member Author

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
Member

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
Member Author

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
Member Author

superpmi-replay is clean.

directhex pushed a commit to directhex/runtime that referenced this pull request Apr 21, 2022
@kunalspathak @directhex
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

@BruceForstall BruceForstall BruceForstall approved these changes

@jakobbotsch jakobbotsch jakobbotsch approved these changes

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