Code Monkey home page Code Monkey logo

Comments (16)

matbesancon avatar matbesancon commented on July 24, 2024

cc @pfetsch do you have hints on what is happening here? Since this is about the SCIP memory management

from scip.

svigerske avatar svigerske commented on July 24, 2024

It could be helpful to know what version of SCIP this is, since the line numbers don't seem to match in the last release (8.0.4).

Compiling with NOBLKBUFMEM=true (or -DBMS_NOBLOCKMEM -DSCIP_NOBUFFERMEM) may also help to get more precise output, though in this case it seems quite clear what this is about. It should be harmless, but I wonder why this didn't happen more often.

from scip.

lperron avatar lperron commented on July 24, 2024

from scip.

lperron avatar lperron commented on July 24, 2024

from scip.

svigerske avatar svigerske commented on July 24, 2024

OK, I seem to have had mixed up line numbers.

Does this change fixes it for you?

--- a/src/scip/paramset.c
+++ b/src/scip/paramset.c
@@ -1147,6 +1147,7 @@ SCIP_RETCODE paramCreateChar(
    (*param)->paramtype = SCIP_PARAMTYPE_CHAR;
    (*param)->data.charparam.valueptr = valueptr;
    (*param)->data.charparam.defaultvalue = defaultvalue;
+   (*param)->data.charparam.curvalue = defaultvalue;
    if( allowedvalues != NULL )
    {
       SCIP_ALLOC( BMSduplicateMemoryArray(&(*param)->data.charparam.allowedvalues, allowedvalues, strlen(allowedvalues)+1) );

from scip.

lperron avatar lperron commented on July 24, 2024

It does not. Is the init called ?

from scip.

svigerske avatar svigerske commented on July 24, 2024

Not sure which "init" you mean.

The use-of-uninitialized-value is in SCIPparamGetChar in third_party/scip/src/scip/paramset.c:886:1, this is

   return param->data.charparam.curvalue;

It is called from SCIPparamSetChar(), which is called from paramCreateChar() (paramset.c:1157), so I hoped that adding an initialization of param->data.charparam.curvalue a few lines before (paramset.c:1150) should fix this.

My only other guess now is that a char is small and for some reason, it is reading more than the one curvalue byte, which then gives a read of uninitialized data. But this seems a bit far fetched.

from scip.

svigerske avatar svigerske commented on July 24, 2024

I now see that this is about the first parameter created. So, do you actually get more than this one warning? Sure that after the proposed fix, the remaining warning is still about paramCreateChar() and not another paramCreate*() ?

from scip.

lperron avatar lperron commented on July 24, 2024

This is the only one I get (surprisingly as all params share the same structure).

from scip.

svigerske avatar svigerske commented on July 24, 2024

OK.
third_party/scip/src/scip/paramset.c:886:1 is actually '}', and it should actually return the value in valueptr in the case. So try this change instead:

--- a/src/scip/paramset.c
+++ b/src/scip/paramset.c
@@ -1147,6 +1147,8 @@ SCIP_RETCODE paramCreateChar(
    (*param)->paramtype = SCIP_PARAMTYPE_CHAR;
    (*param)->data.charparam.valueptr = valueptr;
    (*param)->data.charparam.defaultvalue = defaultvalue;
+   if( valueptr != NULL )
+      *valueptr = defaultvalue;
    if( allowedvalues != NULL )
    {
       SCIP_ALLOC( BMSduplicateMemoryArray(&(*param)->data.charparam.allowedvalues, allowedvalues, strlen(allowedvalues)+1) );

from scip.

lperron avatar lperron commented on July 24, 2024

Good news, it works for char.
Bad news, it crashes later:

==3776419==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x561ed56ae273 in SCIPparamGetReal third_party/scip/src/scip/paramset.c:839:1
#1 0x561ed56b2602 in SCIPparamSetReal third_party/scip/src/scip/paramset.c:4720:18
#2 0x561ed56afb21 in paramCreateReal third_party/scip/src/scip/paramset.c:1121:4
#3 0x561ed56afb21 in SCIPparamsetAddReal third_party/scip/src/scip/paramset.c:1611:4
#4 0x561ed5cab7bf in SCIPsetAddRealParam third_party/scip/src/scip/set.c:3015:4
#5 0x561ed5ca38aa in SCIPsetCreate third_party/scip/src/scip/set.c:1252:4

from scip.

svigerske avatar svigerske commented on July 24, 2024

I don't think it crashes. It reads an uninitialized value. But it hardly does anything with it, that's why this is a harmless warning.

The same fix as for paramCreateChar() would then need to go into the other paramCreate*() functions.

from scip.

lperron avatar lperron commented on July 24, 2024

from scip.

lperron avatar lperron commented on July 24, 2024

from scip.

svigerske avatar svigerske commented on July 24, 2024

Here is a more complete patch:

--- a/src/scip/paramset.c
+++ b/src/scip/paramset.c
@@ -640,7 +640,7 @@ SCIP_RETCODE paramCopyString(
 
    /* get value of source parameter and copy it to target parameter */
    value = SCIPparamGetString(sourceparam);
-   SCIP_CALL( SCIPparamSetString(targetparam, set, messagehdlr, value, TRUE) );
+   SCIP_CALL( SCIPparamSetString(targetparam, set, messagehdlr, value, FALSE, TRUE) );
 
    return SCIP_OKAY;
 }
@@ -1186,7 +1186,7 @@ SCIP_RETCODE paramCreateString(
    SCIP_ALLOC( BMSduplicateMemoryArray(&(*param)->data.stringparam.defaultvalue, defaultvalue, strlen(defaultvalue)+1) );
    (*param)->data.stringparam.curvalue = NULL;
 
-   SCIP_CALL( SCIPparamSetString(*param, NULL, messagehdlr, defaultvalue, TRUE) );
+   SCIP_CALL( SCIPparamSetString(*param, NULL, messagehdlr, defaultvalue, TRUE, TRUE) );
 
    return SCIP_OKAY;
 }
@@ -1412,7 +1412,7 @@ SCIP_RETCODE paramParseString(
    /* remove the quotes */
    valuestr[len-1] = '\0';
    valuestr++;
-   SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, valuestr, TRUE) );
+   SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, valuestr, FALSE, TRUE) );
 
    return SCIP_OKAY;
 }
@@ -2135,7 +2135,7 @@ SCIP_RETCODE SCIPparamsetSetString(
    }
 
    /* set the parameter's current value */
-   SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, value, TRUE) );
+   SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, value, FALSE, TRUE) );
 
    return SCIP_OKAY;
 }
@@ -4529,15 +4529,17 @@ SCIP_RETCODE SCIPparamSetBool(
       /* check if the parameter is not fixed */
       SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
 
+      if( !initialize )
+         oldvalue = SCIPparamGetBool(param);
+
       /* set the parameter's current value */
-      oldvalue = SCIPparamGetBool(param);
       if( param->data.boolparam.valueptr != NULL )
          *param->data.boolparam.valueptr = value;
       else
          param->data.boolparam.curvalue = value;
 
-      /* call the parameter's change information method */
-      if( param->paramchgd != NULL && set != NULL )
+      /* call the parameter's change information method, unless initializing */
+      if( !initialize && param->paramchgd != NULL && set != NULL )
       {
          SCIP_RETCODE retcode;
 
@@ -4589,15 +4591,17 @@ SCIP_RETCODE SCIPparamSetInt(
       /* check if the parameter is not fixed */
       SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
 
+      if( !initialize )
+         oldvalue = SCIPparamGetInt(param);
+
       /* set the parameter's current value */
-      oldvalue = SCIPparamGetInt(param);
       if( param->data.intparam.valueptr != NULL )
          *param->data.intparam.valueptr = value;
       else
          param->data.intparam.curvalue = value;
 
-      /* call the parameter's change information method */
-      if( param->paramchgd != NULL && set != NULL )
+      /* call the parameter's change information method, unless initialization */
+      if( !initialize && param->paramchgd != NULL && set != NULL )
       {
          SCIP_RETCODE retcode;
 
@@ -4649,15 +4653,17 @@ SCIP_RETCODE SCIPparamSetLongint(
       /* check if the parameter is not fixed */
       SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
 
+      if( !initialize )
+         oldvalue = SCIPparamGetLongint(param);
+
       /* set the parameter's current value */
-      oldvalue = SCIPparamGetLongint(param);
       if( param->data.longintparam.valueptr != NULL )
          *param->data.longintparam.valueptr = value;
       else
          param->data.longintparam.curvalue = value;
 
-      /* call the parameter's change information method */
-      if( param->paramchgd != NULL && set != NULL )
+      /* call the parameter's change information method, unless initialization */
+      if( !initialize && param->paramchgd != NULL && set != NULL )
       {
          SCIP_RETCODE retcode;
 
@@ -4711,15 +4717,17 @@ SCIP_RETCODE SCIPparamSetReal(
       /* check if the parameter is not fixed */
       SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
 
+      if( !initialize )
+         oldvalue = SCIPparamGetReal(param);
+
       /* set the parameter's current value */
-      oldvalue = SCIPparamGetReal(param);
       if( param->data.realparam.valueptr != NULL )
          *param->data.realparam.valueptr = value;
       else
          param->data.realparam.curvalue = value;
 
-      /* call the parameter's change information method */
-      if( param->paramchgd != NULL && set != NULL )
+      /* call the parameter's change information method, unless initializing */
+      if( !initialize && param->paramchgd != NULL && set != NULL )
       {
          SCIP_RETCODE retcode;
 
@@ -4770,15 +4778,17 @@ SCIP_RETCODE SCIPparamSetChar(
 
       SCIP_CALL_QUIET( paramTestFixed(param, messagehdlr) );
 
+      if( !initialize )
+         oldvalue = SCIPparamGetChar(param);
+
       /* set the parameter's current value */
-      oldvalue = SCIPparamGetChar(param);
       if( param->data.charparam.valueptr != NULL )
          *param->data.charparam.valueptr = value;
       else
          param->data.charparam.curvalue = value;
 
-      /* call the parameter's change information method */
-      if( param->paramchgd != NULL && set != NULL )
+      /* call the parameter's change information method, unless initializing */
+      if( !initialize && param->paramchgd != NULL && set != NULL )
       {
          SCIP_RETCODE retcode;
 
@@ -4812,6 +4822,7 @@ SCIP_RETCODE SCIPparamSetString(
    SCIP_SET*             set,                /**< global SCIP settings, or NULL if param change method should not be called */
    SCIP_MESSAGEHDLR*     messagehdlr,        /**< message handler */
    const char*           value,              /**< new value of the parameter */
+   SCIP_Bool             initialize,         /**< is this the initialization of the parameter? */
    SCIP_Bool             quiet               /**< should the parameter be set quiet (no output) */
    )
 {
@@ -4826,17 +4837,19 @@ SCIP_RETCODE SCIPparamSetString(
    /* set the parameter's current value */
    if( param->data.stringparam.valueptr != NULL )
    {
-      oldvalue = *param->data.stringparam.valueptr;
+      if( !initialize )
+         oldvalue = *param->data.stringparam.valueptr;
       SCIP_ALLOC( BMSduplicateMemoryArray(param->data.stringparam.valueptr, value, strlen(value)+1) );
    }
    else
    {
-      oldvalue = param->data.stringparam.curvalue;
+      if( !initialize )
+         oldvalue = param->data.stringparam.curvalue;
       SCIP_ALLOC( BMSduplicateMemoryArray(&param->data.stringparam.curvalue, value, strlen(value)+1) );
    }
 
-   /* call the parameter's change information method */
-   if( param->paramchgd != NULL && set != NULL )
+   /* call the parameter's change information method, unless initializing */
+   if( !initialize && param->paramchgd != NULL && set != NULL )
    {
       SCIP_RETCODE retcode;
 
@@ -4993,7 +5006,7 @@ SCIP_RETCODE SCIPparamSetToDefault(
       break;
 
    case SCIP_PARAMTYPE_STRING:
-      SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, SCIPparamGetStringDefault(param), TRUE) );
+      SCIP_CALL( SCIPparamSetString(param, set, messagehdlr, SCIPparamGetStringDefault(param), FALSE, TRUE) );
       break;
 
    default:
diff --git a/src/scip/paramset.h b/src/scip/paramset.h
index 9674b46c76..d744e805fc 100644
--- a/src/scip/paramset.h
+++ b/src/scip/paramset.h
@@ -523,6 +523,7 @@ SCIP_RETCODE SCIPparamSetString(
    SCIP_SET*             set,                /**< global SCIP settings, or NULL if param change method should not be called */
    SCIP_MESSAGEHDLR*     messagehdlr,        /**< message handler */
    const char*           value,              /**< new value of the parameter */
+   SCIP_Bool             initialize,         /**< is this the initialization of the parameter? */
    SCIP_Bool             quiet               /**< should the parameter be set quiet (no output) */
    );
 
diff --git a/src/scip/set.c b/src/scip/set.c
index 33dcdb06e5..caf3485cb0 100644
--- a/src/scip/set.c
+++ b/src/scip/set.c
@@ -3414,7 +3414,7 @@ SCIP_RETCODE SCIPsetChgStringParam(
    assert(set != NULL);
    assert(param != NULL);
 
-   retcode = SCIPparamSetString(param, set, messagehdlr, value, TRUE);
+   retcode = SCIPparamSetString(param, set, messagehdlr, value, FALSE, TRUE);
 
    if( retcode != SCIP_PARAMETERWRONGVAL )
    {

(paramset.patch.txt)

If something still comes up, then we need the backtrace again.

from scip.

lperron avatar lperron commented on July 24, 2024

First tests seem ok. Thanks for the speedy patch.

from scip.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.