RFR: 8340801: Disable ubsan checks in some awt/2d coding [v4]

Lutz Schmidt lucy at openjdk.org
Mon Oct 14 09:58:12 UTC 2024


On Fri, 11 Oct 2024 18:05:58 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Matthias Baesken has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/master' into JDK-8340801
>>  - move ub.h to a better location
>>  - remove ubsan changes from jni_md.h
>>  - JDK-8340801
>
> Not a review, just commenting.
> 
> I very much support the work @MBaesken has done with ubsan in HotSpot.
> There's a bit of a chicken or egg problem with this kind of thing.  ubsan
> isn't really usable and in any way supported or supportable until the code has
> been made pretty much ubsan-clean.  Matthias has made substantial progress
> toward that for HotSpot, and I thank him for this effort.
> 
> There are currently four uses of the disabling attribute in HotSpot.  Two are
> for intentional things (writing to address zero intentionally, for testing
> signal handling and the like).  One is for a known issue that is being worked
> on, with an associated JBS issue; the attribute is being used to reduce
> testing noise in the meantime.  The fourth is related to some of my recent
> work (adjacent to, not caused by), and something that I think ought to be
> fixed.  I'll be filing a JBS issue.  Along the way there have been a
> substantial number of real bugs uncovered and addressed.
> 
> My only complaint has been a tendency to be a bit too quick to reach for the
> disabling attribute, without sufficient analysis and attempt to resolve in a
> way that corrects the problem.  Of the issues I've observed, the result of a
> real fix nearly always ben a straight-up improvement.

I agree with @kimbarrett in his fear that a convenience macro may lower the hurdle of just suppressing an ubsan complaint. On the other hand, using a macro makes the code look cleaner. Should there ever be a need to change how ubsan is quiesced, it can be done at one place with the use of a macro. I therefore will give the change a LGTM.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/21184#issuecomment-2410653806


More information about the client-libs-dev mailing list