Modify

Ticket #3869 (closed Bugs: fixed)

Opened 4 years ago

Last modified 23 months ago

Unconditional call to memset in value_initialized()

Reported by: agurtovoy Owned by: niels_dekker
Milestone: Boost 1.42.0 Component: utility
Version: Boost 1.41.0 Severity: Problem
Keywords: Cc: agurtovoy@…

Description

The call is there to workaround bugs in a few specific compilers, but is applied unconditionally, cannot be optimized away, and thus significantly limits applicability of the component in a library context.

See  http://old.nabble.com/boost::mpl::for_each-and-value_initialized-td25246848.html for the full discussion.

Attachments

config_compiler.patch Download (3.4 KB) - added by niels_dekker 4 years ago.
Adds BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET to trunk/boost/config/compiler
no_complete_value_initialization.patch Download (10.9 KB) - added by niels_dekker 4 years ago.
New proposed patch, adding BOOST_NO_COMPLETE_VALUE_INITIALIZATION. Replaces the old config_compiler.patch (Update: removed Boost.Test dependency, as suggested by John Maddock.)
value_init_workaround.patch Download (1.6 KB) - added by niels_dekker 4 years ago.
Made memset call conditional, based on BOOST_NO_COMPLETE_VALUE_INITIALIZATION
value_init_workaround_test.cpp Download (3.3 KB) - added by niels_dekker 4 years ago.

Change History

Changed 4 years ago by niels_dekker

Adds BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET to trunk/boost/config/compiler

comment:1 Changed 4 years ago by niels_dekker

Fernando (the author of value_init) mailed me that it's okay to him if I add an #ifdef to utility/value_init.hpp:

  #ifdef BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET
       std::memset(&x, 0, sizeof(x));
  #endif

But therefor, this macro needs to be defined in <boost/config/compiler/...>, for the relevant compiler versions. Hereby I have attached my proposed additions to the trunk version of boost/config/compiler:  config_compiler.patch Please check if it's okay.

comment:2 Changed 4 years ago by johnmaddock

The patch is OK as far as it goes, but we should have documentation and tests for each new macro, please see: http://www.boost.org/doc/libs/1_41_0/libs/config/doc/html/boost_config/guidelines_for_boost_authors.html#boost_config.guidelines_for_boost_authors.adding_new_defect_macros

Thanks, John.

comment:3 Changed 4 years ago by niels_dekker

Thanks, John. I read that there's a BOOST_NO_SOMETHING naming convention. So now I'm considering to name the macro BOOST_NO_PROPER_VALUE_INITIALIZATION_FOR_ALL_TYPES. (Instead of BOOST_VALUE_INITIALIZATION_NEEDS_MEMSET.) So that the unit test should show that compilers that have this macro defined do indeed do an improper value-initialization, for at least one type.

I'll have a closer look later...

comment:4 Changed 4 years ago by niels_dekker

Hereby I have just added a new patch for boost/config (to be applied to the trunk):  https://svn.boost.org/trac/boost/attachment/ticket/3869/no_complete_value_initialization.patch It adds a new macro, BOOST_NO_COMPLETE_VALUE_INITIALIZATION, and includes documentation and tests. Please tell me what you think.

Changed 4 years ago by niels_dekker

New proposed patch, adding BOOST_NO_COMPLETE_VALUE_INITIALIZATION. Replaces the old config_compiler.patch (Update: removed Boost.Test dependency, as suggested by John Maddock.)

Changed 4 years ago by niels_dekker

Made memset call conditional, based on BOOST_NO_COMPLETE_VALUE_INITIALIZATION

comment:5 Changed 4 years ago by niels_dekker

The  value_init_workaround.patch I just attached should resolve this ticket. It assumes that the defect macro BOOST_NO_COMPLETE_VALUE_INITIALIZATION is defined for those compilers that need it (ticket #4080). Note that it adds an extra macro, BOOST_DETAIL_VALUE_INIT_WORKAROUND (defined as either 0 or 1), which controls whether the memset workaround is applied. This macro is merely intended as an implementation detail: I would like to have an extra test which toggles the value of the macro, in order to check if the workaround is both effective and necessary.

comment:6 Changed 4 years ago by niels_dekker

(In [62307]) Made memset call in value_init conditional, see #3869. Updated the section "compiler issues" of its documentation.

Changed 4 years ago by niels_dekker

comment:7 Changed 4 years ago by niels_dekker

(In [63014]) Added value_init_workaround_test, reviewed by Fernando Cacciola, see #3869

comment:8 Changed 4 years ago by niels_dekker

(In [63638]) Merged value_init fixes (extra tests + documentation) from trunk, see #3472, #3869.

comment:9 Changed 23 months ago by viboes

Is there something else to be added? If not, could you close the issue?

comment:10 Changed 23 months ago by niels_dekker

  • Status changed from new to closed
  • Resolution set to fixed

Closed, as suggested by Vicente (viboes).

View

Add a comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
The resolution will be deleted. Next status will be 'reopened'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.