# HG changeset patch # User kbarrett # Date 1546288850 18000 # Node ID e90315ae8aa9b694e452c69477bcb22da5fb7d8a # Parent cfceb4df249928df62dfa4eed1e6141106e5cc1d 8213481: [REDO] Fix incorrect copy constructors in hotspot Summary: Fix and use ResourceObj copy constructor. Reviewed-by: coleenp, dholmes, kvn diff -r cfceb4df2499 -r e90315ae8aa9 src/hotspot/share/classfile/stackMapFrame.hpp --- a/src/hotspot/share/classfile/stackMapFrame.hpp Sun Dec 30 08:57:24 2018 +0700 +++ b/src/hotspot/share/classfile/stackMapFrame.hpp Mon Dec 31 15:40:50 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -63,6 +63,7 @@ ClassVerifier* _verifier; // the verifier verifying this method StackMapFrame(const StackMapFrame& cp) : + ResourceObj(cp), _offset(cp._offset), _locals_size(cp._locals_size), _stack_size(cp._stack_size), _stack_mark(cp._stack_mark), _max_locals(cp._max_locals), _max_stack(cp._max_stack), diff -r cfceb4df2499 -r e90315ae8aa9 src/hotspot/share/libadt/dict.cpp --- a/src/hotspot/share/libadt/dict.cpp Sun Dec 30 08:57:24 2018 +0700 +++ b/src/hotspot/share/libadt/dict.cpp Mon Dec 31 15:40:50 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -161,7 +161,7 @@ //------------------------------Dict----------------------------------------- // Deep copy a dictionary. -Dict::Dict( const Dict &d ) : _arena(d._arena), _size(d._size), _cnt(d._cnt), _hash(d._hash), _cmp(d._cmp) { +Dict::Dict( const Dict &d ) : ResourceObj(d), _arena(d._arena), _size(d._size), _cnt(d._cnt), _hash(d._hash), _cmp(d._cmp) { _bin = (bucket*)_arena->Amalloc_4(sizeof(bucket)*_size); memcpy( (void*)_bin, (void*)d._bin, sizeof(bucket)*_size ); for( uint i=0; i<_size; i++ ) { diff -r cfceb4df2499 -r e90315ae8aa9 src/hotspot/share/libadt/set.hpp --- a/src/hotspot/share/libadt/set.hpp Sun Dec 30 08:57:24 2018 +0700 +++ b/src/hotspot/share/libadt/set.hpp Mon Dec 31 15:40:50 2018 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -112,16 +112,15 @@ //------------------------------Set-------------------------------------------- class Set : public ResourceObj { - public: + protected: // Creates a new, empty set. - // DO NOT CONSTRUCT A Set. THIS IS AN ABSTRACT CLASS, FOR INHERITENCE ONLY Set(Arena *arena) : _set_arena(arena) {}; // Creates a new set from an existing set - // DO NOT CONSTRUCT A Set. THIS IS AN ABSTRACT CLASS, FOR INHERITENCE ONLY - Set(const Set &) {}; + Set(const Set & s) : ResourceObj(s) {}; + public: // Set assignment; deep-copy guts virtual Set &operator =(const Set &s)=0; virtual Set &clone(void) const=0; diff -r cfceb4df2499 -r e90315ae8aa9 src/hotspot/share/memory/allocation.cpp --- a/src/hotspot/share/memory/allocation.cpp Sun Dec 30 08:57:24 2018 +0700 +++ b/src/hotspot/share/memory/allocation.cpp Mon Dec 31 15:40:50 2018 -0500 @@ -162,78 +162,81 @@ #ifdef ASSERT void ResourceObj::set_allocation_type(address res, allocation_type type) { - // Set allocation type in the resource object - uintptr_t allocation = (uintptr_t)res; - assert((allocation & allocation_mask) == 0, "address should be aligned to 4 bytes at least: " INTPTR_FORMAT, p2i(res)); - assert(type <= allocation_mask, "incorrect allocation type"); - ResourceObj* resobj = (ResourceObj *)res; - resobj->_allocation_t[0] = ~(allocation + type); - if (type != STACK_OR_EMBEDDED) { - // Called from operator new() and CollectionSetChooser(), - // set verification value. - resobj->_allocation_t[1] = (uintptr_t)&(resobj->_allocation_t[1]) + type; - } + // Set allocation type in the resource object + uintptr_t allocation = (uintptr_t)res; + assert((allocation & allocation_mask) == 0, "address should be aligned to 4 bytes at least: " INTPTR_FORMAT, p2i(res)); + assert(type <= allocation_mask, "incorrect allocation type"); + ResourceObj* resobj = (ResourceObj *)res; + resobj->_allocation_t[0] = ~(allocation + type); + if (type != STACK_OR_EMBEDDED) { + // Called from operator new() and CollectionSetChooser(), + // set verification value. + resobj->_allocation_t[1] = (uintptr_t)&(resobj->_allocation_t[1]) + type; + } } ResourceObj::allocation_type ResourceObj::get_allocation_type() const { - assert(~(_allocation_t[0] | allocation_mask) == (uintptr_t)this, "lost resource object"); - return (allocation_type)((~_allocation_t[0]) & allocation_mask); + assert(~(_allocation_t[0] | allocation_mask) == (uintptr_t)this, "lost resource object"); + return (allocation_type)((~_allocation_t[0]) & allocation_mask); } bool ResourceObj::is_type_set() const { - allocation_type type = (allocation_type)(_allocation_t[1] & allocation_mask); - return get_allocation_type() == type && - (_allocation_t[1] - type) == (uintptr_t)(&_allocation_t[1]); + allocation_type type = (allocation_type)(_allocation_t[1] & allocation_mask); + return get_allocation_type() == type && + (_allocation_t[1] - type) == (uintptr_t)(&_allocation_t[1]); } -ResourceObj::ResourceObj() { // default constructor - if (~(_allocation_t[0] | allocation_mask) != (uintptr_t)this) { - // Operator new() is not called for allocations - // on stack and for embedded objects. - set_allocation_type((address)this, STACK_OR_EMBEDDED); - } else if (allocated_on_stack()) { // STACK_OR_EMBEDDED - // For some reason we got a value which resembles - // an embedded or stack object (operator new() does not - // set such type). Keep it since it is valid value - // (even if it was garbage). - // Ignore garbage in other fields. - } else if (is_type_set()) { - // Operator new() was called and type was set. - assert(!allocated_on_stack(), - "not embedded or stack, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")", - p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]); - } else { - // Operator new() was not called. - // Assume that it is embedded or stack object. - set_allocation_type((address)this, STACK_OR_EMBEDDED); - } - _allocation_t[1] = 0; // Zap verification value +// This whole business of passing information from ResourceObj::operator new +// to the ResourceObj constructor via fields in the "object" is technically UB. +// But it seems to work within the limitations of HotSpot usage (such as no +// multiple inheritance) with the compilers and compiler options we're using. +// And it gives some possibly useful checking for misuse of ResourceObj. +void ResourceObj::initialize_allocation_info() { + if (~(_allocation_t[0] | allocation_mask) != (uintptr_t)this) { + // Operator new() is not called for allocations + // on stack and for embedded objects. + set_allocation_type((address)this, STACK_OR_EMBEDDED); + } else if (allocated_on_stack()) { // STACK_OR_EMBEDDED + // For some reason we got a value which resembles + // an embedded or stack object (operator new() does not + // set such type). Keep it since it is valid value + // (even if it was garbage). + // Ignore garbage in other fields. + } else if (is_type_set()) { + // Operator new() was called and type was set. + assert(!allocated_on_stack(), + "not embedded or stack, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")", + p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]); + } else { + // Operator new() was not called. + // Assume that it is embedded or stack object. + set_allocation_type((address)this, STACK_OR_EMBEDDED); + } + _allocation_t[1] = 0; // Zap verification value } -ResourceObj::ResourceObj(const ResourceObj& r) { // default copy constructor - // Used in ClassFileParser::parse_constant_pool_entries() for ClassFileStream. - // Note: garbage may resembles valid value. - assert(~(_allocation_t[0] | allocation_mask) != (uintptr_t)this || !is_type_set(), - "embedded or stack only, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")", - p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]); - set_allocation_type((address)this, STACK_OR_EMBEDDED); - _allocation_t[1] = 0; // Zap verification value +ResourceObj::ResourceObj() { + initialize_allocation_info(); } -ResourceObj& ResourceObj::operator=(const ResourceObj& r) { // default copy assignment - // Used in InlineTree::ok_to_inline() for WarmCallInfo. - assert(allocated_on_stack(), - "copy only into local, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")", - p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]); - // Keep current _allocation_t value; - return *this; +ResourceObj::ResourceObj(const ResourceObj&) { + // Initialize _allocation_t as a new object, ignoring object being copied. + initialize_allocation_info(); +} + +ResourceObj& ResourceObj::operator=(const ResourceObj& r) { + assert(allocated_on_stack(), + "copy only into local, this(" PTR_FORMAT ") type %d a[0]=(" PTR_FORMAT ") a[1]=(" PTR_FORMAT ")", + p2i(this), get_allocation_type(), _allocation_t[0], _allocation_t[1]); + // Keep current _allocation_t value; + return *this; } ResourceObj::~ResourceObj() { - // allocated_on_C_heap() also checks that encoded (in _allocation) address == this. - if (!allocated_on_C_heap()) { // ResourceObj::delete() will zap _allocation for C_heap. - _allocation_t[0] = (uintptr_t)badHeapOopVal; // zap type - } + // allocated_on_C_heap() also checks that encoded (in _allocation) address == this. + if (!allocated_on_C_heap()) { // ResourceObj::delete() will zap _allocation for C_heap. + _allocation_t[0] = (uintptr_t)badHeapOopVal; // zap type + } } #endif // ASSERT diff -r cfceb4df2499 -r e90315ae8aa9 src/hotspot/share/memory/allocation.hpp --- a/src/hotspot/share/memory/allocation.hpp Sun Dec 30 08:57:24 2018 +0700 +++ b/src/hotspot/share/memory/allocation.hpp Mon Dec 31 15:40:50 2018 -0500 @@ -365,12 +365,14 @@ // Use second array's element for verification value to distinguish garbage. uintptr_t _allocation_t[2]; bool is_type_set() const; + void initialize_allocation_info(); public: allocation_type get_allocation_type() const; bool allocated_on_stack() const { return get_allocation_type() == STACK_OR_EMBEDDED; } bool allocated_on_res_area() const { return get_allocation_type() == RESOURCE_AREA; } bool allocated_on_C_heap() const { return get_allocation_type() == C_HEAP; } bool allocated_on_arena() const { return get_allocation_type() == ARENA; } +protected: ResourceObj(); // default constructor ResourceObj(const ResourceObj& r); // default copy constructor ResourceObj& operator=(const ResourceObj& r); // default copy assignment