[MIR] Fix vreg flag vector memory leak #112479
Conversation
SmallVector allocates on stack so that works out here.
optimisan
force-pushed
the
mem-leak-mir-flag
branch
from
66518e9 to
c1690d0
Compare
I don't see how this changes that. I would expect this to just move the leak point to cases where the small vector size is exceeded. Whenever VRegInfo is destroyed, the destructor must be called. That's the bug. But in this case I don't understand why Flags is a vector in the first place. The flags should just be a bitfield, this should be a simple integer field |
|
Was about to comment to say if this is even a right fix, but bitfield would do it.
|
| @@ -703,7 +703,7 @@ bool MIRParserImpl::parseRegisterInfo(PerFunctionMIParsingState &PFS, | |||
| return error(FlagStringValue.SourceRange.Start, | |||
| Twine("use of undefined register flag '") + | |||
| FlagStringValue.Value + "'"); | |||
| Info.Flags.push_back(FlagValue); | |||
| Info.Flags |= FlagValue; | |||
There was a problem hiding this comment.
We should also catch if a flag is set multiple times
There was a problem hiding this comment.
Would a warning be better for it?
There was a problem hiding this comment.
Added the check but can't be tested till relanding the WWM_REG flag patch.
There was a problem hiding this comment.
Can just add the error in a separate PR
This reverts commit f4f550c.
A fix-it patch for bec839d #110229.
SmallVectorwill inline 40 values (here) statically on the stack. Although once that is exhausted it'll heap allocate and won't be deallocated.No need for a container. This allows 8 flags for a register.
The virtual register flags vector had a memory leak because the vector's memory is not freed.
The
BumpPtrAllocatorhandles the deallocation and misses calling thestd::vector<uint8_t> Flagsdestructor.