RFR: 8305590: Remove nothrow exception specifications from operator new

Kim Barrett kbarrett at openjdk.org
Tue Apr 18 19:02:50 UTC 2023


On Mon, 17 Apr 2023 17:09:44 GMT, Afshin Zafari <duke at openjdk.org> wrote:

> - The `throw()` (i.e., no throw) specifications are removed from the instances of `operator new` where _do not_ return `nullptr`.
> 
> - The `-fcheck-new` is removed from the gcc compile flags.
> 
> - The `operator new` and `operator delete` are deleted from `StackObj`.
> 
> - The `GrowableArrayCHeap::operator delete` is added to be matched with its corresponding allocation`AnyObj::operator new`, because gcc complains on that after removing the `-fcheck-new` flag. 
> - The `Thread::operator new`with and without `null` return are removed.
> 
> ### Tests
> local: linux-x64 gtest:GrowableArrayCHeap, macosx-aarch64 hotspot:tier1
> mach5: tiers 1-5

Changes requested by kbarrett (Reviewer).

src/hotspot/share/jfr/utilities/jfrAllocation.hpp line 58:

> 56:   NOINLINE void* operator new(size_t size);
> 57:   NOINLINE void* operator new (size_t size, const std::nothrow_t&  nothrow_constant) throw();
> 58:   NOINLINE void* operator new [](size_t size);

The changes to JfrCHeapObj are not correct, because these allocators currently _can_ return null.
Their implementation is just to return the result of calling the non-throwing allocator.  That's probably
not an ideal implementation.  Either the declaration needs to be left as-is or the implementation changed.

src/hotspot/share/memory/allocation.hpp line 287:

> 285:  private:
> 286:   void* operator new(size_t size) throw() = delete;
> 287:   void* operator new [](size_t size) throw() = delete;

The lingering nothrow exception-specs here are just clutter and can be removed.

src/hotspot/share/memory/allocation.hpp line 289:

> 287:   void* operator new [](size_t size) throw() = delete;
> 288:   void  operator delete(void* p) = delete;
> 289:   void  operator delete [](void* p) = delete;

Making these deleted functions public might provide better error messages if someone accidentally attempts to reference them.

src/hotspot/share/memory/allocation.hpp line 504:

> 502:   // Arena allocations
> 503:   void* operator new(size_t size, Arena *arena);
> 504:   void* operator new [](size_t size, Arena *arena) = delete;

`operator new[](size_t)` (down below, where github won't let me comment directly) should also have it's nothrow exception-spec removed.

src/hotspot/share/prims/jvmtiRawMonitor.hpp line 114:

> 112: 
> 113:   // Non-aborting operator new
> 114:   void* operator new(size_t size) {

This change is incorrect, as this can quite obviously return null.  And that seems to be intentional.
Presumably the callers are checking for a possible null allocation result (else there is a bug).  I think
it would be less confusing if this took a `std::nothrow_t` to be explicit about it's behavior, and updated
the caller(s) accordingly.  That would match the usual idiom.

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

PR Review: https://git.openjdk.org/jdk/pull/13498#pullrequestreview-1390703118
PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1170425313
PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1170429604
PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1170428457
PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1170434730
PR Review Comment: https://git.openjdk.org/jdk/pull/13498#discussion_r1170438594


More information about the serviceability-dev mailing list