On 12/5/18 7:58 AM, Richard Biener wrote:
On Wed, 5 Dec 2018, Jeff Law wrote:
On 12/4/18 6:16 AM, Richard Biener wrote:
This tries to make bugs like that in PR88317 harder to create by
introducing a bitmap_release function that can be used as
pendant to bitmap_initialize for non-allocated bitmap heads.
The function makes sure to poison the bitmaps obstack member
so the obstack the bitmap was initialized with can be safely
released.
The patch also adds a default constructor to bitmap_head
doing the same, but for C++ reason initializes to a
all-zero bitmap_obstack rather than 0xdeadbeef because
the latter isn't possible in constexpr context (it is
by using unions but then things start to look even more ugly).
The stage1 compiler might end up with a few extra runtime
initializers but constexpr makes sure they'll vanish for
later stages.
I had to paper over that you-may-not-use-memset-to-zero classes
with non-trivial constructors warning in two places and I
had to teach gengtype about CONSTEXPR (probably did so in
an awkward way - suggestions and pointers into gengtype
appreciated).
Bootstrapped (with host GCC 4.8) on x86_64-unknown-linux-gnu,
testing in progress.
The LRA issue seems to be rare enough (on x86_64...) that
I didn't trip over it sofar.
Comments? Do we want this? Not sure how we can easily
discover all bitmap_clear () users that should really
use bitmap_release (suggestion for a better name appreciated
as well - I thought about bitmap_uninitialize)
Richard.
2018-12-04 Richard Biener <rguent...@suse.de>
* bitmap.c (bitmap_head::crashme): Define.
* bitmap.h (bitmap_head): Add constexpr default constructor
poisoning the obstack member.
(bitmap_head::crashme): Declare.
(bitmap_release): New function clearing a bitmap and poisoning
the obstack member.
* gengtype.c (main): Make it recognize CONSTEXPR.
* lra-constraints.c (lra_inheritance): Use bitmap_release
instead of bitmap_clear.
* ira.c (ira): Work around warning.
* regrename.c (create_new_chain): Likewise.
I don't see enough complexity in here to be concerning -- so if it makes
it harder to make mistakes, then I'm for it.
Any comment about the -Wclass-memaccess workaround sprinkling around two
(void *) conversions? I didn't dig deep enough to look for a more
appropriate solution, also because there were some issues with older
host compilers and workarounds we installed elsewhere...
Using '*head = du_head ();' is the solution I like to encourage
to zero out typed objects. When the memory isn't initialized
and the type has a user-defined ctor, bypassing it is undefined
even if the ctor is a no-op (the ctor starts the lifetime of
the object). In that case, placement new is the appropriate
way to bring the object to life and value-initialize it:
new (head) du_head ();
If that's not good enough or portable enough to ancient compilers
then I would suggest adding a comment to explain the intent of
the cast.
Martin
Otherwise yes, it makes it harder to do mistakes. I'll probably
use bitmap_head::crashme instead of 0xdeadbeef in bitmap_release.
And of course we'd need to hunt down users of bitmap_clear that
should be bitmap_release instead...
Thanks,
Richard.
