On 3/3/21 10:33 PM, Jason Merrill wrote:
On 3/3/21 6:20 PM, Martin Sebor wrote:
...
I see what you mean, thanks, but I can't think of a test case to
trigger a false negative due to the smaller offset. Any suggestions?
My only ideas involve undefined behavior, casting the address to a
pointer to an unrelated too-large class. I don't know if that would
show up as a MEM_REF of this pattern.
Okay, let me see if I can come up with something based on that.
With more testing I also realized that focusing solely on an underlying
DECL like in the above doesn't prevent the warning when an object is
created in dynamically allocated memory or in a backing buffer. So
the attached revision both adjusts the offset computation upward and
handles all kinds of backing store and moves the logic into a helper
function for better readability. I've also added more tests.
Retested on x86_64-linux.
Thanks again for your help!
Martin
PS The TYPE_BINFO test isn't as quite as restrictive as I had hoped.
It means we consider all derived class, not just those with virtual
bases.
It's even less restrictive than that: all C++ classes have TYPE_BINFO.
I tried also requiring BINFO_VIRTUAL_P() to be true but that
doesn't work.
Right, BINFO_VIRTUAL_P is true for the binfo representing the virtual
base in the inheritance hierarchy, not on the binfo for the derived class.
Using BINFO_VTABLE() does work but it still isn't the same.
Indeed, virtual functions and virtual bases both cause a class to have a
vtable. You could also check BINFO_N_BASE_BINFOS.
Or not bother trying to restrict this to classes with virtual bases
(i.e. leave the patch as it is), since the result is just as correct for
other classes.
Jason
I think I'll just go with this last version if you're okay with it
as is. Okay to commit?
Martin
PS Let me propose a separate patch to add some text to the BINFO
comments in tree.h to clarify the macros per the above.
