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.

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

Bug fixes #752

Merged
merged 2 commits into from
Feb 27, 2025
Merged

Bug fixes #752

merged 2 commits into from
Feb 27, 2025

Conversation

mjp41
Copy link
Member

@mjp41 mjp41 commented Feb 25, 2025

  • Address overflow in alignment calculation not failing an allocation.
  • Fix heuristic for how long to process a batch for

Copy link
Collaborator

@nwf nwf left a comment

Choose a reason for hiding this comment

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

LGTM, other than a request for more documentation.

return ((alignment - 1) | (size - 1)) + 1;
size_t result = ((alignment - 1) | (size - 1)) + 1;
// Check for overflow and return the maximum size.
return result < size ? result - 1 : result;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think (and I think z3 agrees, see below) that result < size implies result == 0, but it'd be good to write that down or perhaps use the other spelling?

(define-fun result ( (size (_ BitVec 32)) (align (_ BitVec 32)) ) (_ BitVec 32)
  (bvadd #x00000001 (bvor (bvsub align #x00000001) (bvsub size #x00000001))))

(declare-const align (_ BitVec 32))
(declare-const size (_ BitVec 32))

(define-fun conjecture () Bool
  (=> (bvult (result size align) size) (= (result size align) #x00000000)))
(assert (not conjecture))
(check-sat)

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. I used result-1 as it was a quick way to give -1 of the right size. I have moved to SIZE_MAX now, and also I rewrote it to generate better code with the sizeclass calc.

The bytes freed was not added to the total, but
overrode it.  This meant it never fired. This
commit fixes that.
@mjp41 mjp41 merged commit 6622dc5 into microsoft:main Feb 27, 2025
83 checks passed
@mjp41 mjp41 deleted the small_fixes branch February 27, 2025 08:52
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.

2 participants